Add a "relaxed" memory order - #8984
Conversation
The memory order is not the C++ relaxed memory order, but rather the slightly stronger load-store ordered memory order, which prevents out-of-thin-air values by forbidding earlier atomic loads from being reordered past later atomic stores. This makes it impossible to construct the self-justifying causality cycle that gives rise to out-of-thin-air values. The name of the memory order in both the implementation and the text format is likely to change in the future to become more descriptive.
| } | ||
| // Shared location accesses cannot be reordered after (but may be able to be | ||
| // reordered before) release stores. | ||
| if (other.writeOrder >= MemoryOrder::AcqRel && |
There was a problem hiding this comment.
Looks like this was a bug before the MemoryOrder enum ordering changed in this PR? Ditto for line 381. Currently before this PR AcqRel > SeqCst.
There was a problem hiding this comment.
Yeah, thankfully I think the bug was harmless because we fell through to the explicit handling of MemoryOrder::SeqCst below, which still gave us the correct behavior.
stevenfontanella
left a comment
There was a problem hiding this comment.
Looks like the atomics.js test fails on the emscripten CI, is that related to this change? And second, do we plan to add this to the fuzzer and C + JS APIs in a future PR?
|
Yeah, this PR had attempted to add support to the JS API, but 1) the syntax was wrong and 2) support was missing in the C API. Both are fixed in the last commit. |
The memory order is not the C++ relaxed memory order, but rather the slightly stronger load-store ordered memory order, which prevents out-of-thin-air values by forbidding earlier atomic loads from being reordered past later atomic stores. This makes it impossible to construct the self-justifying causality cycle that gives rise to out-of-thin-air values. The name of the memory order in both the implementation and the text format is likely to change in the future to become more descriptive.