Skip to content

Handle irreducible control flow in branch simplification#13391

Merged
cfallin merged 3 commits into
bytecodealliance:mainfrom
fitzgen:issue-13365-branch-optimizations-bug-take-2
May 18, 2026
Merged

Handle irreducible control flow in branch simplification#13391
cfallin merged 3 commits into
bytecodealliance:mainfrom
fitzgen:issue-13365-branch-optimizations-bug-take-2

Conversation

@fitzgen
Copy link
Copy Markdown
Member

@fitzgen fitzgen commented May 15, 2026

We previously removed a block from the CFG if it was not marked as reachable by the time the egraph pass visited it. The pass's traversal is both (1) a depth-first pre-order dominator traversal, and (2) a reverse post-order CFG traversal. This traversal visits all of a block's non-back edge predecessors before visiting the block itself. For reducible control flow, this is all that is necessary because we've already visited every back edge's target block already.

However, for irreducible control flow, blocks can be reachable only through back edges, and so the traversal's property alone was not sufficient. (The EgraphBlockIter's proof is still correct, at least to the best of my knowledge since we haven't mechanically proven it, but the implicit assumption that its proven property is sufficient for our reachability-based block removal is incorrect in the face of irreducible control flow.)

This commit's fix is to stop removing newly-unreachable blocks in the middle of the egraphs pass, and instead simply call eliminate_unreachable_code after the egraph pass completes to remove any code that became unreachable after branch simplification.

Fixes #13365

We previously removed a block from the CFG if it was not marked as reachable by
the time the egraph pass visited it. The pass's traversal is both (1) a
depth-first pre-order dominator traversal, and (2) a reverse post-order CFG
traversal. This traversal visits all of a block's non-back edge predecessors
before visiting the block itself. For reducible control flow, this is all that
is necessary because we've already visited every back edge's target block
already.

However, for irreducible control flow, blocks can be reachable *only* through
back edges, and so the traversal's property alone was not sufficient. (The
`EgraphBlockIter`'s proof is still correct, at least to the best of my knowledge
since we haven't mechanically proven it, but the implicit assumption that its
proven property is sufficient for our reachability-based block removal is
incorrect in the face of irreducible control flow.)

This commit's fix is to stop removing newly-unreachable blocks in the middle of
the egraphs pass, and instead simply call `eliminate_unreachable_code` after the
egraph pass completes to remove any code that became unreachable after branch
simplification.

Fixes bytecodealliance#13365
@fitzgen fitzgen requested review from a team as code owners May 15, 2026 20:55
@fitzgen fitzgen requested review from cfallin and removed request for a team May 15, 2026 20:55
Copy link
Copy Markdown
Member

@cfallin cfallin left a comment

Choose a reason for hiding this comment

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

Thanks. I think we'll need to change the design here: this is making elaboration more complex (to be tolerant of unreachable code) and then doing DCE; I would have expected us not to elaborate unreachable blocks, instead.

Comment thread cranelift/codegen/src/egraph/elaborate.rs Outdated
Comment thread cranelift/codegen/src/egraph/elaborate.rs Outdated
Comment thread cranelift/codegen/src/egraph/elaborate.rs Outdated
@github-actions github-actions Bot added the cranelift Issues related to the Cranelift code generator label May 16, 2026
@fitzgen
Copy link
Copy Markdown
Member Author

fitzgen commented May 18, 2026

@cfallin I updated the PR to call eliminate_unreachable_code in the middle of the egraph pass, after optimizing and removing instructions, but before elaboration. Probably easiest to look at the complete diff, rather than per-commit.

@fitzgen fitzgen requested a review from cfallin May 18, 2026 17:58
Copy link
Copy Markdown
Member

@cfallin cfallin left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@cfallin cfallin added this pull request to the merge queue May 18, 2026
Merged via the queue into bytecodealliance:main with commit deedb95 May 18, 2026
51 checks passed
@fitzgen fitzgen deleted the issue-13365-branch-optimizations-bug-take-2 branch May 18, 2026 18:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cranelift Issues related to the Cranelift code generator

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Invalid CLIF generated from conditional branch rewrite optimizations

2 participants