WebAssembly / shared-everything-threads

A draft proposal for spawning threads in WebAssembly
Other
34 stars 1 forks source link

Update memory ordering encodings #67

Closed tlively closed 2 months ago

tlively commented 3 months ago

Add separate release and acquire encodings since an acquire-release fence is not the same as an acquire fence. Encode all orderings as u8 instead of u32 and in the case of memory instructions, use a bit in the memarg to signal the presence of a separate ordering immediate rather than encoding the ordering directly in the memarg. For RMW ops, pack both a read ordering and a write ordering into the single u8 ordering immediate.

This design is similar to the one described in https://github.com/WebAssembly/shared-everything-threads/issues/60#issuecomment-2136475267, but uses release and acquire instead of release-acquire for load and store orderings, using release-acquire only for fences.

Fixes #59. Fixes #60.

tlively commented 2 months ago

ping @conrad-watt

rossberg commented 2 months ago

I feel uncomfortable about introducing this over-generalisation and added complexity of 4 orderings at this point already. IIUC, the distinction isn't relevant right now, and we don't know if and how we ever want to introduce a more relaxed ordering where it would be. So I'd rather not go into over-engineering mode.

tlively commented 2 months ago

We could exploit the fact that reads never use release ordering and writes never use acquire ordering to collapse this back down to just two orderings: seqcst and acqrel. The only thing we'd lose by doing that is the ability to have release or acquire fences that are not full acqrel fences (and to be honest, I don't quite understand the distinction yet). We could always add separate acquire and release orderings in the future to recover this expressivity if we wanted to. Would you prefer to go this route, @rossberg?

The other changes here is to encode two ordering immediates (packed into one u8) for RMW ops and to pull the ordering immediate out of the memarg to give us more future extensibility. Are you fine with those, @rossberg?

@conrad-watt, I'm fine making late edits to cmpxchg if necessary. I'll make sure to get a note about that possibility into the README.

rossberg commented 2 months ago

Would you prefer to go this route, @rossberg?

Yes, that's what I was shooting at.

The other changes here is to encode two ordering immediates (packed into one u8) for RMW ops and to pull the ordering immediate out of the memarg to give us more future extensibility. Are you fine with those, @rossberg?

Would that mean that this is just a future-proof binary encoding choice, i.e., we don't actually consider bit patterns expressing mixed orderings well-formed for now? If so, then yes, that seems fine to me.

tlively commented 2 months ago

PTAL at the updates in the last commit.