From 05ba8dcab652d306b24c12ef1a547c087057ff6c Mon Sep 17 00:00:00 2001 From: Denis Merigoux Date: Thu, 3 Aug 2017 18:26:41 -0700 Subject: [PATCH] Added partial recompute of dominator tree in case of Ebb splitting (#135) * Partial recompute for dominator tree in case of ebb splitting Implemented via striding in RPO numbers --- lib/cretonne/src/dominator_tree.rs | 138 ++++++++++++++++++++++++++++- lib/cretonne/src/verifier/mod.rs | 30 +++++++ 2 files changed, 164 insertions(+), 4 deletions(-) diff --git a/lib/cretonne/src/dominator_tree.rs b/lib/cretonne/src/dominator_tree.rs index 14f08e4263..6e61eeaa9d 100644 --- a/lib/cretonne/src/dominator_tree.rs +++ b/lib/cretonne/src/dominator_tree.rs @@ -7,10 +7,16 @@ use packed_option::PackedOption; use std::cmp::Ordering; +// RPO numbers are not first assigned in a contiguous way but as multiples of STRIDE, to leave +// room for modifications of the dominator tree. +const STRIDE: u32 = 4; + // Dominator tree node. We keep one of these per EBB. #[derive(Clone, Default)] struct DomNode { // Number of this node in a reverse post-order traversal of the CFG, starting from 1. + // This number is monotonic in the reverse postorder but not contiguous, since we leave + // holes for later localized modifications of the dominator tree. // Unreachable nodes get number 0, all others are positive. rpo_number: u32, @@ -137,7 +143,6 @@ impl DominatorTree { ebb_b = layout.inst_ebb(idom).expect("Dominator got removed."); inst_b = Some(idom); } - if a == ebb_b { inst_b } else { None } } @@ -266,10 +271,11 @@ impl DominatorTree { debug_assert_eq!(Some(entry_block), func.layout.entry_block()); // Do a first pass where we assign RPO numbers to all reachable nodes. - self.nodes[entry_block].rpo_number = 2; + self.nodes[entry_block].rpo_number = 2 * STRIDE; for (rpo_idx, &ebb) in postorder.iter().rev().enumerate() { // Update the current node and give it an RPO number. - // The entry block got 2, the rest start at 3. + // The entry block got 2, the rest start at 3 by multiples of STRIDE to leave + // room for future dominator tree modifications. // // Since `compute_idom` will only look at nodes with an assigned RPO number, the // function will never see an uninitialized predecessor. @@ -278,7 +284,7 @@ impl DominatorTree { // least one predecessor that has previously been visited during this RPO. self.nodes[ebb] = DomNode { idom: self.compute_idom(ebb, cfg, &func.layout).into(), - rpo_number: rpo_idx as u32 + 3, + rpo_number: (rpo_idx as u32 + 3) * STRIDE, } } @@ -323,11 +329,69 @@ impl DominatorTree { } } +impl DominatorTree { + /// When splitting an `Ebb` using `Layout::split_ebb`, you can use this method to update + /// the dominator tree locally rather than recomputing it. + /// + /// `old_ebb` is the `Ebb` before splitting, and `new_ebb` is the `Ebb` which now contains + /// the second half of `old_ebb`. `split_jump_inst` is the terminator jump instruction of + /// `old_ebb` that points to `new_ebb`. + pub fn recompute_split_ebb(&mut self, old_ebb: Ebb, new_ebb: Ebb, split_jump_inst: Inst) { + if !self.is_reachable(old_ebb) { + // old_ebb is unreachable, it stays so and new_ebb is unreachable too + *self.nodes.ensure(new_ebb) = Default::default(); + return; + } + // We use the RPO comparison on the postorder list so we invert the operands of the + // comparison + let old_ebb_postorder_index = + self.postorder + .as_slice() + .binary_search_by(|probe| self.rpo_cmp_ebb(old_ebb, *probe)) + .expect("the old ebb is not declared to the dominator tree"); + let new_ebb_rpo = self.insert_after_rpo(old_ebb, old_ebb_postorder_index, new_ebb); + *self.nodes.ensure(new_ebb) = DomNode { + rpo_number: new_ebb_rpo, + idom: Some(split_jump_inst).into(), + }; + + } + + // Insert new_ebb just after ebb in the RPO. This function checks + // if there is a gap in rpo numbers; if yes it returns the number in the gap and if + // not it renumbers. + fn insert_after_rpo(&mut self, ebb: Ebb, ebb_postorder_index: usize, new_ebb: Ebb) -> u32 { + let ebb_rpo_number = self.nodes[ebb].rpo_number; + let inserted_rpo_number = ebb_rpo_number + 1; + // If there is no gaps in RPo numbers to insert this new number, we iterate + // forward in RPO numbers and backwards in the postorder list of EBBs, renumbering the Ebbs + // until we find a gap + for (¤t_ebb, current_rpo) in + self.postorder[0..ebb_postorder_index] + .iter() + .rev() + .zip(inserted_rpo_number + 1..) { + if self.nodes[current_ebb].rpo_number < current_rpo { + // There is no gap, we renumber + self.nodes[current_ebb].rpo_number = current_rpo; + } else { + // There is a gap, we stop the renumbering and exit + break; + } + } + // TODO: insert in constant time? + self.postorder.insert(ebb_postorder_index, new_ebb); + inserted_rpo_number + } +} + #[cfg(test)] mod test { use flowgraph::ControlFlowGraph; use ir::{Function, InstBuilder, Cursor, types}; use super::*; + use ir::types::*; + use verifier::verify_context; #[test] fn empty() { @@ -465,4 +529,70 @@ mod test { assert!(!dt.dominates(jmp21, ebb2, &func.layout)); assert!(dt.dominates(jmp21, jmp21, &func.layout)); } + + #[test] + fn renumbering() { + let mut func = Function::new(); + let ebb0 = func.dfg.make_ebb(); + let ebb100 = func.dfg.make_ebb(); + + let inst2; + let inst3; + let inst4; + let inst5; + { + let dfg = &mut func.dfg; + let cur = &mut Cursor::new(&mut func.layout); + + cur.insert_ebb(ebb0); + let cond = dfg.ins(cur).iconst(I32, 0); + inst2 = dfg.ins(cur).brz(cond, ebb0, &[]); + inst3 = dfg.ins(cur).brz(cond, ebb0, &[]); + inst4 = dfg.ins(cur).brz(cond, ebb0, &[]); + inst5 = dfg.ins(cur).brz(cond, ebb0, &[]); + dfg.ins(cur).jump(ebb100, &[]); + cur.insert_ebb(ebb100); + dfg.ins(cur).return_(&[]); + } + let mut cfg = ControlFlowGraph::with_function(&func); + let mut dt = DominatorTree::with_function(&func, &cfg); + + let ebb1 = func.dfg.make_ebb(); + func.layout.split_ebb(ebb1, inst2); + let middle_jump_inst = { + let cur = &mut Cursor::new(&mut func.layout); + cur.goto_bottom(ebb0); + func.dfg.ins(cur).jump(ebb1, &[]) + }; + dt.recompute_split_ebb(ebb0, ebb1, middle_jump_inst); + + let ebb2 = func.dfg.make_ebb(); + func.layout.split_ebb(ebb2, inst3); + let middle_jump_inst = { + let cur = &mut Cursor::new(&mut func.layout); + cur.goto_bottom(ebb1); + func.dfg.ins(cur).jump(ebb2, &[]) + }; + dt.recompute_split_ebb(ebb1, ebb2, middle_jump_inst); + + let ebb3 = func.dfg.make_ebb(); + func.layout.split_ebb(ebb3, inst4); + let middle_jump_inst = { + let cur = &mut Cursor::new(&mut func.layout); + cur.goto_bottom(ebb2); + func.dfg.ins(cur).jump(ebb3, &[]) + }; + dt.recompute_split_ebb(ebb2, ebb3, middle_jump_inst); + + let ebb4 = func.dfg.make_ebb(); + func.layout.split_ebb(ebb4, inst5); + let middle_jump_inst = { + let cur = &mut Cursor::new(&mut func.layout); + cur.goto_bottom(ebb3); + func.dfg.ins(cur).jump(ebb4, &[]) + }; + dt.recompute_split_ebb(ebb3, ebb4, middle_jump_inst); + cfg.compute(&func); + verify_context(&func, &cfg, &dt, None).unwrap(); + } } diff --git a/lib/cretonne/src/verifier/mod.rs b/lib/cretonne/src/verifier/mod.rs index 751f6d5f6f..d3f05ef4c8 100644 --- a/lib/cretonne/src/verifier/mod.rs +++ b/lib/cretonne/src/verifier/mod.rs @@ -63,6 +63,8 @@ use std::error as std_error; use std::fmt::{self, Display, Formatter}; use std::result; use std::collections::BTreeSet; +use std::cmp::Ordering; +use iterators::IteratorExtras; pub use self::liveness::verify_liveness; pub use self::cssa::verify_cssa; @@ -409,6 +411,34 @@ impl<'a> Verifier<'a> { got); } } + // We also verify if the postorder defined by `DominatorTree` is sane + if self.domtree.cfg_postorder().len() != domtree.cfg_postorder().len() { + return err!(AnyEntity::Function, + "incorrect number of Ebbs in postorder traversal"); + } + for (index, (&true_ebb, &test_ebb)) in + self.domtree + .cfg_postorder() + .iter() + .zip(domtree.cfg_postorder().iter()) + .enumerate() { + if true_ebb != test_ebb { + return err!(test_ebb, + "invalid domtree, postorder ebb number {} should be {}, got {}", + index, + true_ebb, + test_ebb); + } + } + // We verify rpo_cmp on pairs of adjacent ebbs in the postorder + for (&prev_ebb, &next_ebb) in self.domtree.cfg_postorder().iter().adjacent_pairs() { + if domtree.rpo_cmp(prev_ebb, next_ebb, &self.func.layout) != Ordering::Greater { + return err!(next_ebb, + "invalid domtree, rpo_cmp does not says {} is greater than {}", + prev_ebb, + next_ebb); + } + } Ok(()) }