Skip to content

OptimizeInstructions: Improve areConsecutiveInputsEqualAndFoldable#8355

Merged
kripken merged 2 commits intoWebAssembly:mainfrom
kripken:oi.fold.no.remove
Feb 23, 2026
Merged

OptimizeInstructions: Improve areConsecutiveInputsEqualAndFoldable#8355
kripken merged 2 commits intoWebAssembly:mainfrom
kripken:oi.fold.no.remove

Conversation

@kripken
Copy link
Member

@kripken kripken commented Feb 20, 2026

To check if we can fold A, B into A, we can just check if B has
effects we can't remove. The old code used a utility that checks if
A is also removable, but that is suboptimal, and that entire
utility is not needed.

@kripken kripken requested a review from tlively February 20, 2026 18:56
@kripken kripken mentioned this pull request Feb 20, 2026
Copy link
Member

@tlively tlively left a comment

Choose a reason for hiding this comment

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

LGTM if the fuzzer is happy.

@kripken kripken merged commit 9656b8e into WebAssembly:main Feb 23, 2026
17 checks passed
@kripken kripken deleted the oi.fold.no.remove branch February 23, 2026 21:24
@tlively
Copy link
Member

tlively commented Feb 28, 2026

Fuzzer is not happy! Bisected the following to this change:

;; bin/wasm-opt -all w.wast --print --optimize-instructions -S -o - --fuzz-exec
(module
 (import "fuzzing-support" "log-f32" (func $import))
 (func $test (export "test") (param $2 i32)
  (if
   (select
    (local.tee $2
     (i32.eqz
      (local.get $2)
     )
    )
    (i32.const 0)
    (i32.eqz
     (local.get $2)
    )
   )
   (then
    (call $import)
   )
  )
 )
)

@kripken
Copy link
Member Author

kripken commented Mar 4, 2026

Yikes, while this PR's changes were right, it uncovered a bug in the code it calls...

Fix in #8415

kripken added a commit that referenced this pull request Mar 4, 2026
…d one reads (#8415)

`areConsecutiveInputsEqual` looks at fallthrough values when comparing,
but it must look at the original values for effects. It did that for the
right side, but not the left.

This was incorrect for a very long time, and only uncovered by

#8355 (comment)

I suspect it might have even been undiscoverable until then, even though
it was logically false (in that nothing reached it that could hit the bug).
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.

2 participants