diff --git a/cranelift/codegen/src/alias_analysis.rs b/cranelift/codegen/src/alias_analysis.rs index 6a6e9f2748ba..ae95849b73e9 100644 --- a/cranelift/codegen/src/alias_analysis.rs +++ b/cranelift/codegen/src/alias_analysis.rs @@ -63,7 +63,7 @@ use crate::{ cursor::{Cursor, FuncCursor}, - dominator_tree::DominatorTree, + dominator_tree::DominatorTreePreorder, inst_predicates::{ has_memory_fence_semantics, inst_addr_offset_type, inst_store_data, visit_block_succs, }, @@ -176,7 +176,7 @@ struct MemoryLoc { /// An alias-analysis pass. pub struct AliasAnalysis<'a> { /// The domtree for the function. - domtree: &'a DominatorTree, + domtree: &'a DominatorTreePreorder, /// Input state to a basic block. block_input: FxHashMap, @@ -191,7 +191,7 @@ pub struct AliasAnalysis<'a> { impl<'a> AliasAnalysis<'a> { /// Perform an alias analysis pass. - pub fn new(func: &Function, domtree: &'a DominatorTree) -> AliasAnalysis<'a> { + pub fn new(func: &Function, domtree: &'a DominatorTreePreorder) -> AliasAnalysis<'a> { trace!("alias analysis: input is:\n{:?}", func); let mut analysis = AliasAnalysis { domtree, @@ -333,7 +333,7 @@ impl<'a> AliasAnalysis<'a> { value.index(), def_inst.index() ); - if self.domtree.dominates(def_inst, inst, &func.layout) { + if self.domtree.dominates_inst(def_inst, inst, &func.layout) { trace!( " -> dominates; value equiv from v{} to v{} inserted", load_result.index(), diff --git a/cranelift/codegen/src/context.rs b/cranelift/codegen/src/context.rs index d95e985a4854..5fc8b767ac9f 100644 --- a/cranelift/codegen/src/context.rs +++ b/cranelift/codegen/src/context.rs @@ -10,7 +10,7 @@ //! single ISA instance. use crate::alias_analysis::AliasAnalysis; -use crate::dominator_tree::DominatorTree; +use crate::dominator_tree::{DominatorTree, DominatorTreePreorder}; use crate::egraph::EgraphPass; use crate::flowgraph::ControlFlowGraph; use crate::ir::Function; @@ -46,6 +46,9 @@ pub struct Context { /// Dominator tree for `func`. pub domtree: DominatorTree, + /// Domniator tree preorder, be on look out to merge with domtree, although unlikely + pub domtree_preorder: DominatorTreePreorder, + /// Loop analysis of `func`. pub loop_analysis: LoopAnalysis, @@ -74,6 +77,7 @@ impl Context { func, cfg: ControlFlowGraph::new(), domtree: DominatorTree::new(), + domtree_preorder: DominatorTreePreorder::new(), loop_analysis: LoopAnalysis::new(), compiled_code: None, want_disasm: false, @@ -309,7 +313,9 @@ impl Context { /// Compute dominator tree. pub fn compute_domtree(&mut self) { - self.domtree.compute(&self.func, &self.cfg) + self.domtree.compute(&self.func, &self.cfg); + self.domtree_preorder + .compute(&self.domtree, &self.func.layout); } /// Compute the loop analysis. @@ -339,7 +345,7 @@ impl Context { /// by a store instruction to the same instruction (so-called /// "store-to-load forwarding"). pub fn replace_redundant_loads(&mut self) -> CodegenResult<()> { - let mut analysis = AliasAnalysis::new(&self.func, &self.domtree); + let mut analysis = AliasAnalysis::new(&self.func, &self.domtree_preorder); analysis.compute_and_update_aliases(&mut self.func); Ok(()) } @@ -371,7 +377,8 @@ impl Context { ); let fisa = fisa.into(); self.compute_loop_analysis(); - let mut alias_analysis = AliasAnalysis::new(&self.func, &self.domtree); + let mut alias_analysis = AliasAnalysis::new(&self.func, &self.domtree_preorder); + let mut pass = EgraphPass::new( &mut self.func, &self.domtree, diff --git a/cranelift/codegen/src/dominator_tree.rs b/cranelift/codegen/src/dominator_tree.rs index 12288362b48b..bfe26930731a 100644 --- a/cranelift/codegen/src/dominator_tree.rs +++ b/cranelift/codegen/src/dominator_tree.rs @@ -439,6 +439,13 @@ impl DominatorTreePreorder { } } } + + /// Clear the data structure + /// Recursively clears its underlying children + pub fn clear(&mut self) { + self.nodes.clear(); + self.stack.clear(); + } } /// An iterator that enumerates the direct children of a block in the dominator tree. @@ -482,7 +489,42 @@ impl DominatorTreePreorder { pub fn dominates(&self, a: Block, b: Block) -> bool { let na = &self.nodes[a]; let nb = &self.nodes[b]; - na.pre_number <= nb.pre_number && na.pre_max >= nb.pre_max + // debug_assert!(na.pre_number != 0); + // debug_assert!(nb.pre_number != 0); + if a == b { + // check first if they're the same block + // if they are then they dominate themselves + true + } else if nb.pre_number == 0 { + // if the supposedly dominated is not reachable (pre_number = 0), is it + // really being dominated though ? + false + } else { + na.pre_number <= nb.pre_number && na.pre_max >= nb.pre_max + } + } + + /// Checks if one instruction dominates another. + /// + /// From @jameysharp: + /// Instruction a dominates instruction b if either of these conditions are true: + /// a and b are in the same block, and a is not after b. + /// Or the block containing a dominates the block containing b. + /// + /// "I would prefer to have dominates_inst next to the dominates method that it uses. + /// Then someone looking at one of them can easily see the other one too." + /// + pub fn dominates_inst(&self, a: Inst, b: Inst, layout: &Layout) -> bool { + match (layout.inst_block(a), layout.inst_block(b)) { + (Some(block_a), Some(block_b)) => { + if block_a == block_b { + layout.pp_cmp(a, b) != Ordering::Greater + } else { + self.dominates(block_a, block_b) + } + } + _ => false, + } } /// Compare two blocks according to the dominator pre-order.