Skip to content

Commit

Permalink
Attempt to fix #7954
Browse files Browse the repository at this point in the history
 Changes to be committed:
	modified:   cranelift/codegen/src/alias_analysis.rs: Use
        dominator tree
        modified:   cranelift/codegen/src/context.rs: Use dominator tree
	modified:   cranelift/codegen/src/dominator_tree.rs: Fix
        dominates and computes and modify logic of domination
  • Loading branch information
badumbatish committed Sep 7, 2024
1 parent 7081b8f commit 1246a81
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 9 deletions.
8 changes: 4 additions & 4 deletions cranelift/codegen/src/alias_analysis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down Expand Up @@ -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<Block, LastStores>,
Expand All @@ -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,
Expand Down Expand Up @@ -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(),
Expand Down
15 changes: 11 additions & 4 deletions cranelift/codegen/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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(())
}
Expand Down Expand Up @@ -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,
Expand Down
44 changes: 43 additions & 1 deletion cranelift/codegen/src/dominator_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down

0 comments on commit 1246a81

Please sign in to comment.