Merge branch 'efficient_licm' of github.com:denismerigoux/cretonne in…

…to efficient_licm

lib/cretonne/src/context.rs

@@ -121,7 +121,13 @@ impl Context {
         self.verify(None).map_err(Into::into)
     }
 
-    /// Perform LICM on the function.
+    /// Recompute the loop analysis of the function. Needs the flowgraph to be computed first.
+    pub fn loops(&mut self) {
+        self.loop_analysis
+            .compute(&mut self.func, &mut self.cfg, &mut self.domtree)
+    }
+
+    /// Perform LICM on the function. Needs the loop analysis to be computed first.
     pub fn licm(&mut self) -> CtonResult {
         do_licm(&mut self.func,
                 &mut self.cfg,

lib/cretonne/src/dominator_tree.rs

@@ -12,7 +12,7 @@ use std::cmp::Ordering;
 const STRIDE: u32 = 4;
 
 // Dominator tree node. We keep one of these per EBB.
-#[derive(Clone, Default)]
+#[derive(Clone, Default, Debug)]
 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
@@ -29,6 +29,7 @@ struct DomNode {
 }
 
 /// The dominator tree for a single function.
+#[derive(Debug)]
 pub struct DominatorTree {
     nodes: EntityMap<Ebb, DomNode>,
 

lib/cretonne/src/flowgraph.rs

@@ -98,7 +98,7 @@ impl ControlFlowGraph {
         // Temporarily take ownership because we need mutable access to self.data inside the loop.
         // Unfortunately borrowck cannot see that our mut accesses to predecessors don't alias
         // our iteration over successors.
-        let mut successors = mem::replace(&mut self.data[ebb].successors, Vec::new());
+        let mut successors = mem::replace(&mut self.data.ensure(ebb).successors, Vec::new());
         for suc in successors.iter().cloned() {
             self.data[suc].predecessors.retain(|&(e, _)| e != ebb);
         }
@@ -118,8 +118,8 @@ impl ControlFlowGraph {
     }
 
     fn add_edge(&mut self, from: BasicBlock, to: Ebb) {
-        self.data[from.0].successors.push(to);
-        self.data[to].predecessors.push(from);
+        self.data.ensure(from.0).successors.push(to);
+        self.data.ensure(to).predecessors.push(from);
     }
 
     /// Get the CFG predecessor basic blocks to `ebb`.

lib/cretonne/src/loop_analysis.rs

@@ -20,7 +20,11 @@ entity_impl!(Loop, "loop");
 /// Loops are referenced by the Loop object, and for each loop you can access its header EBB,
 /// its eventual parent in the loop tree and all the EBB belonging to the loop.
 pub struct LoopAnalysis {
+<<<<<<< HEAD
     loops: PrimaryMap<Loop, LoopData>,
+=======
+    loops: EntityMap<Loop, LoopData>,
+>>>>>>> 3ccc9dd9524f3c54e228e8adc565616533ad1696
     ebb_loop_map: EntityMap<Ebb, EbbLoopData>,
 }
 
@@ -40,6 +44,22 @@ impl LoopData {
     }
 }
 
+<<<<<<< HEAD
+=======
+// The loop analysis can split an Ebb that belongs partially to two loops or more.
+// We have to record that because splitting involves incrementally updating the dominator tree,
+// putting it out of sync with a dominator tree recomputed from scratch.
+struct SideEffects {
+    ebb_splitted: bool,
+}
+
+impl SideEffects {
+    fn propagate(&mut self, other: SideEffects) {
+        self.ebb_splitted = self.ebb_splitted || other.ebb_splitted;
+    }
+}
+
+>>>>>>> 3ccc9dd9524f3c54e228e8adc565616533ad1696
 /// If an `Ebb` is part of a loop, then we record two things: the id of the loop it's part of
 /// and the last instruction in the `Ebb` pertaining to the loop. If the `Ebb` is part of multiple
 /// loops, then we make sure by splitting the `Ebb` that it is part of at most two loops, one being
@@ -99,6 +119,34 @@ impl LoopAnalysis {
         }
     }
 
+<<<<<<< HEAD
+=======
+    /// Returns the inner-most loop of which this `Ebb` is a part of.
+    pub fn base_loop_ebb(&self, ebb: Ebb) -> Option<Loop> {
+        self.ebb_loop_map[ebb].loop_id.expand()
+    }
+
+    /// Returns the inner-most loop of which this `Inst` is a part of.
+    pub fn base_loop_inst(&self, inst: Inst, layout: &Layout) -> Option<Loop> {
+        let ebb = layout.inst_ebb(inst).expect("inst should be inserted");
+        let (lp, last_lp_inst) = match self.ebb_loop_map[ebb].loop_id.expand() {
+            None => return None,
+            Some(lp) => (lp, self.ebb_loop_map[ebb].last_inst),
+        };
+        if last_lp_inst.is_none() || layout.cmp(inst, last_lp_inst.unwrap()) != Ordering::Greater {
+            Some(lp)
+        } else {
+            // If the instruction is beyond the inner-most loop limit
+            // then by construction it either in the parent loop or in
+            // no loop at all if the parent loop doesn't exist
+            match self.loop_parent(lp) {
+                None => None,
+                Some(lp_parent) => Some(lp_parent),
+            }
+        }
+    }
+
+>>>>>>> 3ccc9dd9524f3c54e228e8adc565616533ad1696
     /// Determine which region of an `Ebb` belongs to a particular loop.
     ///
     /// Three cases arise:
@@ -136,6 +184,40 @@ impl LoopAnalysis {
         }
         false
     }
+
+    /// Returns the outermost loop of which `lp` is a child of (goes all the way up the loop tree).
+    /// If `limit` is not `None`, returns the outermost loop which is not `limit`. When `limit`
+    /// is a parent of `lp`, the returned loop is a direct child of `limit`.
+    pub fn outermost_loop(&self, lp: Loop, limit: Option<Loop>) -> Loop {
+        let mut finger = Some(lp);
+        let mut parent = lp;
+        while let Some(finger_loop) = finger {
+            match limit {
+                None => parent = finger_loop,
+                Some(limit) => {
+                    if finger_loop == limit {
+                        return parent;
+                    } else {
+                        parent = finger_loop;
+                    }
+                }
+            }
+            finger = self.loop_parent(finger_loop);
+        }
+        parent
+    }
+
+    /// Returns the least common ancestor of two loops in the loop tree, if they share one.
+    pub fn least_common_ancestor(&self, lp1: Loop, lp2: Loop) -> Option<Loop> {
+        let mut finger = Some(lp1);
+        while let Some(finger_loop) = finger {
+            if self.is_child_loop(lp2, finger_loop) {
+                return Some(finger_loop);
+            }
+            finger = self.loop_parent(finger_loop);
+        }
+        None
+    }
 }
 
 impl LoopAnalysis {
@@ -154,7 +236,21 @@ impl LoopAnalysis {
         self.ebb_loop_map.clear();
         self.ebb_loop_map.resize(func.dfg.num_ebbs());
         self.find_loop_headers(cfg, domtree, &func.layout);
+<<<<<<< HEAD
         self.discover_loop_blocks(cfg, domtree, func);
+=======
+        let side_effects = self.discover_loop_blocks(cfg, domtree, func);
+        // During the loop block discovery, the loop analysis can split ebbs. While doing so,
+        // it incrementally updates the dominator tree so that the algorithm can continue its work.
+        // However, the incremental update of the dominator tree breaks the property that if we
+        // recompute the dominator tree from scratch, it will be exactly the same as the one we
+        // have.
+        // So if we have splitted an Ebb we have to recompute it from scratch now, to make sure
+        // it passes verifier::verify_context.
+        if side_effects.ebb_splitted {
+            domtree.compute(func, cfg);
+        }
+>>>>>>> 3ccc9dd9524f3c54e228e8adc565616533ad1696
     }
 
     // Traverses the CFG in reverse postorder and create a loop object for every EBB having a
@@ -183,8 +279,15 @@ impl LoopAnalysis {
     fn discover_loop_blocks(&mut self,
                             cfg: &mut ControlFlowGraph,
                             domtree: &mut DominatorTree,
+<<<<<<< HEAD
                             func: &mut Function) {
         let mut stack: Vec<(Ebb, Inst)> = Vec::new();
+=======
+                            func: &mut Function)
+                            -> SideEffects {
+        let mut stack: Vec<(Ebb, Inst)> = Vec::new();
+        let mut global_side_effects = SideEffects { ebb_splitted: false };
+>>>>>>> 3ccc9dd9524f3c54e228e8adc565616533ad1696
         // We handle each loop header in reverse order, corresponding to a pesudo postorder
         // traversal of the graph.
         for lp in self.loops().rev() {
@@ -196,16 +299,29 @@ impl LoopAnalysis {
             }
             // Then we proceed to discover loop blocks by doing a "reverse" DFS
             while let Some((ebb, loop_edge_inst)) = stack.pop() {
+<<<<<<< HEAD
                 let continue_dfs = self.visit_loop_ebb(lp, ebb, loop_edge_inst, func, domtree, cfg);
+=======
+                let (continue_dfs, side_effects) =
+                    self.visit_loop_ebb(lp, ebb, loop_edge_inst, func, domtree, cfg);
+>>>>>>> 3ccc9dd9524f3c54e228e8adc565616533ad1696
                 // Now we have handled the popped Ebb and need to continue the DFS by adding the
                 // predecessors of that Ebb
                 if let Some(continue_dfs) = continue_dfs {
                     for &(pre, pre_inst) in cfg.get_predecessors(continue_dfs) {
                         stack.push((pre, pre_inst))
                     }
                 }
+<<<<<<< HEAD
             }
         }
+=======
+                // We also propagate the side effects
+                global_side_effects.propagate(side_effects);
+            }
+        }
+        global_side_effects
+>>>>>>> 3ccc9dd9524f3c54e228e8adc565616533ad1696
     }
 
     fn visit_loop_ebb(&mut self,
@@ -215,8 +331,14 @@ impl LoopAnalysis {
                       func: &mut Function,
                       domtree: &mut DominatorTree,
                       cfg: &mut ControlFlowGraph)
+<<<<<<< HEAD
                       -> Option<Ebb> {
         let continue_dfs: Option<Ebb>;
+=======
+                      -> (Option<Ebb>, SideEffects) {
+        let continue_dfs: Option<Ebb>;
+        let mut split_ebb = false;
+>>>>>>> 3ccc9dd9524f3c54e228e8adc565616533ad1696
         match self.ebb_loop_map[ebb].loop_id.expand() {
             None => {
                 // The ebb hasn't been visited yet, we tag it as part of the loop
@@ -323,6 +445,7 @@ impl LoopAnalysis {
                             match self.ebb_loop_map[ebb].last_inst.expand() {
                                 None => (),
                                 Some(last_inner_loop) => {
+<<<<<<< HEAD
                                     if func.layout
                                            .last_inst(ebb)
                                            .map_or(false, |ebb_last_inst| {
@@ -345,13 +468,83 @@ impl LoopAnalysis {
                                                                                 domtree,
                                                                                 cfg);
                                         }
+=======
+                                    let limit =
+                                        match func.layout.cmp(loop_edge_inst, last_inner_loop) {
+                                            Ordering::Greater | Ordering::Equal => loop_edge_inst,
+                                            Ordering::Less => last_inner_loop,
+                                        };
+                                    if func.layout
+                                           .last_inst(ebb)
+                                           .map_or(false,
+                                                   |ebb_last_inst| ebb_last_inst != limit) {
+                                        // This handles the second case
+                                        self.split_ebb_containing_two_loops(ebb,
+                                                                            limit,
+                                                                            lp,
+                                                                            func,
+                                                                            domtree,
+                                                                            cfg);
+                                        split_ebb = true;
+>>>>>>> 3ccc9dd9524f3c54e228e8adc565616533ad1696
                                     }
                                 }
                             };
                         }
                     }
                 }
             }
+<<<<<<< HEAD
+=======
+        }
+        (continue_dfs, SideEffects { ebb_splitted: split_ebb })
+    }
+
+    // We are in the case where `ebb` belongs partially to two different loops, the child and
+    // the parent. `lp` here is the parent loop and we create a new Ebb so that `ebb` belongs
+    // in its entirety to the parent loop.
+    // We also update the cfd and domtree to reflect that.
+    fn split_ebb_containing_two_loops(&mut self,
+                                      ebb: Ebb,
+                                      split_inst: Inst,
+                                      lp: Loop,
+                                      func: &mut Function,
+                                      domtree: &mut DominatorTree,
+                                      cfg: &mut ControlFlowGraph) {
+        if func.layout.inst_ebb(split_inst).unwrap() != ebb {
+            // This is a tricky edge case. Basically this function can be called twice with the
+            // same arguments, in some pathological situations where an ebb can be splitted for
+            // two different reasons and the iterator in visit_loop_ebb is not modified to ignore
+            // the second reason after the ebb split.
+            // Because of that, we simply do nothing the second time.
+            return;
+        }
+        let new_ebb = func.dfg.make_ebb();
+        func.layout.split_ebb(new_ebb, split_inst);
+        let middle_jump_inst = {
+            let cur = &mut Cursor::new(&mut func.layout);
+            cur.goto_bottom(ebb);
+            func.dfg.ins(cur).jump(new_ebb, &[])
+        };
+        *self.ebb_loop_map.ensure(new_ebb) = EbbLoopData {
+            loop_id: lp.into(),
+            last_inst: split_inst.into(),
+        };
+        cfg.recompute_ebb(func, ebb);
+        cfg.recompute_ebb(func, new_ebb);
+        domtree.recompute_split_ebb(ebb, new_ebb, middle_jump_inst);
+    }
+}
+
+impl LoopAnalysis {
+    /// Updates the loop analysis information when a loop pre-header is created.
+    pub fn recompute_loop_preheader(&mut self, pre_header: Ebb, header: Ebb) {
+        let header_lp = self.base_loop_ebb(header)
+            .expect("the header should belong to a loop");
+        *self.ebb_loop_map.ensure(pre_header) = EbbLoopData {
+            loop_id: self.loop_parent(header_lp).into(),
+            last_inst: None.into(),
+>>>>>>> 3ccc9dd9524f3c54e228e8adc565616533ad1696
         }
         continue_dfs
     }
@@ -390,8 +583,12 @@ impl LoopAnalysis {
 #[cfg(test)]
 mod test {
 
+<<<<<<< HEAD
     use ir::{Function, InstBuilder, Cursor, CursorBase, types};
     use ir::entities::Ebb;
+=======
+    use ir::{Function, InstBuilder, Cursor, CursorBase, Ebb, types};
+>>>>>>> 3ccc9dd9524f3c54e228e8adc565616533ad1696
     use loop_analysis::{Loop, LoopAnalysis};
     use flowgraph::ControlFlowGraph;
     use dominator_tree::DominatorTree;

lib/cretonne/src/verifier/mod.rs

@@ -464,7 +464,9 @@ impl<'a> Verifier<'a> {
         // 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");
+                        "incorrect number of Ebbs in postorder traversal: {}, should be {}",
+                        domtree.cfg_postorder().len(),
+                        self.domtree.cfg_postorder().len());
         }
         for (index, (&true_ebb, &test_ebb)) in
             self.domtree