Skip to content

Skip folding branches of unreachable If#8724

Merged
tlively merged 4 commits into
mainfrom
no-hoist-unreachable-if
May 20, 2026
Merged

Skip folding branches of unreachable If#8724
tlively merged 4 commits into
mainfrom
no-hoist-unreachable-if

Conversation

@tlively
Copy link
Copy Markdown
Member

@tlively tlively commented May 18, 2026

OptimizeInstructions generally skips optimizing unreachable code, leaving that to DCE instead. But it did not have this check before folding identical instructions out of If arms. The fuzzer found a case where cont.new instructions were hoisted out of an unreachable If but not refinalized, resulting in the concretely-typed cont.new having an unreachable child. This caused an assertion failure in the validator.

Fix the root problem by skipping this optimization for unreachable Ifs, but also fix the validator so that it will no longer crash on such invalid IR.

OptimizeInstructions generally skips optimizing unreachable code, leaving that to DCE instead. But it did not have this check before folding identical instructions out of If arms. The fuzzer found a case where cont.new instructions were hoisted out of an unreachable If but not refinalized, resulting in the concretely-typed cont.new having an unreachable child. This caused an assertion failure in the validator.

Fix the root problem by skipping this optimization for unreachable Ifs, but also fix the validator so that it will no longer crash on such invalid IR.
@tlively tlively requested a review from a team as a code owner May 18, 2026 22:41
@tlively tlively requested review from stevenfontanella and removed request for a team May 18, 2026 22:41
@stevenfontanella
Copy link
Copy Markdown
Member

The tests don't mention cont.new, what's the case that the fuzzer found? Should we check that in?

@tlively
Copy link
Copy Markdown
Member Author

tlively commented May 19, 2026

The fuzzer's test case looks like this:

(if (unreachable)
  (then (cont.new $k (ref.func $a))
  (else (cont.new $k (ref.func $b))
)

It was previously optimized to this:

(cont.new $k ;; should have type `unreachable`, but does not.
  (if (unreachable)
    (then (ref.func $a))
    (else (ref.func $b))
  )
)

We could add this as a test, but the fix really had nothing to do with cont.new in particular. I suppose I could add a unit test for the cont.new validation fix, though.

@tlively tlively enabled auto-merge (squash) May 20, 2026 01:19
@tlively tlively merged commit c82a630 into main May 20, 2026
12 of 16 checks passed
@tlively tlively deleted the no-hoist-unreachable-if branch May 20, 2026 01:43
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