Change EbbHeaderBlockData's predecessors list from a HashMap to a Vec. (#148)

In addition to efficiency, this change also eliminates some nondeterminsm
resulting from HashMap key ordering.

lib/frontend/src/frontend.rs

@@ -7,6 +7,7 @@ use cretonne::ir::function::DisplayFunction;
 use cretonne::isa::TargetIsa;
 use ssa::{SSABuilder, SideEffects, Block};
 use cretonne::entity::{EntityRef, EntityMap};
+use std::collections::HashSet;
 use std::hash::Hash;
 
 /// Permanent structure used for translating into Cretonne IL.
@@ -146,6 +147,10 @@ impl<'short, 'long, Variable> InstBuilderBase<'short> for FuncInstBuilder<'short
                         // If jump table we declare all entries successor
                         // TODO: not collect with vector?
                         InstructionData::BranchTable { table, .. } => {
+                            // Unlike all other jumps/branches, jump tables are
+                            // capable of having the same successor appear
+                            // multiple times. Use a HashSet to deduplicate.
+                            let mut unique = HashSet::new();
                             for dest_ebb in self.builder
                                     .func
                                     .jump_tables
@@ -153,6 +158,7 @@ impl<'short, 'long, Variable> InstBuilderBase<'short> for FuncInstBuilder<'short
                                     .expect("you are referencing an undeclared jump table")
                                     .entries()
                                     .map(|(_, ebb)| ebb)
+                                    .filter(|dest_ebb| unique.insert(*dest_ebb))
                                     .collect::<Vec<Ebb>>() {
                                 self.builder.declare_successor(dest_ebb, inst)
                             }

lib/frontend/src/ssa.rs

@@ -71,7 +71,7 @@ impl<Variable> BlockData<Variable> {
         match self {
             &mut BlockData::EbbBody { .. } => panic!("you can't add a predecessor to a body block"),
             &mut BlockData::EbbHeader(ref mut data) => {
-                data.predecessors.insert(pred, inst);
+                data.predecessors.push((pred, inst));
             }
         }
     }
@@ -81,14 +81,12 @@ impl<Variable> BlockData<Variable> {
             &mut BlockData::EbbHeader(ref mut data) => {
                 // This a linear complexity operation but the number of predecessors is low
                 // in all non-pathological cases
-                let pred: Block = match data.predecessors
+                let pred: usize =
+                    data.predecessors
                         .iter()
-                          .find(|&(_, &jump_inst)| jump_inst == inst) {
+                        .position(|pair| pair.1 == inst)
-                    None => panic!("the predecessor you are trying to remove is not declared"),
+                        .expect("the predecessor you are trying to remove is not declared");
-                    Some((&b, _)) => b.clone(),
+                data.predecessors.swap_remove(pred).0
-                };
-                data.predecessors.remove(&pred);
-                pred
             }
         }
     }
@@ -96,7 +94,7 @@ impl<Variable> BlockData<Variable> {
 
 struct EbbHeaderBlockData<Variable> {
     // The predecessors of the Ebb header block, with the block and branch instruction.
-    predecessors: HashMap<Block, Inst>,
+    predecessors: Vec<(Block, Inst)>,
     // A ebb header block is sealed if all of its predecessors have been declared.
     sealed: bool,
     // The ebb which this block is part of.
@@ -227,13 +225,10 @@ impl<Variable> SSABuilder<Variable>
                 if data.sealed {
                     if data.predecessors.len() == 1 {
                         // Only one predecessor, straightforward case
-                        UseVarCases::SealedOnePredecessor(*data.predecessors.keys().next().unwrap())
+                        UseVarCases::SealedOnePredecessor(data.predecessors[0].0)
                     } else {
                         let val = dfg.append_ebb_arg(data.ebb, ty);
-                        let preds = data.predecessors
+                        let preds = data.predecessors.clone();
-                            .iter()
-                            .map(|(&pred, &inst)| (pred, inst))
-                            .collect();
                         UseVarCases::SealedMultiplePredecessors(preds, val, data.ebb)
                     }
                 } else {
@@ -287,7 +282,7 @@ impl<Variable> SSABuilder<Variable>
     pub fn declare_ebb_header_block(&mut self, ebb: Ebb) -> Block {
         let block = self.blocks
             .push(BlockData::EbbHeader(EbbHeaderBlockData {
-                                           predecessors: HashMap::new(),
+                                           predecessors: Vec::new(),
                                            sealed: false,
                                            ebb: ebb,
                                            undef_variables: Vec::new(),
@@ -311,6 +306,9 @@ impl<Variable> SSABuilder<Variable>
     /// Note that the predecessor is a `Block` and not an `Ebb`. This `Block` must be filled
     /// before added as predecessor. Note that you must provide no jump arguments to the branch
     /// instruction when you create it since `SSABuilder` will fill them for you.
+    ///
+    /// Callers are expected to avoid adding the same predecessor more than once in the case
+    /// of a jump table.
     pub fn declare_ebb_predecessor(&mut self, ebb: Ebb, pred: Block, inst: Inst) {
         let header_block = match self.blocks[self.header_block(ebb)] {
             BlockData::EbbBody { .. } => panic!("you can't add predecessors to an Ebb body block"),
@@ -385,9 +383,7 @@ impl<Variable> SSABuilder<Variable>
                                               Ebb) = match self.blocks[block] {
             BlockData::EbbBody { .. } => panic!("this should not happen"),
             BlockData::EbbHeader(ref mut data) => {
-                (data.predecessors.iter().map(|(&x, &y)| (x, y)).collect(),
+                (data.predecessors.clone(), data.undef_variables.clone(), data.ebb)
-                 data.undef_variables.clone(),
-                 data.ebb)
             }
         };
 
@@ -577,7 +573,7 @@ impl<Variable> SSABuilder<Variable>
     }
 
     /// Returns the list of `Ebb`s that have been declared as predecessors of the argument.
-    pub fn predecessors(&self, ebb: Ebb) -> &HashMap<Block, Inst> {
+    pub fn predecessors(&self, ebb: Ebb) -> &[(Block, Inst)] {
         let block = match self.ebb_headers[ebb].expand() {
             Some(block) => block,
             None => panic!("the ebb has not been declared yet"),