Fix areConsecutiveInputsEqual for idempotent calls#8804
Conversation
OptimizeInstruction's `areConsecutiveInputsEqual` function finds the effects that are executed in between the left-hand and right-hand expressions and checks that they cannot affect the value of the right-hand expression or its operands. However, it did not previously handle the case where the left-hand expression itself changes the value of the operands flowing into the right-hand expression. This can happen in the case of calls to idempotent functions, where the first call changes a global that is passed as an argument to the second call.
| (call $import) | ||
| ) | ||
|
|
||
|
|
There was a problem hiding this comment.
Is this intentional? We have only one blank line between other functions, so
| for (auto* child : ChildIterator(right)) { | ||
| rightEffects.walk(child); | ||
| } | ||
| ShallowEffectAnalyzer leftEffects(getPassOptions(), *getModule(), left); |
There was a problem hiding this comment.
Why do we use ShallowEffectAnalyzer?
There was a problem hiding this comment.
All of the children of the left expression are executed before the left expression itself as well as the right expression, so their side effects affect both equally and cannot cause any differences. (There may be other interfering side effects from other sources, but we already check those below.)
There was a problem hiding this comment.
so their side effects affect both equally and cannot cause any differences.
What are the "both" here? Cannot cause any differences between what? Can you give an example?
There was a problem hiding this comment.
The side effects of the children of the lef-hand expression affect both the left-hand expression and the right-hand expression equally and cannot cause any differences in the values that they receive or produce. Here are examples:
If these are the left-hand and right-hand sides:
(call $some-idempotent-func
(global.get $g-mut)
)
(call $some-idempotent-func
(global.get $g-mut)
)
Then we have to assume that the left-hand call might change the value of the global and therefore the right-hand call might have a different input and therefore produce a different value. But the left-hand global.get doesn't matter and we can ignore it.
If the operand has write effects:
(call $some-idempotent-func
(block (result i32)
(global.set $g-mut (i32.const 0))
(i32.const 0)
)
)
(call $some-idempotent-func
(block (result i32)
(global.set $g-mut (i32.const 0))
(i32.const 0)
)
)
Then it looks like the left-hand global.set could affect the value of the right-hand call, but that doesn't actually matter because it would also affect the value of the left-hand call the same way. So we want to ignore the left-hand global.set. However, we still wouldn't optimize in this case because the left-hand call might write the global, so we have a write-write conflict between the left-hand call and the right-hand children that makes leftEffects.orderedBefore(rightEffects) true. We could do better in this case if we could ignore write-write conflicts.
There was a problem hiding this comment.
What if LHS is stateful? For example, LHS (and RHS) can be incrementing a global.
(module
(global $g (mut f32) (f32.const -3))
(func $test (result f32)
(f32.abs
(f32.mul
;; LHS
(f32.neg
(block (result f32)
(global.set $g (f32.add (global.get $g) (f32.const 2)))
(global.get $g)
)
)
;; RHS
(f32.neg
(block (result f32)
(global.set $g (f32.add (global.get $g) (f32.const 2)))
(global.get $g)
)
)
)
)
)
)Here f32.abs should not be optimized out, no?
There was a problem hiding this comment.
Yikes! You're right. Thanks for the example.
OptimizeInstruction's
areConsecutiveInputsEqualfunction finds the effects that are executed in between the left-hand and right-hand expressions and checks that they cannot affect the value of the right-hand expression or its operands. However, it did not previously handle the case where the left-hand expression itself changes the value of the operands flowing into the right-hand expression. This can happen in the case of calls to idempotent functions, where the first call changes a global that is passed as an argument to the second call.