Closed
Bug 1146902
Opened 10 years ago
Closed 10 years ago
ARMv6: Do not inline atomics if the assembler will MOZ_CRASH
Categories
(Core :: JavaScript Engine: JIT, defect)
Tracking
()
RESOLVED
FIXED
mozilla39
Tracking | Status | |
---|---|---|
firefox39 | --- | fixed |
People
(Reporter: lth, Assigned: lth)
References
(Blocks 1 open bug)
Details
Attachments
(2 files)
1.21 KB,
patch
|
Details | Diff | Splinter Review | |
2.73 KB,
patch
|
jandem
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•10 years ago
|
||
Hm, not totally trivial to use that workaround for asm.js.
Assignee | ||
Comment 2•10 years ago
|
||
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.
Assignee | ||
Comment 3•10 years ago
|
||
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.
Attachment #8582596 -
Flags: review?(jdemooij)
Comment 4•10 years ago
|
||
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.
Attachment #8582596 -
Flags: review?(jdemooij) → review+
Assignee | ||
Comment 5•10 years ago
|
||
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•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
status-firefox39:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
You need to log in
before you can comment on or make changes to this bug.
Description
•