Closed Bug 1481338 Opened Last year Closed Last year

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

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
firefox63 --- wontfix
firefox64 --- fixed

People

(Reporter: glandium, Assigned: glandium)

References

Details

Attachments

(1 file)

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.
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.
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.
Flags: needinfo?(lhansen)
> 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.
Flags: needinfo?(lhansen)
Priority: -- → P2
Assignee: nobody → mh+mozilla
Comment on attachment 9010493 [details]
Bug 1481338 - Disable -Werror for -Watomic-alignment

Nathan Froyd [:froydnj] has approved the revision.
Attachment #9010493 - Flags: review+
Attachment #9010493 - Attachment description: Bug 1481338 - Disable -Werror for -Walign-alignment → Bug 1481338 - Disable -Werror for -Watomic-alignment
Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/autoland/rev/8778c2cfdfab
Disable -Werror for -Watomic-alignment r=froydnj
https://hg.mozilla.org/mozilla-central/rev/8778c2cfdfab
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Is there any downstream need to uplift this to Beta or can it ride the trains?
Flags: needinfo?(mh+mozilla)
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.
Flags: needinfo?(mh+mozilla)
You need to log in before you can comment on or make changes to this bug.