Open Bug 1423491 Opened 6 years ago Updated 2 years ago

Abstract JIT & MASM over memory order parameter

Categories

(Core :: JavaScript Engine: JIT, enhancement, P3)

enhancement

Tracking

()

People

(Reporter: lth, Unassigned)

References

Details

Currently the memory order of the atomic operations is partly baked into the operation, partly not:

- Load and store operations take not a memory order parameter but a set of
  barrier flags.  This happened because atomic load and store reused existing
  load and store code.

- Other atomic operations assume seq_cst everywhere.  This happened because the
  code was new and is not shared with non-atomic ops, and JS only has seq_cst
  atomics.

Wasm will certainly move in the direction where we add more memory orders, even for MVP we've been discussion (on and off) acq_rel, and we'd want to accomodate that.  Also, the atomic ops that have the memory order baked in don't allow for useful optimizations, notably redundant-barrier removal.

So we'll want to parameterize the new atomic ops with some notion of memory order, at a minimum.  It could be the same as for load and store: barrier flags.

But another question is whether the barrier flags are the correct expression of the memory order.  They do allow for optimizations to be implemented, but do they manage to transmit the semantics correctly in the presence of optimizations?  And are they sufficiently platform independent?
Bug 1425149 sets this up by introducing a notion of a synchronization spec, and parameterizing all the atomic operations on that spec, and hiding the internals of the spec somewhat.

We still need to fix LMemoryBarrier, which we can split into LMemoryBarrierBefore and LMemoryBarrierAfter, to mirror the new masm.memoryBarrierBefore() and masm.memoryBarrierAfter(); the new LIR nodes would then carry the synchronization spec, not barrier bits.

We also need to lift the introduction of the synchronization specs many places.  For example, search for Synchronization::Full() in the jit directory, and you'll see that it is now introduced in the CodeGenerators, which improves on the previous case (MacroAssembler), but is not what we want - we want this to come from the MIR generator level, based on information about the bytecode/source language semantics.  Also search for Synchronization::Load/Store/None.

Finally, the Full/Load/Store/None specs should really carry semantics about the memory order (could be parameters to those operations to generate appropriate barrier bits), but we should fix that after we have lifted the introduction of those specs to the appropriate level.
Depends on: 1425149
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.