Skip to content

Fix areConsecutiveInputsEqual for idempotent calls #8804

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tlively merged 3 commits into from
Jun 5, 2026
+51 −3
Merged

Fix areConsecutiveInputsEqual for idempotent calls #8804

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b95d478
Fix areConsecutiveInputsEqual for idempotent calls
tlively Jun 5, 2026
bba9feb
Merge branch 'main' into consecutive-inputs-equal-left-right-interfer…
tlively Jun 5, 2026
d8bd578
remove stray newline
tlively Jun 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 18 additions & 3 deletions src/passes/OptimizeInstructions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2867,9 +2867,24 @@ struct OptimizeInstructions
return false;
}

// They do look the same! Make sure nothing executed in between them can
// affect the value of `right` and make it different from `left`.
if (interferingEffects.orderedBefore(effects(right))) {
// They do look the same! Determine whether the left hand expression can
// change the value of the operands flowing into the right-hand expression.
// Do not consider the right-hand expression itself here because the
// expressions might be idempotent function calls and we might otherwise
// mistakenly conclude that the first call interferes with the second.
EffectAnalyzer rightEffects(getPassOptions(), *getModule());
for (auto* child : ChildIterator(right)) {
rightEffects.walk(child);
}
ShallowEffectAnalyzer leftEffects(getPassOptions(), *getModule(), left);
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.

Why do we use ShallowEffectAnalyzer?

Copy link
Copy Markdown
Member Author

@tlively tlively Jun 5, 2026

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

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

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

@tlively tlively Jun 5, 2026

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 •
edited
Loading

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

@tlively tlively Jun 5, 2026

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

@tlively tlively Jun 5, 2026

Choose a reason for hiding this comment

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

Fix is in #8808

if (leftEffects.orderedBefore(rightEffects)) {
return false;
}

// Make sure nothing executed in between the expressions can affect the
// value of `right` (or its operands) and make it different from `left`.
rightEffects.visit(right);
if (interferingEffects.orderedBefore(rightEffects)) {
return false;
}

Expand Down
33 changes: 33 additions & 0 deletions test/lit/passes/optimize-instructions_idempotent.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,20 @@
;; CHECK: (import "a" "b" (func $import (type $2) (result f32)))
(import "a" "b" (func $import (result f32)))

;; CHECK: (global $g (mut f32) (f32.const 1))
(global $g (mut f32) (f32.const 1))

;; CHECK: (@binaryen.idempotent)
;; CHECK-NEXT: (func $idempotent (type $0) (param $x f32) (result f32)
;; CHECK-NEXT: (global.set $g
;; CHECK-NEXT: (f32.const -1)
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.get $x)
;; CHECK-NEXT: )
(@binaryen.idempotent)
(func $idempotent (param $x f32) (result f32)
;; This function is idempotent: same inputs, same outputs.
(global.set $g (f32.const -1))
(local.get $x)
)

Expand Down Expand Up @@ -61,6 +68,18 @@
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (f32.abs
;; CHECK-NEXT: (f32.mul
;; CHECK-NEXT: (call $idempotent
;; CHECK-NEXT: (global.get $g)
;; CHECK-NEXT: )
;; CHECK-NEXT: (call $idempotent
;; CHECK-NEXT: (global.get $g)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
(func $test-abs
;; These calls are identical, since the second returns the same. We can
Expand Down Expand Up @@ -103,6 +122,20 @@
)
)
)
;; Here the arguments look the same, but the first call will change the
;; value of the argument to the second call, so we cannot optimize.
(drop
(f32.abs
(f32.mul
(call $idempotent
(global.get $g)
)
(call $idempotent
(global.get $g)
)
)
)
)
)

;; CHECK: (func $test-abs-calls (type $1)
Expand Down
Loading