Expose CFG predecessors only as an iterator.

Define two public iterator types in the flowgraph module, PredIter and
SuccIter, which are by-value iterators over an EBB's predecessors and
successors respectively.

Provide matching pred_iter() and succ_iter() methods for inspecting the
CFG. Remove the get_predecessors() method which returned a slice.

Update the uses of get_predecessors(), none of which depended on it
being a slice.

This abstraction makes it possible to change the internal representation
of the CFG.

cranelift/src/print_cfg.rs

@@ -76,7 +76,7 @@ impl<'a> CFGPrinter<'a> {
 
     fn cfg_connections(&self, w: &mut Write) -> Result {
         for ebb in &self.func.layout {
-            for &(parent, inst) in self.cfg.get_predecessors(ebb) {
+            for (parent, inst) in self.cfg.pred_iter(ebb) {
                 writeln!(w, "    {}:{} -> {}", parent, inst, ebb)?;
             }
         }

lib/cretonne/src/dominator_tree.rs

@@ -415,11 +415,9 @@ impl DominatorTree {
         // Get an iterator with just the reachable, already visited predecessors to `ebb`.
         // Note that during the first pass, `rpo_number` is 1 for reachable blocks that haven't
         // been visited yet, 0 for unreachable blocks.
-        let mut reachable_preds = cfg.get_predecessors(ebb).iter().cloned().filter(
+        let mut reachable_preds = cfg.pred_iter(ebb).filter(|&(pred, _)| {
-            |&(pred, _)| {
             self.nodes[pred].rpo_number > 1
-            },
+        });
-        );
 
         // The RPO must visit at least one predecessor before this node.
         let mut idom = reachable_preds.next().expect(

lib/cretonne/src/flowgraph.rs

@@ -28,6 +28,7 @@ use ir::{Function, Inst, Ebb};
 use ir::instructions::BranchInfo;
 use entity::EntityMap;
 use std::mem;
+use std::slice;
 
 /// A basic block denoted by its enclosing Ebb and last instruction.
 pub type BasicBlock = (Ebb, Inst);
@@ -138,14 +139,13 @@ impl ControlFlowGraph {
         self.data[to].predecessors.push(from);
     }
 
-    /// Get the CFG predecessor basic blocks to `ebb`.
+    /// Get an iterator over the CFG predecessors to `ebb`.
-    pub fn get_predecessors(&self, ebb: Ebb) -> &[BasicBlock] {
+    pub fn pred_iter(&self, ebb: Ebb) -> PredIter {
-        debug_assert!(self.is_valid());
+        PredIter(self.data[ebb].predecessors.iter())
-        &self.data[ebb].predecessors
     }
 
     /// Get an iterator over the CFG successors to `ebb`.
-    pub fn succ_iter(&self, ebb: Ebb) -> bforest::SetIter<Ebb, ()> {
+    pub fn succ_iter(&self, ebb: Ebb) -> SuccIter {
         debug_assert!(self.is_valid());
         self.data[ebb].successors.iter(&self.succ_forest)
     }
@@ -160,6 +160,22 @@ impl ControlFlowGraph {
     }
 }
 
+/// An iterator over EBB predecessors. The iterator type is `BasicBlock`.
+///
+/// Each predecessor is an instruction that branches to the EBB.
+pub struct PredIter<'a>(slice::Iter<'a, BasicBlock>);
+
+impl<'a> Iterator for PredIter<'a> {
+    type Item = BasicBlock;
+
+    fn next(&mut self) -> Option<BasicBlock> {
+        self.0.next().cloned()
+    }
+}
+
+/// An iterator over EBB successors. The iterator type is `Ebb`.
+pub type SuccIter<'a> = bforest::SetIter<'a, Ebb, ()>;
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -187,7 +203,7 @@ mod tests {
         let mut fun_ebbs = func.layout.ebbs();
         for ebb in func.layout.ebbs() {
             assert_eq!(ebb, fun_ebbs.next().unwrap());
-            assert_eq!(cfg.get_predecessors(ebb).len(), 0);
+            assert_eq!(cfg.pred_iter(ebb).count(), 0);
             assert_eq!(cfg.succ_iter(ebb).count(), 0);
         }
     }
@@ -222,13 +238,13 @@ mod tests {
         let mut cfg = ControlFlowGraph::with_function(&func);
 
         {
-            let ebb0_predecessors = cfg.get_predecessors(ebb0);
+            let ebb0_predecessors = cfg.pred_iter(ebb0).collect::<Vec<_>>();
-            let ebb1_predecessors = cfg.get_predecessors(ebb1);
+            let ebb1_predecessors = cfg.pred_iter(ebb1).collect::<Vec<_>>();
-            let ebb2_predecessors = cfg.get_predecessors(ebb2);
+            let ebb2_predecessors = cfg.pred_iter(ebb2).collect::<Vec<_>>();
 
-            let ebb0_successors = cfg.succ_iter(ebb0);
+            let ebb0_successors = cfg.succ_iter(ebb0).collect::<Vec<_>>();
-            let ebb1_successors = cfg.succ_iter(ebb1);
+            let ebb1_successors = cfg.succ_iter(ebb1).collect::<Vec<_>>();
-            let ebb2_successors = cfg.succ_iter(ebb2);
+            let ebb2_successors = cfg.succ_iter(ebb2).collect::<Vec<_>>();
 
             assert_eq!(ebb0_predecessors.len(), 0);
             assert_eq!(ebb1_predecessors.len(), 2);
@@ -239,9 +255,9 @@ mod tests {
             assert_eq!(ebb2_predecessors.contains(&(ebb0, br_ebb0_ebb2)), true);
             assert_eq!(ebb2_predecessors.contains(&(ebb1, jmp_ebb1_ebb2)), true);
 
-            assert_eq!(ebb0_successors.collect::<Vec<_>>(), [ebb1, ebb2]);
+            assert_eq!(ebb0_successors, [ebb1, ebb2]);
-            assert_eq!(ebb1_successors.collect::<Vec<_>>(), [ebb1, ebb2]);
+            assert_eq!(ebb1_successors, [ebb1, ebb2]);
-            assert_eq!(ebb2_successors.collect::<Vec<_>>(), []);
+            assert_eq!(ebb2_successors, []);
         }
 
         // Change some instructions and recompute ebb0
@@ -251,9 +267,9 @@ mod tests {
         let br_ebb0_ebb1 = br_ebb0_ebb2;
 
         {
-            let ebb0_predecessors = cfg.get_predecessors(ebb0);
+            let ebb0_predecessors = cfg.pred_iter(ebb0).collect::<Vec<_>>();
-            let ebb1_predecessors = cfg.get_predecessors(ebb1);
+            let ebb1_predecessors = cfg.pred_iter(ebb1).collect::<Vec<_>>();
-            let ebb2_predecessors = cfg.get_predecessors(ebb2);
+            let ebb2_predecessors = cfg.pred_iter(ebb2).collect::<Vec<_>>();
 
             let ebb0_successors = cfg.succ_iter(ebb0);
             let ebb1_successors = cfg.succ_iter(ebb1);

lib/cretonne/src/legalizer/split.rs

@@ -125,7 +125,7 @@ fn split_any(
 
     // We have split the value requested, and now we may need to fix some EBB predecessors.
     while let Some(repair) = repairs.pop() {
-        for &(_, inst) in cfg.get_predecessors(repair.ebb) {
+        for (_, inst) in cfg.pred_iter(repair.ebb) {
             let branch_opc = pos.func.dfg[inst].opcode();
             assert!(
                 branch_opc.is_branch(),

lib/cretonne/src/licm.rs

@@ -74,7 +74,7 @@ fn create_pre_header(
     for typ in header_args_types {
         pre_header_args_value.push(func.dfg.append_ebb_param(pre_header, typ), pool);
     }
-    for &(_, last_inst) in cfg.get_predecessors(header) {
+    for (_, last_inst) in cfg.pred_iter(header) {
         // We only follow normal edges (not the back edges)
         if !domtree.dominates(header, last_inst, &func.layout) {
             change_branch_jump_destination(last_inst, pre_header, func);
@@ -103,7 +103,7 @@ fn has_pre_header(
 ) -> Option<(Ebb, Inst)> {
     let mut result = None;
     let mut found = false;
-    for &(pred_ebb, last_inst) in cfg.get_predecessors(header) {
+    for (pred_ebb, last_inst) in cfg.pred_iter(header) {
         // We only count normal edges (not the back edges)
         if !domtree.dominates(header, last_inst, layout) {
             if found {

lib/cretonne/src/loop_analysis.rs

@@ -134,7 +134,7 @@ impl LoopAnalysis {
     ) {
         // We traverse the CFG in reverse postorder
         for &ebb in domtree.cfg_postorder().iter().rev() {
-            for &(_, pred_inst) in cfg.get_predecessors(ebb) {
+            for (_, pred_inst) in cfg.pred_iter(ebb) {
                 // If the ebb dominates one of its predecessors it is a back edge
                 if domtree.dominates(ebb, pred_inst, layout) {
                     // This ebb is a loop header, so we create its associated loop
@@ -160,7 +160,7 @@ impl LoopAnalysis {
         // We handle each loop header in reverse order, corresponding to a pseudo postorder
         // traversal of the graph.
         for lp in self.loops().rev() {
-            for &(pred, pred_inst) in cfg.get_predecessors(self.loops[lp].header) {
+            for (pred, pred_inst) in cfg.pred_iter(self.loops[lp].header) {
                 // We follow the back edges
                 if domtree.dominates(self.loops[lp].header, pred_inst, layout) {
                     stack.push(pred);
@@ -210,7 +210,7 @@ impl LoopAnalysis {
                 // Now we have handled the popped node and need to continue the DFS by adding the
                 // predecessors of that node
                 if let Some(continue_dfs) = continue_dfs {
-                    for &(pred, _) in cfg.get_predecessors(continue_dfs) {
+                    for (pred, _) in cfg.pred_iter(continue_dfs) {
                         stack.push(pred)
                     }
                 }

lib/cretonne/src/regalloc/coalescing.rs

@@ -8,7 +8,7 @@
 use cursor::{Cursor, EncCursor};
 use dbg::DisplayList;
 use dominator_tree::DominatorTree;
-use flowgraph::{ControlFlowGraph, BasicBlock};
+use flowgraph::ControlFlowGraph;
 use ir::{DataFlowGraph, Layout, InstBuilder, ValueDef};
 use ir::{Function, Ebb, Inst, Value, ExpandedProgramPoint};
 use regalloc::affinity::Affinity;
@@ -246,6 +246,7 @@ struct Context<'a> {
     encinfo: EncInfo,
 
     func: &'a mut Function,
+    cfg: &'a ControlFlowGraph,
     domtree: &'a DominatorTree,
     liveness: &'a mut Liveness,
     virtregs: &'a mut VirtRegs,
@@ -288,6 +289,7 @@ impl Coalescing {
             isa,
             encinfo: isa.encoding_info(),
             func,
+            cfg,
             domtree,
             liveness,
             virtregs,
@@ -299,11 +301,8 @@ impl Coalescing {
         // TODO: The iteration order matters here. We should coalesce in the most important blocks
         // first, so they get first pick at forming virtual registers.
         for &ebb in domtree.cfg_postorder() {
-            let preds = cfg.get_predecessors(ebb);
-            if !preds.is_empty() {
             for argnum in 0..context.func.dfg.num_ebb_params(ebb) {
-                    context.coalesce_ebb_param(ebb, argnum, preds)
+                context.coalesce_ebb_param(ebb, argnum)
-                }
             }
         }
     }
@@ -311,7 +310,7 @@ impl Coalescing {
 
 impl<'a> Context<'a> {
     /// Coalesce the `argnum`'th parameter on `ebb`.
-    fn coalesce_ebb_param(&mut self, ebb: Ebb, argnum: usize, preds: &[BasicBlock]) {
+    fn coalesce_ebb_param(&mut self, ebb: Ebb, argnum: usize) {
         self.split_values.clear();
         let mut succ_val = self.func.dfg.ebb_params(ebb)[argnum];
         dbg!("Processing {}/{}: {}", ebb, argnum, succ_val);
@@ -342,7 +341,7 @@ impl<'a> Context<'a> {
         // A successor copy is always required if the `succ_val` virtual register is live at
         // any predecessor branch.
 
-        while let Some(bad_value) = self.try_coalesce(argnum, succ_val, preds) {
+        while let Some(bad_value) = self.try_coalesce(argnum, succ_val, ebb) {
             dbg!("Isolating interfering value {}", bad_value);
             // The bad value has some conflict that can only be reconciled by excluding its
             // congruence class from the new virtual register.
@@ -363,7 +362,7 @@ impl<'a> Context<'a> {
             }
 
             // Check the predecessors.
-            for &(pred_ebb, pred_inst) in preds {
+            for (pred_ebb, pred_inst) in self.cfg.pred_iter(ebb) {
                 let pred_val = self.func.dfg.inst_variable_args(pred_inst)[argnum];
                 if self.virtregs.same_class(bad_value, pred_val) {
                     self.split_pred(pred_inst, pred_ebb, argnum, pred_val);
@@ -404,12 +403,7 @@ impl<'a> Context<'a> {
     ///
     /// Returns a value from a congruence class that needs to be split before starting over, or
     /// `None` if everything was successfully coalesced into `self.values`.
-    fn try_coalesce(
+    fn try_coalesce(&mut self, argnum: usize, succ_val: Value, succ_ebb: Ebb) -> Option<Value> {
-        &mut self,
-        argnum: usize,
-        succ_val: Value,
-        preds: &[BasicBlock],
-    ) -> Option<Value> {
         // Initialize the value list with the split values. These are guaranteed to be
         // interference free, and anything that interferes with them must be split away.
         self.reset_values();
@@ -421,7 +415,7 @@ impl<'a> Context<'a> {
             return Some(succ_val);
         }
 
-        for &(pred_ebb, pred_inst) in preds {
+        for (pred_ebb, pred_inst) in self.cfg.pred_iter(succ_ebb) {
             let pred_val = self.func.dfg.inst_variable_args(pred_inst)[argnum];
             dbg!(
                 "Checking {}: {}: {}",

lib/cretonne/src/regalloc/liveness.rs

@@ -269,7 +269,7 @@ fn extend_to_use(
     while let Some(livein) = worklist.pop() {
         // We've learned that the value needs to be live-in to the `livein` EBB.
         // Make sure it is also live at all predecessor branches to `livein`.
-        for &(pred, branch) in cfg.get_predecessors(livein) {
+        for (pred, branch) in cfg.pred_iter(livein) {
             if lr.extend_in_ebb(pred, branch, &func.layout) {
                 // This predecessor EBB also became live-in. We need to process it later.
                 worklist.push(pred);
@@ -463,7 +463,7 @@ impl Liveness {
                         _ => continue,
                     };
 
-                    for &(_, pred_branch) in cfg.get_predecessors(succ_ebb) {
+                    for (_, pred_branch) in cfg.pred_iter(succ_ebb) {
                         let pred_arg = func.dfg.inst_variable_args(pred_branch)[num];
                         let pred_affinity = &mut self.ranges.get_mut(pred_arg).unwrap().affinity;
                         if pred_affinity.is_none() {

lib/cretonne/src/verifier/cssa.rs

@@ -102,7 +102,7 @@ impl<'a> CssaVerifier<'a> {
     fn check_cssa(&self) -> Result {
         for ebb in self.func.layout.ebbs() {
             let ebb_params = self.func.dfg.ebb_params(ebb);
-            for &(_, pred) in self.cfg.get_predecessors(ebb) {
+            for (_, pred) in self.cfg.pred_iter(ebb) {
                 let pred_args = self.func.dfg.inst_variable_args(pred);
                 // This should have been caught by an earlier verifier pass.
                 assert_eq!(

lib/cretonne/src/verifier/flags.rs

@@ -60,7 +60,7 @@ impl<'a> FlagsVerifier<'a> {
                     // Revisit any predecessor blocks the first time we see a live-in for `ebb`.
                     None => {
                         self.livein[ebb] = value.into();
-                        for &(pred, _) in self.cfg.get_predecessors(ebb) {
+                        for (pred, _) in self.cfg.pred_iter(ebb) {
                             worklist.insert(pred);
                         }
                     }

lib/cretonne/src/verifier/liveness.rs

@@ -226,7 +226,7 @@ impl<'a> LivenessVerifier<'a> {
             // Check all the EBBs in the interval independently.
             loop {
                 // If `val` is live-in at `ebb`, it must be live at all the predecessors.
-                for &(_, pred) in self.cfg.get_predecessors(ebb) {
+                for (_, pred) in self.cfg.pred_iter(ebb) {
                     if !self.live_at_use(lr, pred) {
                         return err!(
                             pred,

lib/cretonne/src/verifier/mod.rs

@@ -933,9 +933,8 @@ impl<'a> Verifier<'a> {
                 return err!(ebb, "cfg had unexpected successor(s) {:?}", excess_succs);
             }
 
-            expected_preds.extend(self.expected_cfg.get_predecessors(ebb).iter().map(|&(_,
+            expected_preds.extend(self.expected_cfg.pred_iter(ebb).map(|(_, inst)| inst));
-               inst)| inst));
+            got_preds.extend(cfg.pred_iter(ebb).map(|(_, inst)| inst));
-            got_preds.extend(cfg.get_predecessors(ebb).iter().map(|&(_, inst)| inst));
 
             let missing_preds: Vec<Inst> = expected_preds.difference(&got_preds).cloned().collect();
             if !missing_preds.is_empty() {