[Wasm RyuJit] avoid reverse ops#124189
Conversation
For Wasm we generally want operands evaluated in the normal order. So suppress creating nodes with GT_REVERSE_OPS. On Wasm reversal is still possible for relops (where we can change the oper) and for commutative ops (where we can swap the operands). We may want to enable reversal in other cases where there is a big imbalance in child tree complexity, but then we'll need to add changes to lower to spill the out of order tree to a temp to handle this.
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
There was a problem hiding this comment.
Pull request overview
This PR adjusts JIT tree evaluation-order heuristics for the WebAssembly target to prefer normal (non-reversed) operand evaluation order, avoiding use of GTF_REVERSE_OPS except where reversal can be expressed via operand swapping (commutative ops) or operator swapping (relops).
Changes:
- Disable reversal for indirect stores on WASM by forcing
allowReversal = falseinSetIndirectStoreEvalOrder. - On WASM, suppress setting
GTF_REVERSE_OPSin the “can’t commute” default swap path in bothgtSetEvalOrderandgtSetEvalOrderMinOpts.
|
fyi @dotnet/jit-contrib Fixes some "IR not in stack order" asserts that we hit after lowering a reversed The remaining instances of the "IR not in stack order" assert mostly seem to come from call lowering now, as best I can tell. |
SingleAccretion
left a comment
There was a problem hiding this comment.
You can also consider adding a condition into OperSupportsReverseOpEvalOrder.
Good idea. |
|
Various errors, all unrelated as this change is currently untestable in CI. One looks like #123912. Another is infra... |
|
/ba-g unrelated errors |
|
/ba-g retry |
For Wasm we generally want operands evaluated in the normal order. So, suppress marking nodes with GT_REVERSE_OPS. On Wasm reversal is still possible for relops (where we can change the oper) and for commutative ops (where we can swap the operands).
We may want to enable reversal in other cases where there is a big imbalance in child tree complexity, but then we'll need to add changes to lower to spill the out of order tree to a temp to handle this.