Remove support for Atomics.fence

RESOLVED FIXED in Firefox 48

Status

()

defect
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: lth, Assigned: lth)

Tracking

(Blocks 1 bug)

unspecified
mozilla48
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox48 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
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

4 years ago
Also in asm.js, of course.
(Assignee)

Comment 2

3 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

3 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

3 years ago
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+
(Assignee)

Comment 5

3 years ago
Excellent call on MMemoryBarrier, I will investigate further.  Thanks.
(Assignee)

Comment 6

3 years ago
Good thing that nobody was calling LMemoryBarrier::mir(), since LMemoryBarrier is emitted for all sorts of nodes...

Comment 8

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/859f435f2ca0
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.