Make InstructionData comparable and hashable (#388)

* Don't implement Eq and Hash for EntityList

* Generate eq and hash methods for InstructionData

* Use the eq() and hash() methods of InstructionData in simple_gvn

lib/codegen/src/simple_gvn.rs

@@ -4,6 +4,8 @@ use cursor::{Cursor, FuncCursor};
 use dominator_tree::DominatorTree;
 use ir::{Function, Inst, InstructionData, Opcode, Type};
 use scoped_hash_map::ScopedHashMap;
+use std::cell::{Ref, RefCell};
+use std::hash::{Hash, Hasher};
 use std::vec::Vec;
 use timing;
 
@@ -20,64 +22,103 @@ fn trivially_unsafe_for_gvn(opcode: Opcode) -> bool {
         || opcode.writes_cpu_flags()
 }
 
+/// Wrapper around `InstructionData` which implements `Eq` and `Hash`
+#[derive(Clone)]
+struct HashKey<'a, 'f: 'a> {
+    inst: InstructionData,
+    ty: Type,
+    pos: &'a RefCell<FuncCursor<'f>>,
+}
+impl<'a, 'f: 'a> Hash for HashKey<'a, 'f> {
+    fn hash<H: Hasher>(&self, state: &mut H) {
+        let pool = &self.pos.borrow().func.dfg.value_lists;
+        self.inst.hash(state, pool);
+        self.ty.hash(state);
+    }
+}
+impl<'a, 'f: 'a> PartialEq for HashKey<'a, 'f> {
+    fn eq(&self, other: &Self) -> bool {
+        let pool = &self.pos.borrow().func.dfg.value_lists;
+        self.inst.eq(&other.inst, pool) && self.ty == other.ty
+    }
+}
+impl<'a, 'f: 'a> Eq for HashKey<'a, 'f> {}
+
 /// Perform simple GVN on `func`.
 ///
 pub fn do_simple_gvn(func: &mut Function, domtree: &mut DominatorTree) {
     let _tt = timing::gvn();
     debug_assert!(domtree.is_valid());
 
-    let mut visible_values: ScopedHashMap<(InstructionData, Type), Inst> = ScopedHashMap::new();
+    // Visit EBBs in a reverse post-order.
+    //
+    // The RefCell here is a bit ugly since the HashKeys in the ScopedHashMap
+    // need a reference to the function.
+    let pos = RefCell::new(FuncCursor::new(func));
+
+    let mut visible_values: ScopedHashMap<HashKey, Inst> = ScopedHashMap::new();
     let mut scope_stack: Vec<Inst> = Vec::new();
 
-    // Visit EBBs in a reverse post-order.
-    let mut pos = FuncCursor::new(func);
-
     for &ebb in domtree.cfg_postorder().iter().rev() {
-        // Pop any scopes that we just exited.
-        loop {
-            if let Some(current) = scope_stack.last() {
-                if domtree.dominates(*current, ebb, &pos.func.layout) {
+        {
+            // Pop any scopes that we just exited.
+            let layout = &pos.borrow().func.layout;
+            loop {
+                if let Some(current) = scope_stack.last() {
+                    if domtree.dominates(*current, ebb, layout) {
+                        break;
+                    }
+                } else {
                     break;
                 }
-            } else {
-                break;
+                scope_stack.pop();
+                visible_values.decrement_depth();
             }
-            scope_stack.pop();
-            visible_values.decrement_depth();
+
+            // Push a scope for the current block.
+            scope_stack.push(layout.first_inst(ebb).unwrap());
+            visible_values.increment_depth();
         }
 
-        // Push a scope for the current block.
-        scope_stack.push(pos.func.layout.first_inst(ebb).unwrap());
-        visible_values.increment_depth();
-
-        pos.goto_top(ebb);
-        while let Some(inst) = pos.next_inst() {
+        pos.borrow_mut().goto_top(ebb);
+        while let Some(inst) = {
+            let mut pos = pos.borrow_mut();
+            pos.next_inst()
+        } {
             // Resolve aliases, particularly aliases we created earlier.
-            pos.func.dfg.resolve_aliases_in_arguments(inst);
+            pos.borrow_mut().func.dfg.resolve_aliases_in_arguments(inst);
 
-            let opcode = pos.func.dfg[inst].opcode();
+            let func = Ref::map(pos.borrow(), |pos| &pos.func);
+
+            let opcode = func.dfg[inst].opcode();
             if opcode.is_branch() && !opcode.is_terminator() {
-                scope_stack.push(pos.func.layout.next_inst(inst).unwrap());
+                scope_stack.push(func.layout.next_inst(inst).unwrap());
                 visible_values.increment_depth();
             }
             if trivially_unsafe_for_gvn(opcode) {
                 continue;
             }
 
-            let ctrl_typevar = pos.func.dfg.ctrl_typevar(inst);
-            let key = (pos.func.dfg[inst].clone(), ctrl_typevar);
+            let ctrl_typevar = func.dfg.ctrl_typevar(inst);
+            let key = HashKey {
+                inst: func.dfg[inst].clone(),
+                ty: ctrl_typevar,
+                pos: &pos,
+            };
             let entry = visible_values.entry(key);
             use scoped_hash_map::Entry::*;
             match entry {
                 Occupied(entry) => {
-                    debug_assert!(domtree.dominates(*entry.get(), inst, &pos.func.layout));
+                    debug_assert!(domtree.dominates(*entry.get(), inst, &func.layout));
                     // If the redundant instruction is representing the current
                     // scope, pick a new representative.
                     let old = scope_stack.last_mut().unwrap();
                     if *old == inst {
-                        *old = pos.func.layout.next_inst(inst).unwrap();
+                        *old = func.layout.next_inst(inst).unwrap();
                     }
                     // Replace the redundant instruction and remove it.
+                    drop(func);
+                    let mut pos = pos.borrow_mut();
                     pos.func.dfg.replace_with_aliases(inst, *entry.get());
                     pos.remove_inst_and_step_back();
                 }