Modify the dominator tree's intersect method to interact with Basic Blocks

Corresponding changes to test cases are also included.

src/libcretonne/dominator_tree.rs

@@ -10,6 +10,11 @@ pub struct DominatorTree {
 }
 
 impl DominatorTree {
+    /// Insert data directly into a dominator tree.
+    pub fn from_data(data: EntityMap<Ebb, Option<BasicBlock>>) -> DominatorTree {
+        DominatorTree { data: data }
+    }
+
     /// Build a dominator tree from a control flow graph using Keith D. Cooper's
     /// "Simple, Fast Dominator Algorithm."
     pub fn new(cfg: &ControlFlowGraph) -> DominatorTree {
@@ -40,21 +45,20 @@ impl DominatorTree {
                 let preds = cfg.get_predecessors(ebb);
                 let mut new_idom = None;
 
-                for &(p_ebb, _) in preds {
+                for pred in preds {
                     if new_idom == None {
-                        new_idom = Some((p_ebb, NO_INST));
+                        new_idom = Some(pred.clone());
                         continue;
                     }
                     // If this predecessor has an idom available find its common
                     // ancestor with the current value of new_idom.
-                    if let Some(_) = data[p_ebb] {
+                    if let Some(_) = data[pred.0] {
                         new_idom = match new_idom {
                             Some(cur_idom) => {
                                 Some((DominatorTree::intersect(&mut data,
                                                                &postorder_map,
-                                                               p_ebb,
+                                                               *pred,
-                                                               cur_idom.0),
+                                                               cur_idom)))
-                                      NO_INST))
                             }
                             None => panic!("A 'current idom' should have been set!"),
                         }
@@ -67,7 +71,7 @@ impl DominatorTree {
                     }
                     Some(idom) => {
                         // Old idom != New idom
-                        if idom != new_idom.unwrap() {
+                        if idom.0 != new_idom.unwrap().0 {
                             data[ebb] = new_idom;
                             changed = true;
                         }
@@ -76,39 +80,15 @@ impl DominatorTree {
             }
         }
 
-        // At this point the basic blocks in the tree are incomplete
-        // since they have all been set with NO_INST. Here we add instructions
-        // by iterating through each Ebb -> BasicBlock mapping in the dominator
-        // tree and replacing the basic block with a corresponding predecessor
-        // from the Ebb (on the left hand side).
-        //
-        // The predecessor chosen should have the lowest instruction number and
-        // an Ebb which matches the Ebb from the dummy basic block. Because
-        // extended basic blocks have a single entry point this will always
-        // result in the correct basic block being chosen.
-        for lhs_ebb in ebbs {
-            let rhs_bb = data[lhs_ebb].unwrap();
-            for pred_bb in cfg.get_predecessors(lhs_ebb) {
-                if rhs_bb.0 == pred_bb.0 {
-                    // Predecessors are added in order while iterating through
-                    // instructions from lowest to highest. Because of this,
-                    // the first match we encounter will have the lowest instruction
-                    // number.
-                    data[lhs_ebb] = Some(pred_bb.clone());
-                    break;
-                }
-            }
-        }
-
         DominatorTree { data: data }
     }
 
     /// Find the common dominator of two ebbs.
     fn intersect(data: &EntityMap<Ebb, Option<BasicBlock>>,
                  ordering: &EntityMap<Ebb, usize>,
-                 first: Ebb,
+                 first: BasicBlock,
-                 second: Ebb)
+                 second: BasicBlock)
-                 -> Ebb {
+                 -> BasicBlock {
         let mut a = first;
         let mut b = second;
 
@@ -116,15 +96,23 @@ impl DominatorTree {
         // visitation number, to ensure that we move upward through the tree.
         // Walking upward means that we may always expect self.data[a] and
         // self.data[b] to contain non-None entries.
-        while a != b {
+        while a.0 != b.0 {
-            while ordering[a] < ordering[b] {
+            while ordering[a.0] < ordering[b.0] {
-                a = data[a].unwrap().0;
+                a = data[a.0].unwrap();
             }
-            while ordering[b] < ordering[a] {
+            while ordering[b.0] < ordering[a.0] {
-                b = data[b].unwrap().0;
+                b = data[b.0].unwrap();
             }
         }
-        a
+
+        // TODO: we can't rely on instruction numbers to always be ordered
+        // from lowest to highest. Given that, it will be necessary to create
+        // an abolute mapping to determine the instruction order in the future.
+        if a.1 == NO_INST || a.1 < b.1 {
+            a
+        } else {
+            b
+        }
     }
 
     /// Returns the immediate dominator of some ebb or None if the

src/tools/Cargo.toml

@@ -14,3 +14,4 @@ cretonne = { path = "../libcretonne" }
 cretonne-reader = { path = "../libreader" }
 docopt = "0.6.80"
 rustc-serialize = "0.3.19"
+regex = "0.1.73"

src/tools/tests/dominator_tree.rs

@@ -1,49 +1,77 @@
 extern crate cretonne;
 extern crate cton_reader;
+extern crate regex;
 
+use regex::Regex;
 use self::cretonne::ir::Ebb;
 use self::cton_reader::parser::Parser;
+use self::cretonne::ir::function::Function;
+use self::cretonne::entity_map::EntityMap;
 use self::cretonne::ir::entities::NO_INST;
 use self::cretonne::cfg::ControlFlowGraph;
-use self::cretonne::ir::instructions::BranchInfo;
 use self::cretonne::dominator_tree::DominatorTree;
 
-fn test_dominator_tree(function_source: &str, idoms: Vec<u32>) {
+/// Construct a dominator tree from specially formatted comments in
+/// cton source. Each line with a  jump/branch instruction should
+/// have a comment of the format: `dominates(n, ..., N)`, where each `n`
+/// is the Ebb number for which this instruction is the immediate dominator.
+fn dominator_tree_from_source(func: &Function, function_source: &str) -> DominatorTree {
+    let ebb_re = Regex::new("^[ \t]*ebb[0-9]+.*:").unwrap();
+    let dom_re = Regex::new("dominates\\(([0-9,]+)\\)").unwrap();
+    let inst_re = Regex::new("^[ \t]*[a-zA-Z0-9]+[^{}]*").unwrap();
+    let func_re = Regex::new("^[ \t]*function.*").unwrap();
+
+    let ebbs = func.layout.ebbs().collect::<Vec<_>>();
+    let mut data = EntityMap::with_capacity(ebbs.len());
+
+    if ebbs.len() < 1 {
+        return DominatorTree::from_data(data);
+    }
+
+    let mut ebb_offset = 0;
+    let mut inst_offset = 0;
+
+    let mut cur_ebb = ebbs[0];
+    let mut insts = func.layout.ebb_insts(ebbs[ebb_offset]).collect::<Vec<_>>();
+
+    for line in function_source.lines() {
+        if ebb_re.is_match(line) {
+            cur_ebb = ebbs[ebb_offset];
+            insts = func.layout.ebb_insts(cur_ebb).collect::<Vec<_>>();
+            ebb_offset += 1;
+            inst_offset = 0;
+        } else if inst_re.is_match(line) && !func_re.is_match(line) {
+            inst_offset += 1;
+        }
+        match dom_re.captures(line) {
+            Some(caps) => {
+                for s in caps.at(1).unwrap().split(",") {
+                    let this_ebb = Ebb::with_number(s.parse::<u32>().unwrap()).unwrap();
+                    let inst = if inst_offset == 0 {
+                        NO_INST
+                    } else {
+                        insts[inst_offset - 1].clone()
+                    };
+                    data[this_ebb] = Some((cur_ebb.clone(), inst));
+                }
+            },
+            None => continue,
+        };
+
+    }
+    DominatorTree::from_data(data)
+}
+
+fn test_dominator_tree(function_source: &str) {
+
     let func = &Parser::parse(function_source).unwrap()[0];
+    let src_dtree = dominator_tree_from_source(&func, function_source);
+
     let cfg = ControlFlowGraph::new(&func);
     let dtree = DominatorTree::new(&cfg);
-    assert_eq!(dtree.ebbs().collect::<Vec<_>>().len(), idoms.len());
-    for (i, j) in idoms.iter().enumerate() {
-        let ebb = Ebb::with_number(i.clone() as u32).unwrap();
-        let idom_ebb = Ebb::with_number(*j).unwrap();
-        let mut idom_inst = NO_INST;
 
-        // Find the first branch/jump instruction which points to the idom_ebb
+    for ebb in func.layout.ebbs() {
-        // and use it to denote our idom basic block.
+        assert_eq!(dtree.idom(ebb), src_dtree.idom(ebb));
-        for inst in func.layout.ebb_insts(idom_ebb) {
-           match func.dfg[inst].analyze_branch() {
-                BranchInfo::SingleDest(dest, _) => {
-                    if dest == ebb {
-                        idom_inst = inst;
-                        break;
-                    }
-                }
-                BranchInfo::Table(jt) => {
-                    for (_, dest) in func.jump_tables[jt].entries() {
-                        if dest == ebb {
-                            idom_inst = inst;
-                            break;
-                        }
-                    }
-                    // We already found our inst!
-                    if idom_inst != NO_INST {
-                        break;
-                    }
-                }
-                BranchInfo::NotABranch => {}
-            }
-        }
-        assert_eq!(dtree.idom(ebb).unwrap(), (idom_ebb, idom_inst));
     }
 }
 
@@ -51,26 +79,26 @@ fn test_dominator_tree(function_source: &str, idoms: Vec<u32>) {
 fn basic() {
     test_dominator_tree("
         function test(i32) {
-            ebb0(v0: i32):
+            ebb0(v0: i32):    ; dominates(0)
-                jump ebb1
+                jump ebb1     ; dominates(1)
             ebb1:
-                brz v0, ebb3
+                brz v0, ebb3  ; dominates(3)
-                jump ebb2
+                jump ebb2     ; dominates(2)
             ebb2:
                 jump ebb3
             ebb3:
                 return
         }
-    ", vec![0, 0, 1, 1]);
+    ");
 }
 
 #[test]
 fn loops() {
     test_dominator_tree("
         function test(i32) {
-            ebb0(v0: i32):
+            ebb0(v0: i32):    ; dominates(0)
-                brz v0, ebb1
+                brz v0, ebb1  ; dominates(1,3,4,5)
-                jump ebb2
+                jump ebb2     ; dominates(2)
             ebb1:
                 jump ebb3
             ebb2:
@@ -85,5 +113,5 @@ fn loops() {
                 brz v0, ebb4
                 return
         }
-    ", vec![0, 0, 0, 0, 0, 0]);
+    ");
 }