From e4890b9f84ba77ba96ad90a53c937fd9d067a779 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Fri, 15 May 2026 04:34:12 -0700 Subject: [PATCH] Handle irreducible control flow in branch simplification 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 only remove blocks when they haven't been marked reachable *and* all of its predecessors have been visited (this latter bit being the thing that irreducible control flow broke). To implement this, we pass in the already-computed `ControlFlowGraph` from the `Context` into the `EgraphPass` so that we can easily iterate of a block's predecessors. Fixes https://github.com/bytecodealliance/wasmtime/issues/13365 --- cranelift/codegen/src/context.rs | 1 + cranelift/codegen/src/egraph/mod.rs | 24 ++++++++++++++- .../filetests/egraph/issue-13365.clif | 29 +++++++++++++++++++ 3 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 cranelift/filetests/filetests/egraph/issue-13365.clif diff --git a/cranelift/codegen/src/context.rs b/cranelift/codegen/src/context.rs index d83388501b9f..2a9b26817f67 100644 --- a/cranelift/codegen/src/context.rs +++ b/cranelift/codegen/src/context.rs @@ -378,6 +378,7 @@ impl Context { let mut alias_analysis = AliasAnalysis::new(&self.func, &self.domtree); let mut pass = EgraphPass::new( &mut self.func, + &self.cfg, &self.domtree, &self.loop_analysis, &mut alias_analysis, diff --git a/cranelift/codegen/src/egraph/mod.rs b/cranelift/codegen/src/egraph/mod.rs index 47455a9ecac8..de5e53843128 100644 --- a/cranelift/codegen/src/egraph/mod.rs +++ b/cranelift/codegen/src/egraph/mod.rs @@ -6,6 +6,7 @@ use crate::ctxhash::{CtxEq, CtxHash, NullCtx}; use crate::cursor::{Cursor, CursorPosition, FuncCursor}; use crate::dominator_tree::DominatorTree; use crate::egraph::elaborate::Elaborator; +use crate::flowgraph::ControlFlowGraph; use crate::inst_predicates::{is_mergeable_for_egraph, is_pure_for_egraph}; use crate::ir::{ Block, DataFlowGraph, Function, Inst, InstructionData, Type, Value, ValueDef, ValueListPool, @@ -251,6 +252,8 @@ impl Iterator for EgraphBlockIter<'_> { pub struct EgraphPass<'a> { /// The function we're operating on. func: &'a mut Function, + /// Control-flow graph for the function. + cfg: &'a ControlFlowGraph, /// Dominator tree for the CFG, used to visit blocks in pre-order /// so we see value definitions before their uses, and also used for /// O(1) dominance checks. @@ -865,6 +868,7 @@ impl<'a> EgraphPass<'a> { /// Create a new EgraphPass. pub fn new( func: &'a mut Function, + cfg: &'a ControlFlowGraph, domtree: &'a DominatorTree, loop_analysis: &'a LoopAnalysis, alias_analysis: &'a mut AliasAnalysis<'a>, @@ -874,6 +878,7 @@ impl<'a> EgraphPass<'a> { reachable_blocks.insert(func.layout.entry_block().unwrap()); Self { func, + cfg, domtree, loop_analysis, alias_analysis, @@ -962,6 +967,7 @@ impl<'a> EgraphPass<'a> { // above join.) let mut available_block: SecondaryMap = SecondaryMap::with_default(Block::reserved_value()); + let mut visited_blocks = EntitySet::with_capacity(cursor.func.dfg.num_blocks()); // To avoid blowing up eclasses too much, we track the size of // each eclass reachable by a tree of union nodes from a given @@ -996,6 +1002,8 @@ impl<'a> EgraphPass<'a> { gvn_map.increment_depth(); gvn_map_blocks.push(block); + // Check that `gvn_map_blocks` is the path from this block up to the + // root in the dominator tree. debug_assert_eq!(gvn_map_blocks, { let mut b = Some(block); let mut v = core::iter::from_fn(move || { @@ -1008,8 +1016,21 @@ impl<'a> EgraphPass<'a> { v }); - if !self.reachable_blocks.contains(block) { + // If the block is not marked reachable and all of its predecessors + // have been visited, then we can simply remove it. + // + // Note that the predecessors check is only required for irreducible + // control flow: irreducible CFGs can have blocks that are *only* + // reachable through back-edges, and so the `EgraphBlockIter` + // proof's property is not sufficient. + if !self.reachable_blocks.contains(block) + && self + .cfg + .pred_iter(block) + .all(|pred| visited_blocks.contains(pred.block)) + { cursor.func.layout.remove_block_and_insts(block); + visited_blocks.insert(block); continue; } @@ -1097,6 +1118,7 @@ impl<'a> EgraphPass<'a> { self.reachable_blocks .insert(dest.block(&cursor.func.dfg.value_lists)); } + visited_blocks.insert(block); } } diff --git a/cranelift/filetests/filetests/egraph/issue-13365.clif b/cranelift/filetests/filetests/egraph/issue-13365.clif new file mode 100644 index 000000000000..4c6addce2b1e --- /dev/null +++ b/cranelift/filetests/filetests/egraph/issue-13365.clif @@ -0,0 +1,29 @@ +test optimize precise-output +set opt_level=speed +target x86_64 + +; Test branch simplifications and irreducible control flow. + +function %f() { +block0: + v0 = iconst.i8 0 + brif v0, block2, block1 + +block1: + jump block2 + +block2: + jump block1 +} + +; function %f() fast { +; block0: +; jump block1 +; +; block1: +; jump block2 +; +; block2: +; jump block1 +; } +