search

riscv / riscv-zalasr

The ISA specification for the Zalasr extension.
Creative Commons Attribution 4.0 International
1 stars 1 forks source link

Atomicity of load-acquire and store-release #1

Open mehnadnerd opened 1 year ago

mehnadnerd commented 1 year ago

I currently specify that load-acquire and store-release are atomic. I think that is a property that should continue to be enforced, but if there was a strong reason not to I wanted to hear it. In addition, this messes with the planned relaxation of the unordered version to a plain load. Upon further reflection, I think that this change is probably good, it provides a way to write an unambigously atomic load/store, even if alignment is unknown (but it does that by faulting if it is nonatomic). Will still need a new mnemoic for the unordered version though.

mehnadnerd commented 1 year ago

The alternative is to just ban the version with neither set, which is probably the easiest way forward. I have done that in https://github.com/mehnadnerd/riscv-zalasr/commit/a2c8b88005493c9b2b00d976baa00ab712ef9966 for now.

hboehm commented 1 year ago

For purposes of discussion, let's focus on the load case here, remembering that there is always a corresponding store issue. I agree that disallowing the version in which the acquire bit is not set is the easiest way forward. In my view, the version with just aq set is orders of magnitude more useful than anything else. I would remove "For maximum compatibility, it is recommended that implementations either support the variant with only rl set or treat it as equivalent to both aq and rl being set." I would leave the non-acquire variants as really reserved, with no hint as to what they might mean, so that we keep the freedom to assign different semantics in the future. IMO, recommending an implementation that is not guaranteed to work really gets in the way of portability. We should either require it, or imply that trapping implementations are encouraged.

mehnadnerd commented 1 year ago

I've removed that text. Do you think they atomicity requirement is what we should stick with, or should we change it to something else? I like that it directly states its requirements.