Skip to content

Fix areConsecutiveInputsEqual for idempotent calls#8804

Merged
tlively merged 3 commits into
mainfrom
consecutive-inputs-equal-left-right-interference
Jun 5, 2026
Merged

Fix areConsecutiveInputsEqual for idempotent calls#8804
tlively merged 3 commits into
mainfrom
consecutive-inputs-equal-left-right-interference

Conversation

@tlively
Copy link
Copy Markdown
Member

@tlively tlively commented Jun 5, 2026

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.

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.
@tlively tlively requested a review from a team as a code owner June 5, 2026 04:14
@tlively tlively requested review from aheejin and removed request for a team June 5, 2026 04:14
(call $import)
)


Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

Is this intentional? We have only one blank line between other functions, so

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not intentional, thanks.

for (auto* child : ChildIterator(right)) {
rightEffects.walk(child);
}
ShallowEffectAnalyzer leftEffects(getPassOptions(), *getModule(), left);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we use ShallowEffectAnalyzer?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member

@aheejin aheejin Jun 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yikes! You're right. Thanks for the example.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix is in #8808

@tlively tlively enabled auto-merge (squash) June 5, 2026 17:05
@tlively tlively merged commit fb29c73 into main Jun 5, 2026
16 checks passed
@tlively tlively deleted the consecutive-inputs-equal-left-right-interference branch June 5, 2026 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants