LocalCSE: Migrate from invalidates to orderedBefore#8744
Conversation
Replace the coarse `invalidates` check and the coarse global state check in LICM with more precise `orderedBefore` checks. This allows LICM to move memory accesses past release stores, while still correctly blocking them from moving past acquire loads. Add a lit test to verify the asymmetrical reordering behavior with release/acquire atomics on shared memory/GC structs.
Replace the coarse `invalidates` check in `LocalCSE` with `orderedBefore`. This allows `LocalCSE` to reuse expression values across release stores, while still correctly blocking reuse across acquire loads. Add a lit test to verify the asymmetrical reordering behavior with release/acquire atomics on shared GC structs and Wasm memory.
| } | ||
| auto& originalInfo = kv.second; | ||
| if (effects.invalidates(originalInfo.effects)) { | ||
| if (effects.orderedBefore(originalInfo.effects)) { |
There was a problem hiding this comment.
We have [original, curr] as the order of expressions. Shouldn't we invalidate like this?
| if (effects.orderedBefore(originalInfo.effects)) { | |
| if (!effects.orderedAfter(originalInfo.effects)) { |
There was a problem hiding this comment.
i.e. effects/curr begin after. Should we not ask if they must remain after, as before/after is not symmetric?
There was a problem hiding this comment.
No, this is correct-as is. Say we have A; B; A', where A and A' is the same, so CSE is trying to optimize out A' and use the result of A instead. This is semantically the same as reordering A' to be before B (or alternatively B after A'): A; A'; B, so we want to invalidate the optimization if B ordered-before A', preventing the move. You're right that originalInfo.effects is calculated from A, but A' is the same expression, so here we're using it as A''s effects. So checking B ordered-before A' is the same as effects.orderedBefore(originalInfo.effects).
There was a problem hiding this comment.
I see, thanks. Maybe add a comment? Specifically the bit about using the effects of original to model those of the thing being optimized away, which is later, seems non-obvious.
There was a problem hiding this comment.
I've added a comment clarifying that originalInfo.effects is standing in for COPY's effects.
3 participants
Replace the coarse
invalidatescheck inLocalCSEwithorderedBefore.This allows
LocalCSEto reuse expression values across release stores,while still correctly blocking reuse across acquire loads.
Add a lit test to verify the asymmetrical reordering behavior with
release/acquire atomics on shared GC structs and Wasm memory.