1146902 - ARMv6: Do not inline atomics if the assembler will MOZ_CRASH

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Lars T Hansen [:lth]]

The ARM assembler does not yet support atomics for ARMv6 before ARMv6-K (since 1-byte and 2-byte atomics must be simulated with 4-byte atomics and there's some hair in implementing that).  The assembler will MOZ_CRASH if it tries to generate code for ARMv6.  A better thing to do would be to not inline the atomics on such platforms.

Comment 1

[Lars T Hansen [:lth]]

Hm, not totally trivial to use that workaround for asm.js.

Comment 2

[Lars T Hansen [:lth]]

Attachment: Disable Atomics / SAB on ARM until we have ARMv6 support

To handle asm.js we pretty much need to implement the support for ARMv6 and I had hoped today would not be the day for that, but it may be the only fix that works.

The attached patch just disables the shared memory types and Atomics on ARM.  But this will run afoul of test cases that test that those names *are* defined in nightly builds (dom/workers/test/test_worker_interfaces.js is just one of those tests).  I'm storing it here in case it is useful, but I do not intend to land it.

Comment 3

[Lars T Hansen [:lth]]

Attachment: Do not inline the Atomics if we're on an ARMv6

This takes care of the Ion piece of the puzzle.  The patch I've offered on bug 1077318 will take care of the asm.js piece.

Together, when I force ARMv6 in the ARM simulator, these two patches make all NYI crashes go away, and all my tests pass.

Comment 4

[Jan de Mooij [:jandem]]

Comment on attachment 8582596 [details] [diff] [review]
Do not inline the Atomics if we're on an ARMv6

Review of attachment 8582596 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

::: js/src/jit/Ion.cpp
@@ +3061,5 @@
>  
> +bool
> +jit::JitSupportsAtomics()
> +{
> +#if JS_CODEGEN_ARM

Super nit: #ifdef? #if probably also works but we use #ifdef everywhere else.

Comment 5

[Lars T Hansen [:lth]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bcd5980d0ec3

Bug 1077318 will also have to land before the problem goes away fully, though.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/bcd5980d0ec3