1481338 - [clang 7] error: misaligned or large atomic operation may incur significant performance penalty [-Werror,-Watomic-alignment]

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, enhancement, P2)

Description

[Mike Hommey [:glandium]]

When building Firefox with clang 7, on linux32 builds, I'm getting the following errors:

[task 2018-08-04T06:08:31.836Z] 06:08:31     INFO -  /builds/worker/workspace/build/src/js/src/jit/x86-shared/AtomicOperations-x86-shared-gcc.h:92:5: error: misaligned or large atomic operation may incur significant performance penalty [-Werror,-Watomic-alignment]
[task 2018-08-04T06:08:31.837Z] 06:08:31     INFO -      __atomic_load(addr, &v, __ATOMIC_SEQ_CST);
[task 2018-08-04T06:08:31.837Z] 06:08:31     INFO -      ^
[task 2018-08-04T06:08:31.837Z] 06:08:31     INFO -  /builds/worker/workspace/build/src/js/src/jit/x86-shared/AtomicOperations-x86-shared-gcc.h:101:5: error: misaligned or large atomic operation may incur significant performance penalty [-Werror,-Watomic-alignment]
[task 2018-08-04T06:08:31.837Z] 06:08:31     INFO -      __atomic_store(addr, &val, __ATOMIC_SEQ_CST);
[task 2018-08-04T06:08:31.837Z] 06:08:31     INFO -      ^
[task 2018-08-04T06:08:31.837Z] 06:08:31     INFO -  /builds/worker/workspace/build/src/js/src/jit/x86-shared/AtomicOperations-x86-shared-gcc.h:110:5: error: misaligned or large atomic operation may incur significant performance penalty [-Werror,-Watomic-alignment]
[task 2018-08-04T06:08:31.838Z] 06:08:31     INFO -      __atomic_exchange(addr, &val, &v, __ATOMIC_SEQ_CST);
[task 2018-08-04T06:08:31.838Z] 06:08:31     INFO -      ^
[task 2018-08-04T06:08:31.838Z] 06:08:31     INFO -  /builds/worker/workspace/build/src/js/src/jit/x86-shared/AtomicOperations-x86-shared-gcc.h:119:5: error: misaligned or large atomic operation may incur significant performance penalty [-Werror,-Watomic-alignment]
[task 2018-08-04T06:08:31.838Z] 06:08:31     INFO -      __atomic_compare_exchange(addr, &oldval, &newval, false, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST);
[task 2018-08-04T06:08:31.838Z] 06:08:31     INFO -      ^
[task 2018-08-04T06:08:31.838Z] 06:08:31     INFO -  /builds/worker/workspace/build/src/js/src/jit/x86-shared/AtomicOperations-x86-shared-gcc.h:128:12: error: misaligned or large atomic operation may incur significant performance penalty [-Werror,-Watomic-alignment]
[task 2018-08-04T06:08:31.838Z] 06:08:31     INFO -      return __atomic_fetch_add(addr, val, __ATOMIC_SEQ_CST);
[task 2018-08-04T06:08:31.838Z] 06:08:31     INFO -             ^
[task 2018-08-04T06:08:31.838Z] 06:08:31     INFO -  /builds/worker/workspace/build/src/js/src/jit/x86-shared/AtomicOperations-x86-shared-gcc.h:136:12: error: misaligned or large atomic operation may incur significant performance penalty [-Werror,-Watomic-alignment]
[task 2018-08-04T06:08:31.838Z] 06:08:31     INFO -      return __atomic_fetch_sub(addr, val, __ATOMIC_SEQ_CST);
[task 2018-08-04T06:08:31.839Z] 06:08:31     INFO -             ^
[task 2018-08-04T06:08:31.839Z] 06:08:31     INFO -  /builds/worker/workspace/build/src/js/src/jit/x86-shared/AtomicOperations-x86-shared-gcc.h:144:12: error: misaligned or large atomic operation may incur significant performance penalty [-Werror,-Watomic-alignment]
[task 2018-08-04T06:08:31.839Z] 06:08:31     INFO -      return __atomic_fetch_and(addr, val, __ATOMIC_SEQ_CST);
[task 2018-08-04T06:08:31.839Z] 06:08:31     INFO -             ^
[task 2018-08-04T06:08:31.839Z] 06:08:31     INFO -  /builds/worker/workspace/build/src/js/src/jit/x86-shared/AtomicOperations-x86-shared-gcc.h:152:12: error: misaligned or large atomic operation may incur significant performance penalty [-Werror,-Watomic-alignment]
[task 2018-08-04T06:08:31.839Z] 06:08:31     INFO -      return __atomic_fetch_or(addr, val, __ATOMIC_SEQ_CST);
[task 2018-08-04T06:08:31.839Z] 06:08:31     INFO -             ^
[task 2018-08-04T06:08:31.839Z] 06:08:31     INFO -  /builds/worker/workspace/build/src/js/src/jit/x86-shared/AtomicOperations-x86-shared-gcc.h:160:12: error: misaligned or large atomic operation may incur significant performance penalty [-Werror,-Watomic-alignment]
[task 2018-08-04T06:08:31.840Z] 06:08:31     INFO -      return __atomic_fetch_xor(addr, val, __ATOMIC_SEQ_CST);
[task 2018-08-04T06:08:31.840Z] 06:08:31     INFO -             ^
[task 2018-08-04T06:08:31.840Z] 06:08:31     INFO -  /builds/worker/workspace/build/src/js/src/jit/x86-shared/AtomicOperations-x86-shared-gcc.h:92:5: error: misaligned or large atomic operation may incur significant performance penalty [-Werror,-Watomic-alignment]
[task 2018-08-04T06:08:31.840Z] 06:08:31     INFO -      __atomic_load(addr, &v, __ATOMIC_SEQ_CST);
[task 2018-08-04T06:08:31.840Z] 06:08:31     INFO -      ^
[task 2018-08-04T06:08:31.840Z] 06:08:31     INFO -  /builds/worker/workspace/build/src/js/src/jit/x86-shared/AtomicOperations-x86-shared-gcc.h:101:5: error: misaligned or large atomic operation may incur significant performance penalty [-Werror,-Watomic-alignment]
[task 2018-08-04T06:08:31.840Z] 06:08:31     INFO -      __atomic_store(addr, &val, __ATOMIC_SEQ_CST);
[task 2018-08-04T06:08:31.840Z] 06:08:31     INFO -      ^
[task 2018-08-04T06:08:31.840Z] 06:08:31     INFO -  /builds/worker/workspace/build/src/js/src/jit/x86-shared/AtomicOperations-x86-shared-gcc.h:110:5: error: misaligned or large atomic operation may incur significant performance penalty [-Werror,-Watomic-alignment]
[task 2018-08-04T06:08:31.840Z] 06:08:31     INFO -      __atomic_exchange(addr, &val, &v, __ATOMIC_SEQ_CST);
[task 2018-08-04T06:08:31.840Z] 06:08:31     INFO -      ^
[task 2018-08-04T06:08:31.841Z] 06:08:31     INFO -  /builds/worker/workspace/build/src/js/src/jit/x86-shared/AtomicOperations-x86-shared-gcc.h:119:5: error: misaligned or large atomic operation may incur significant performance penalty [-Werror,-Watomic-alignment]
[task 2018-08-04T06:08:31.841Z] 06:08:31     INFO -      __atomic_compare_exchange(addr, &oldval, &newval, false, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST);
[task 2018-08-04T06:08:31.841Z] 06:08:31     INFO -      ^
[task 2018-08-04T06:08:31.841Z] 06:08:31     INFO -  /builds/worker/workspace/build/src/js/src/jit/x86-shared/AtomicOperations-x86-shared-gcc.h:128:12: error: misaligned or large atomic operation may incur significant performance penalty [-Werror,-Watomic-alignment]
[task 2018-08-04T06:08:31.841Z] 06:08:31     INFO -      return __atomic_fetch_add(addr, val, __ATOMIC_SEQ_CST);
[task 2018-08-04T06:08:31.841Z] 06:08:31     INFO -             ^
[task 2018-08-04T06:08:31.841Z] 06:08:31     INFO -  /builds/worker/workspace/build/src/js/src/jit/x86-shared/AtomicOperations-x86-shared-gcc.h:136:12: error: misaligned or large atomic operation may incur significant performance penalty [-Werror,-Watomic-alignment]
[task 2018-08-04T06:08:31.842Z] 06:08:31     INFO -      return __atomic_fetch_sub(addr, val, __ATOMIC_SEQ_CST);
[task 2018-08-04T06:08:31.842Z] 06:08:31     INFO -             ^
[task 2018-08-04T06:08:31.842Z] 06:08:31     INFO -  /builds/worker/workspace/build/src/js/src/jit/x86-shared/AtomicOperations-x86-shared-gcc.h:144:12: error: misaligned or large atomic operation may incur significant performance penalty [-Werror,-Watomic-alignment]
[task 2018-08-04T06:08:31.842Z] 06:08:31     INFO -      return __atomic_fetch_and(addr, val, __ATOMIC_SEQ_CST);
[task 2018-08-04T06:08:31.842Z] 06:08:31     INFO -             ^
[task 2018-08-04T06:08:31.842Z] 06:08:31     INFO -  /builds/worker/workspace/build/src/js/src/jit/x86-shared/AtomicOperations-x86-shared-gcc.h:152:12: error: misaligned or large atomic operation may incur significant performance penalty [-Werror,-Watomic-alignment]
[task 2018-08-04T06:08:31.842Z] 06:08:31     INFO -      return __atomic_fetch_or(addr, val, __ATOMIC_SEQ_CST);
[task 2018-08-04T06:08:31.843Z] 06:08:31     INFO -             ^
[task 2018-08-04T06:08:31.843Z] 06:08:31     INFO -  /builds/worker/workspace/build/src/js/src/jit/x86-shared/AtomicOperations-x86-shared-gcc.h:160:12: error: misaligned or large atomic operation may incur significant performance penalty [-Werror,-Watomic-alignment]
[task 2018-08-04T06:08:31.843Z] 06:08:31     INFO -      return __atomic_fetch_xor(addr, val, __ATOMIC_SEQ_CST);
[task 2018-08-04T06:08:31.843Z] 06:08:31     INFO -             ^
[task 2018-08-04T06:08:31.843Z] 06:08:31     INFO -  18 errors generated.

This is something we can "fix" with -Wno-error=atomic-alignment, but it seems worth looking to actually fix the underlying issue.

Comment 1

[Lars T Hansen [:lth]]

These functions should never be used on non-aligned data; even on 32-bit systems we should always be accessing data 64-byte aligned by construction.  We can assert that, though I don't know if that will make the warning go away.

Comment 2

[Mike Hommey [:glandium]]

Filed https://bugs.llvm.org/show_bug.cgi?id=38593.

Comment 3

[Mike Hommey [:glandium]]

That being said, wouldn't it be possible to write those atomic operations in terms of mozilla::Atomic? That would remove the need for separate implementations for gcc and msvc.

Comment 4

[Lars T Hansen [:lth]]

> That being said, wouldn't it be possible to write those atomic operations in terms of mozilla::Atomic? That would remove the need for separate implementations for gcc and msvc.

Not last time I looked.  One reason is that mozilla::atomic only guarantees up to pointer size, while we need 64-bit atomics also on 32-bit systems for WebAssembly.  Another reason is that these atomics must be compatible with the code generated by the JIT, and that code is hairy for some sizes on some platforms (eg, on MIPS32 there's a private spinlock for 64-bit atomics because the architecture does not have atomic doubleword operations).

What you're seeing in the jit/*/AtomicOperations-inl.h files are really temporary fixes.  What I really want to do is generate this code at startup with the JIT, on platforms that have a JIT, so that we can be guaranteed to have JIT-compatible behavior with minimal fuss.  That's bug 1394420, which has been languishing, but now that wasm threads are almost a thing again I probably need to devote some time to it.

Comment 5

[Mike Hommey [:glandium]]

Attachment: Bug 1481338 - Disable -Werror for -Watomic-alignment

Comment 6

[Nathan Froyd [:froydnj]]

Comment on attachment 9010493 [details]
Bug 1481338 - Disable -Werror for -Watomic-alignment

Nathan Froyd [:froydnj] has approved the revision.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8778c2cfdfab
Disable -Werror for -Watomic-alignment r=froydnj

Comment 8

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/8778c2cfdfab

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Is there any downstream need to uplift this to Beta or can it ride the trains?

Comment 10

[Mike Hommey [:glandium]]

This is only a problem for people using -Werror (--enable-warnings-as-errors), and if downstream are doing that, they're bound for self-inflicted pain anyways.