Closed
Bug 1225028
Opened 9 years ago
Closed 9 years ago
Remove support for Atomics.fence
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: lth, Assigned: lth)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
21.10 KB,
patch
|
bbouvier
:
review+
|
Details | Diff | Splinter Review |
Atomics.fence is not part of the current spec (and given the evolving memory model it is probably not supportable and it's not expected to make it into 1.0). We should remove it.
This entails both runtime and JIT work, I think, though in the case of the JIT most of the infrastructure for fences will continue to be used internally, so it's probably not wrenching.
Assignee | ||
Comment 1•9 years ago
|
||
Also in asm.js, of course.
Assignee | ||
Comment 2•9 years ago
|
||
Jukka says our JS code is no longer using Atomics.fence, so we can just remove it from the engine.
Assignee | ||
Comment 3•9 years ago
|
||
This removes Atomics.fence from Ion and Odin. It is not in the spec and not used by any current code. I did remove the few Wasm bits I found here on the assumption that they were only triggered by asm code.
Attachment #8737001 -
Flags: review?(bbouvier)
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → lhansen
Status: NEW → ASSIGNED
Comment 4•9 years ago
|
||
Comment on attachment 8737001 [details] [diff] [review]
remove Atomics.fence
Review of attachment 8737001 [details] [diff] [review]:
-----------------------------------------------------------------
Fire ze missiles. See below, I think you can remove even more code...
::: js/src/asmjs/WasmIonCompile.cpp
@@ -3019,5 @@
>
> // Atomics
> - case Expr::AtomicsFence:
> - *def = nullptr;
> - f.memoryBarrier(MembarFull);
You can also remove memoryBarrier(...) from FunctionCompiler, it's used only here (unless you plan to use it in another NYI function in a very near future).
::: js/src/jit/MCallOptimize.cpp
@@ -2927,5 @@
> - return InliningStatus_NotInlined;
> -
> - callInfo.setImplicitlyUsedUnchecked();
> -
> - MMemoryBarrier* fence = MMemoryBarrier::New(alloc());
If MMemoryBarrier isn't emitted anymore, it can be removed too (MIR.h, MOpcodes.h, Lowering, etc.). I see that LMemoryBarrier is used in other places.
Attachment #8737001 -
Flags: review?(bbouvier) → review+
Assignee | ||
Comment 5•9 years ago
|
||
Excellent call on MMemoryBarrier, I will investigate further. Thanks.
Assignee | ||
Comment 6•9 years ago
|
||
Good thing that nobody was calling LMemoryBarrier::mir(), since LMemoryBarrier is emitted for all sorts of nodes...
Assignee | ||
Comment 7•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/859f435f2ca001acc659cf47a2068fc94287e84a
Bug 1225028 - remove Atomics.fence. r=bbouvier
Comment 8•9 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in
before you can comment on or make changes to this bug.
Description
•