Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

converted DominatorTree to DominatorTreePreorder in alias_analysis file #8535

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

converted DominatorTree to DominatorTreePreorder in alias_analysis file #8535

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
eb42a63
converted DominatorTree to DominatorTreePreorder in alias_analysis file
VamshiReddy02 May 3, 2024
7b725cc
converted DominatorTree to DominatorTreePreorder in alias_analysis file
VamshiReddy02 May 6, 2024
eee4d2f
added dominates_inst method in dominator_tree.rs
VamshiReddy02 May 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions cranelift/codegen/src/alias_analysis.rs
View file
Open in desktop
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 @@ -334,7 +334,11 @@ 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) {
let _inst_block = func
.layout
.inst_block(inst)
.expect("Instruction not in layout.");
Comment on lines +338 to +341
Copy link
Contributor

Choose a reason for hiding this comment

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

This statement can be removed, because the instruction's block is not used here.

The place that I meant you could use this pattern is in the dominates_inst function, where you could use it instead of the match expression. But you don't have to do that if you don't want to; that function is okay the way you wrote it.

trace!(
" -> dominates; value equiv from v{} to v{} inserted",
load_result.index(),
Expand Down
7 changes: 5 additions & 2 deletions cranelift/codegen/src/context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use crate::alias_analysis::AliasAnalysis;
use crate::dce::do_dce;
use crate::dominator_tree::DominatorTree;
use crate::dominator_tree::DominatorTreePreorder;
use crate::egraph::EgraphPass;
use crate::flowgraph::ControlFlowGraph;
use crate::ir::Function;
Expand Down Expand Up @@ -46,6 +47,7 @@ pub struct Context {

/// Dominator tree for `func`.
pub domtree: DominatorTree,
domtree_preorder: DominatorTreePreorder,

/// Loop analysis of `func`.
pub loop_analysis: LoopAnalysis,
Expand Down Expand Up @@ -75,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 @@ -344,7 +347,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 @@ -376,7 +379,7 @@ 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
14 changes: 14 additions & 0 deletions cranelift/codegen/src/dominator_tree.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,20 @@ impl DominatorTreePreorder {
}
}

/// Checks if one instruction dominates another.
pub fn dominates_inst(&self, a: Inst, b: Inst, layout: &Layout) -> bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a minor thing, but 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.

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,
}
}

/// Recompute this data structure to match `domtree`.
pub fn compute(&mut self, domtree: &DominatorTree, layout: &Layout) {
self.nodes.clear();
Expand Down
Loading