From 283f3ea48e7eb8a981744c9e793559418420cdad Mon Sep 17 00:00:00 2001 From: Morgan Phillips Date: Mon, 1 Aug 2016 18:04:25 -0700 Subject: [PATCH] Remove uses of EntityMap::len. --- cranelift/src/libcretonne/cfg.rs | 60 ++++-------------- cranelift/src/libcretonne/dominator_tree.rs | 61 +++++++++++-------- .../src/libcretonne/tests/cfg_traversal.rs | 14 +++-- .../src/libcretonne/tests/dominator_tree.rs | 2 +- 4 files changed, 58 insertions(+), 79 deletions(-) diff --git a/cranelift/src/libcretonne/cfg.rs b/cranelift/src/libcretonne/cfg.rs index 28a39861e2..022c3372e4 100644 --- a/cranelift/src/libcretonne/cfg.rs +++ b/cranelift/src/libcretonne/cfg.rs @@ -25,7 +25,7 @@ use ir::Function; use ir::entities::{Inst, Ebb}; use ir::instructions::BranchInfo; -use entity_map::EntityMap; +use entity_map::{EntityMap, Keys}; use std::collections::HashSet; /// A basic block denoted by its enclosing Ebb and last instruction. @@ -97,12 +97,11 @@ impl ControlFlowGraph { &self.data[ebb].successors } - /// Return ebbs in reverse postorder along with a mapping of - /// the ebb to its [post]order of visitation. - pub fn reverse_postorder_ebbs(&self) -> EntityMap { + /// Return [reachable] ebbs in postorder. + pub fn postorder_ebbs(&self) -> Vec { let entry_block = match self.entry_block { None => { - return EntityMap::new(); + return Vec::new(); } Some(eb) => eb, }; @@ -129,46 +128,12 @@ impl ControlFlowGraph { black.insert(node.clone()); } } - postorder.reverse(); - - let mut result = EntityMap::with_capacity(postorder.len()); - for (offset, ebb) in postorder.iter().enumerate() { - let i = postorder.len() - offset; - result[ebb.clone()] = i; - } - result + postorder } - pub fn len(&self) -> usize { - self.data.len() - } - - pub fn predecessors_iter(&self) -> CFGPredecessorsIter { - CFGPredecessorsIter { - cur: 0, - cfg: &self, - } - } -} - -/// Iterate through every mapping of ebb to predecessors in the CFG -pub struct CFGPredecessorsIter<'a> { - cfg: &'a ControlFlowGraph, - cur: usize, -} - -impl<'a> Iterator for CFGPredecessorsIter<'a> { - type Item = (Ebb, &'a Vec); - - fn next(&mut self) -> Option { - if self.cur < self.cfg.len() { - let ebb = Ebb::with_number(self.cur as u32).unwrap(); - let bbs = self.cfg.get_predecessors(ebb); - self.cur += 1; - Some((ebb, bbs)) - } else { - None - } + /// An iterator across all of the ebbs stored in the cfg. + pub fn ebbs(&self) -> Keys { + self.data.keys() } } @@ -183,7 +148,7 @@ mod tests { fn empty() { let func = Function::new(); let cfg = ControlFlowGraph::new(&func); - assert_eq!(None, cfg.predecessors_iter().next()); + assert_eq!(None, cfg.ebbs().next()); } #[test] @@ -197,14 +162,13 @@ mod tests { func.layout.append_ebb(ebb2); let cfg = ControlFlowGraph::new(&func); - let nodes = cfg.predecessors_iter().collect::>(); + let nodes = cfg.ebbs().collect::>(); assert_eq!(nodes.len(), 3); let mut fun_ebbs = func.layout.ebbs(); - for (ebb, predecessors) in nodes { + for ebb in nodes { assert_eq!(ebb, fun_ebbs.next().unwrap()); - assert_eq!(predecessors.len(), 0); - assert_eq!(predecessors.len(), 0); + assert_eq!(cfg.get_predecessors(ebb).len(), 0); assert_eq!(cfg.get_successors(ebb).len(), 0); } } diff --git a/cranelift/src/libcretonne/dominator_tree.rs b/cranelift/src/libcretonne/dominator_tree.rs index c58726a1ab..8420bcf541 100644 --- a/cranelift/src/libcretonne/dominator_tree.rs +++ b/cranelift/src/libcretonne/dominator_tree.rs @@ -2,31 +2,33 @@ use cfg::*; use ir::entities::Ebb; -use entity_map::EntityMap; +use entity_map::{EntityMap, Keys}; pub struct DominatorTree { data: EntityMap>, } impl DominatorTree { - pub fn new(cfg: &ControlFlowGraph) -> DominatorTree { - let mut dt = DominatorTree { data: EntityMap::with_capacity(cfg.len()) }; - dt.build(cfg); - dt - } - - /// Build a dominator tree from a control flow graph using Keith D. Cooper's /// "Simple, Fast Dominator Algorithm." - fn build(&mut self, cfg: &ControlFlowGraph) { - let reverse_postorder_map = cfg.reverse_postorder_ebbs(); - let ebbs = reverse_postorder_map.keys().collect::>(); - let len = reverse_postorder_map.len(); + pub fn new(cfg: &ControlFlowGraph) -> DominatorTree { + let mut ebbs = cfg.postorder_ebbs(); + ebbs.reverse(); + + let len = ebbs.len(); + + // The mappings which designate the dominator tree. + let mut data = EntityMap::with_capacity(len); + + let mut postorder_map = EntityMap::with_capacity(len); + for (i, ebb) in ebbs.iter().enumerate() { + postorder_map[ebb.clone()] = len - i; + } let mut changed = false; if len > 0 { - self.data[ebbs[0]] = Some(ebbs[0]); + data[ebbs[0]] = Some(ebbs[0]); changed = true; } @@ -44,34 +46,42 @@ impl DominatorTree { } // If this predecessor `p` has an idom available find its common // ancestor with the current value of new_idom. - if let Some(_) = self.data[p] { + if let Some(_) = data[p] { new_idom = match new_idom { Some(cur_idom) => { - Some(self.intersect(&reverse_postorder_map, p, cur_idom)) + Some(DominatorTree::intersect(&mut data, + &postorder_map, + p, + cur_idom)) } None => panic!("A 'current idom' should have been set!"), } } } - match self.data[ebb] { + match data[ebb] { None => { - self.data[ebb] = new_idom; + data[ebb] = new_idom; changed = true; } Some(idom) => { // Old idom != New idom if idom != new_idom.unwrap() { - self.data[ebb] = new_idom; + data[ebb] = new_idom; changed = true; } } } } } + DominatorTree { data: data } } /// Find the common dominator of two ebbs. - fn intersect(&self, ordering: &EntityMap, first: Ebb, second: Ebb) -> Ebb { + fn intersect(data: &EntityMap>, + ordering: &EntityMap, + first: Ebb, + second: Ebb) + -> Ebb { let mut a = first; let mut b = second; @@ -81,10 +91,10 @@ impl DominatorTree { // self.data[b] to contain non-None entries. while a != b { while ordering[a] < ordering[b] { - a = self.data[a].unwrap(); + a = data[a].unwrap(); } while ordering[b] < ordering[a] { - b = self.data[b].unwrap(); + b = data[b].unwrap(); } } a @@ -96,9 +106,9 @@ impl DominatorTree { self.data[ebb].clone() } - /// The total number of nodes in the tree. - pub fn len(&self) -> usize { - self.data.len() + /// An iterator across all of the ebbs stored in the tree. + pub fn ebbs(&self) -> Keys { + self.data.keys() } } @@ -114,7 +124,7 @@ mod test { let func = Function::new(); let cfg = ControlFlowGraph::new(&func); let dtree = DominatorTree::new(&cfg); - assert_eq!(dtree.len(), 0); + assert_eq!(None, dtree.ebbs().next()); } #[test] @@ -143,7 +153,6 @@ mod test { let dt = DominatorTree::new(&cfg); assert_eq!(func.layout.entry_block().unwrap(), ebb3); - assert_eq!(dt.len(), cfg.len()); assert_eq!(dt.idom(ebb3).unwrap(), ebb3); assert_eq!(dt.idom(ebb1).unwrap(), ebb3); assert_eq!(dt.idom(ebb2).unwrap(), ebb1); diff --git a/cranelift/src/libcretonne/tests/cfg_traversal.rs b/cranelift/src/libcretonne/tests/cfg_traversal.rs index ae74445dd4..8908b279d8 100644 --- a/cranelift/src/libcretonne/tests/cfg_traversal.rs +++ b/cranelift/src/libcretonne/tests/cfg_traversal.rs @@ -4,6 +4,7 @@ extern crate cton_reader; use self::cton_reader::parser::Parser; use self::cretonne::ir::entities::Ebb; use self::cretonne::cfg::ControlFlowGraph; +use self::cretonne::entity_map::EntityMap; fn test_reverse_postorder_traversal(function_source: &str, ebb_order: Vec) { let func = &Parser::parse(function_source).unwrap()[0]; @@ -11,11 +12,16 @@ fn test_reverse_postorder_traversal(function_source: &str, ebb_order: Vec) let ebbs = ebb_order.iter().map(|n| Ebb::with_number(*n).unwrap()) .collect::>(); - let reverse_postorder_ebbs = cfg.reverse_postorder_ebbs(); + let mut postorder_ebbs = cfg.postorder_ebbs(); + let mut postorder_map = EntityMap::with_capacity(postorder_ebbs.len()); + for (i, ebb) in postorder_ebbs.iter().enumerate() { + postorder_map[ebb.clone()] = i + 1; + } + postorder_ebbs.reverse(); - assert_eq!(reverse_postorder_ebbs.len(), ebbs.len()); - for ebb in reverse_postorder_ebbs.keys() { - assert_eq!(ebb, ebbs[ebbs.len() - reverse_postorder_ebbs[ebb]]); + assert_eq!(postorder_ebbs.len(), ebbs.len()); + for ebb in postorder_ebbs { + assert_eq!(ebb, ebbs[ebbs.len() - postorder_map[ebb]]); } } diff --git a/cranelift/src/libcretonne/tests/dominator_tree.rs b/cranelift/src/libcretonne/tests/dominator_tree.rs index 6820acd152..8e10307a21 100644 --- a/cranelift/src/libcretonne/tests/dominator_tree.rs +++ b/cranelift/src/libcretonne/tests/dominator_tree.rs @@ -10,7 +10,7 @@ fn test_dominator_tree(function_source: &str, idoms: Vec) { let func = &Parser::parse(function_source).unwrap()[0]; let cfg = ControlFlowGraph::new(&func); let dtree = DominatorTree::new(&cfg); - assert_eq!(dtree.len(), idoms.len()); + assert_eq!(dtree.ebbs().collect::>().len(), idoms.len()); for (i, j) in idoms.iter().enumerate() { let ebb = Ebb::with_number(i.clone() as u32); let idom = Ebb::with_number(*j);