Skip to content

Use deep effects in areConsecutiveInputsEqual #8808

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 2 commits into from
Jun 6, 2026
+112 −15
Merged

Use deep effects in areConsecutiveInputsEqual #8808

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
072b3b7
Use deep effects in areConsecutiveInputsEqual
tlively Jun 5, 2026
0140edf
more comments
tlively Jun 6, 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
34 changes: 20 additions & 14 deletions src/passes/OptimizeInstructions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2873,24 +2873,30 @@ struct OptimizeInstructions
return false;
}

// 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());
// They do look the same! Determine whether there are side effects that
// could make the values different. Note that we use the same
// EffectAnalyzers for effects from the left and right sides because they
// are the same.
EffectAnalyzer fallthroughChildEffects(getPassOptions(), *getModule());
for (auto* child : ChildIterator(right)) {
rightEffects.walk(child);
}
ShallowEffectAnalyzer leftEffects(getPassOptions(), *getModule(), left);
if (leftEffects.orderedBefore(rightEffects)) {
fallthroughChildEffects.walk(child);
}
EffectAnalyzer fallthroughEffects = fallthroughChildEffects;
fallthroughEffects.visit(right);

// Does the left-hand parent expression or side effects in the right-hand
// children affect the values flowing into the right-hand parent expression?
Comment on lines +2887 to +2888

@aheejin aheejin Jun 5, 2026

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.

This sounds a little confusing to me. What does "left-hand parent expression or side effects in the right-hand children" mean? I think what we want to ensure is LHS parent + LHS children should not affect RHS children, right?

Also even though technically LHS and RHS are the same here, it would be nice to comment that, because we just computed RHS effects above and are saying LHS effects here.

@tlively tlively Jun 6, 2026

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.

This sounds a little confusing to me. What does "left-hand parent expression or side effects in the right-hand children" mean? I think what we want to ensure is LHS parent + LHS children should not affect RHS children, right?

But it's not the execution LHS children that causes the RHS to have a different value from the LHS, it's the execution of the RHS children (or possibly the LHS parent expression).

@aheejin aheejin Jun 6, 2026

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.

What I meant by the LHS children were the parameters for the LHS function, and LHS parent was the LHS function execution itself. Did you mean that too?

Then, wasn't it the LHS children's execution (which increased the global's value) that causes RHS's parameters (childrens) to have different values? Maybe we are talking about the same thing with a slightly different terms.

@tlively tlively Jun 6, 2026

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.

What I meant by the LHS children were the parameters for the LHS function, and LHS parent was the LHS function execution itself. Did you mean that too

Yes 👍

Then, wasn't it the LHS children's execution (which increased the global's value) that causes RHS's parameters (childrens) to have different values? Maybe we are talking about the same thing with a slightly different terms.

No, because the LHS children execute before the LHS function call, so their side effects are already visible in the final LHS value. It's those same child expressions executing on the RHS before the RHS function call that make the RHS value different from the LHS value.

// Do not consider the right-hand parent expression because it could be an
// idempotent call and we don't want to mistakenly conclude that the
// left-hand side could interfere with it. (Non-idempotent functions will
// be rejected by the `isGenerative` check below.)
if (fallthroughEffects.orderedBefore(fallthroughChildEffects)) {
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)) {
// Do any effects executed between the expressions affect the right-hand
// expression?
if (interferingEffects.orderedBefore(fallthroughEffects)) {
return false;
}

Expand Down
41 changes: 40 additions & 1 deletion test/lit/passes/optimize-instructions-global-effects-idempotent.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
;; NOTE: Assertions have been generated by update_lit_checks.py and should not be edited.
;; RUN: wasm-opt --generate-global-effects --optimize-instructions -all -S -o - %s | filecheck %s
;; RUN: foreach %s %t wasm-opt --generate-global-effects --optimize-instructions -all -S -o - \
;; RUN: | filecheck %s

(module
;; CHECK: (type $struct (shared (struct (field (mut i32)))))
Expand Down Expand Up @@ -91,3 +92,41 @@
)
)
)

(module
;; CHECK: (global $f (mut f32) (f32.const -3))
(global $f (mut f32) (f32.const -3))

;; CHECK: (func $potent (type $0) (result f32)
;; CHECK-NEXT: (global.set $f
;; CHECK-NEXT: (f32.add
;; CHECK-NEXT: (global.get $f)
;; CHECK-NEXT: (f32.const 2)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (global.get $f)
;; CHECK-NEXT: )
(func $potent (result f32)
(global.set $f (f32.add (global.get $f) (f32.const 2)))
(global.get $f)
)

;; CHECK: (func $function-effects-interfere (type $0) (result f32)
;; CHECK-NEXT: (f32.abs
;; CHECK-NEXT: (f32.mul
;; CHECK-NEXT: (call $potent)
;; CHECK-NEXT: (call $potent)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
(func $function-effects-interfere (result f32)

@aheejin aheejin Jun 5, 2026

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.

Maybe some comments on why this shouldn't be optimized?

@tlively tlively Jun 6, 2026

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.

Done.

;; Even knowing the effects of the call, we should not be able to optimize
;; here because the calls will return different values.
(f32.abs
(f32.mul
(call $potent)
(call $potent)
)
)
)
)
52 changes: 52 additions & 0 deletions test/lit/passes/optimize-instructions-mvp.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@

(memory 0)

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

;; CHECK: (func $and-and (param $i1 i32) (result i32)
;; CHECK-NEXT: (i32.and
;; CHECK-NEXT: (local.get $i1)
Expand Down Expand Up @@ -15757,6 +15760,55 @@
)
))
)
;; CHECK: (func $optimize-float-points-stateful (result f32)
;; CHECK-NEXT: (f32.abs
;; CHECK-NEXT: (f32.mul
;; CHECK-NEXT: (f32.neg
;; CHECK-NEXT: (block (result f32)
;; CHECK-NEXT: (global.set $g-f32
;; CHECK-NEXT: (f32.add
;; CHECK-NEXT: (global.get $g-f32)
;; CHECK-NEXT: (f32.const 2)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (global.get $g-f32)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (f32.neg
;; CHECK-NEXT: (block (result f32)
;; CHECK-NEXT: (global.set $g-f32
;; CHECK-NEXT: (f32.add
;; CHECK-NEXT: (global.get $g-f32)
;; CHECK-NEXT: (f32.const 2)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (global.get $g-f32)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
(func $optimize-float-points-stateful (result f32)
(f32.abs
(f32.mul
;; LHS
(f32.neg
(block (result f32)
(global.set $g-f32 (f32.add (global.get $g-f32) (f32.const 2)))
(global.get $g-f32)
)
)
;; RHS - The increment of the global causes this to produce a different
;; result than the LHS, so we cannot optimize out the f32.abs.
(f32.neg
(block (result f32)
(global.set $g-f32 (f32.add (global.get $g-f32) (f32.const 2)))
(global.get $g-f32)
)
)
)
)
)
;; CHECK: (func $ternary (param $x i32) (param $y i32)
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (i32.eqz
Expand Down
Loading