More ArrayCmpxchg expected opts in Heap2Local#8567
Conversation
When non-escaping allocations flow into the `expected` field of an ArrayCmpxchg, optimize even if the ArrayCmpxchg has a non-constant index. We typically only optimize array instructions with constant fields, but that's because we need to know what field of the array is accessed. Arrays flowing into the `expected` field are not accessed, though, so there is no need for the accessed index to be constant. Make sure that effects in the potentially non-constant index field are preserved in the correct order by using a new scratch local to propagate the index value past the `expected` and `replacement` expressions to the newly generated `struct.atomic.get`.
| return; | ||
| } | ||
| if (curr->ref == child || curr->expected == child) { | ||
| if (child == curr->expected || |
There was a problem hiding this comment.
Worth a comment here I think, as in the PR description.
src/passes/Heap2Local.cpp
Outdated
| return; | ||
| } | ||
|
|
||
| // The accessed array is being optimzied. Convert the ArrayCmpxchg into a |
There was a problem hiding this comment.
| // The accessed array is being optimzied. Convert the ArrayCmpxchg into a | |
| // The accessed array is being optimized. Convert the ArrayCmpxchg into a |
(pre-existing, but while we are here)
| // The `expected` value must be getting optimized. Since the accessed object | ||
| // is remaining an array for now, do not change anything. The ArrayCmpxchg | ||
| // will be optimized later by Struct2Local. | ||
| return; |
There was a problem hiding this comment.
IIUC this moves the trapping case into the handling of ref. But the case of expected should still trap, I think? I don't see that handled in the code that is reached later?
There was a problem hiding this comment.
Since the optimization for expected creates a new array.atomic.get, it will still trap on OOB access precisely when the original code would, no matter what the expression for the index looks like.
| (call $effect-i32) | ||
| (array.new_default $array (i32.const 1)) | ||
| (call $effect-eq) | ||
| ) |
There was a problem hiding this comment.
Is there a test for array.atomic.rmw.cmpxchg where we optimize the expected field but the index is out of bounds?
There was a problem hiding this comment.
The output will look the same, but I will add a test where the index is an OOB constant.
2 participants
When non-escaping allocations flow into the
expectedfield of an ArrayCmpxchg, optimize even if the ArrayCmpxchg has a non-constant index. We typically only optimize array instructions with constant fields, but that's because we need to know what field of the array is accessed. Arrays flowing into theexpectedfield are not accessed, though, so there is no need for the accessed index to be constant.Make sure that effects in the potentially non-constant index field are preserved in the correct order by using a new scratch local to propagate the index value past the
expectedandreplacementexpressions to the newly generatedstruct.atomic.get.