Closed Bug 1225028 Opened 5 years ago Closed 4 years ago

Remove support for Atomics.fence

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: lth, Assigned: lth)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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.
Also in asm.js, of course.
Jukka says our JS code is no longer using Atomics.fence, so we can just remove it from the engine.
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: nobody → lhansen
Status: NEW → ASSIGNED
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+
Excellent call on MMemoryBarrier, I will investigate further.  Thanks.
Good thing that nobody was calling LMemoryBarrier::mir(), since LMemoryBarrier is emitted for all sorts of nodes...
https://hg.mozilla.org/mozilla-central/rev/859f435f2ca0
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.