Use ScopedHashMap in simple_gvn.

This avoids effectively ending up with most of a function body
stored in the hash map at once by removing elements promptly when
they go out of scope.

cranelift/filetests/simple_gvn/scopes.cton

@@ -0,0 +1,76 @@
+test simple-gvn
+
+function %two_diamonds(i32, i32, i32, i32, i32) {
+ebb0(v0: i32, v1: i32, v2: i32, v3: i32, v4: i32):
+    v5 = iconst.i32 16
+    ; check: v5 = iconst.i32 16
+    brz v0, ebb1
+    v6 = iconst.i32 17
+    ; check: v6 = iconst.i32 17
+    v7 = iconst.i32 16
+    ; not: v7 = iconst.i32 16
+    jump ebb2
+
+ebb1:
+    v8 = iconst.i32 18
+    ; check: v8 = iconst.i32 18
+    v9 = iconst.i32 17
+    ; check: v9 = iconst.i32 17
+    v10 = iconst.i32 16
+    ; not: v10 = iconst.i32 16
+    jump ebb2
+
+ebb2:
+    v11 = iconst.i32 19
+    ; check: v11 = iconst.i32 19
+    v12 = iconst.i32 18
+    ; check: v12 = iconst.i32 18
+    v13 = iconst.i32 17
+    ; check: v13 = iconst.i32 17
+    v14 = iconst.i32 16
+    ; not: v14 = iconst.i32 16
+    brz v1, ebb3
+    v15 = iconst.i32 20
+    ; check: v15 = iconst.i32 20
+    v16 = iconst.i32 19
+    ; not: v16 = iconst.i32 19
+    v17 = iconst.i32 18
+    ; not: v17 = iconst.i32 18
+    v18 = iconst.i32 17
+    ; not: v18 = iconst.i32 17
+    v19 = iconst.i32 16
+    ; not: v19 = iconst.i32 16
+    jump ebb4
+
+ebb3:
+    v20 = iconst.i32 21
+    ; check: v20 = iconst.i32 21
+    v21 = iconst.i32 20
+    ; check: v21 = iconst.i32 20
+    v22 = iconst.i32 19
+    ; not: v22 = iconst.i32 19
+    v23 = iconst.i32 18
+    ; not: v23 = iconst.i32 18
+    v24 = iconst.i32 17
+    ; not: v24 = iconst.i32 17
+    v25 = iconst.i32 16
+    ; not: v25 = iconst.i32 16
+    jump ebb4
+
+ebb4:
+    v26 = iconst.i32 22
+    ; check: v26 = iconst.i32 22
+    v27 = iconst.i32 21
+    ; check: v27 = iconst.i32 21
+    v28 = iconst.i32 20
+    ; check: v28 = iconst.i32 20
+    v29 = iconst.i32 19
+    ; not: v29 = iconst.i32 19
+    v30 = iconst.i32 18
+    ; not: v30 = iconst.i32 18
+    v31 = iconst.i32 17
+    ; not: v31 = iconst.i32 17
+    v32 = iconst.i32 16
+    ; not: v32 = iconst.i32 16
+    return
+}

lib/cretonne/src/simple_gvn.rs

@@ -3,7 +3,7 @@
 use flowgraph::ControlFlowGraph;
 use dominator_tree::DominatorTree;
 use ir::{Cursor, CursorBase, InstructionData, Function, Inst, Opcode, Type};
-use std::collections::HashMap;
+use scoped_hash_map::ScopedHashMap;
 
 /// Test whether the given opcode is unsafe to even consider for GVN.
 fn trivially_unsafe_for_gvn(opcode: Opcode) -> bool {
@@ -18,19 +18,40 @@ pub fn do_simple_gvn(func: &mut Function, cfg: &mut ControlFlowGraph, domtree: &
     debug_assert!(cfg.is_valid());
     debug_assert!(domtree.is_valid());
 
-    let mut visible_values: HashMap<(InstructionData, Type), Inst> = HashMap::new();
+    let mut visible_values: ScopedHashMap<(InstructionData, Type), Inst> = ScopedHashMap::new();
+    let mut scope_stack: Vec<Inst> = Vec::new();
 
     // Visit EBBs in a reverse post-order.
     let mut pos = Cursor::new(&mut func.layout);
 
     for &ebb in domtree.cfg_postorder().iter().rev() {
-        pos.goto_top(ebb);
+        // Pop any scopes that we just exited.
+        loop {
+            if let Some(current) = scope_stack.last() {
+                if domtree.dominates(*current, ebb, &pos.layout) {
+                    break;
+                }
+            } else {
+                break;
+            }
+            scope_stack.pop();
+            visible_values.decrement_depth();
+        }
 
+        // Push a scope for the current block.
+        scope_stack.push(pos.layout.first_inst(ebb).unwrap());
+        visible_values.increment_depth();
+
+        pos.goto_top(ebb);
         while let Some(inst) = pos.next_inst() {
             // Resolve aliases, particularly aliases we created earlier.
             func.dfg.resolve_aliases_in_arguments(inst);
 
             let opcode = func.dfg[inst].opcode();
+            if opcode.is_branch() && !opcode.is_terminator() {
+                scope_stack.push(pos.layout.next_inst(inst).unwrap());
+                visible_values.increment_depth();
+            }
             if trivially_unsafe_for_gvn(opcode) {
                 continue;
             }
@@ -38,19 +59,19 @@ pub fn do_simple_gvn(func: &mut Function, cfg: &mut ControlFlowGraph, domtree: &
             let ctrl_typevar = func.dfg.ctrl_typevar(inst);
             let key = (func.dfg[inst].clone(), ctrl_typevar);
             let entry = visible_values.entry(key);
-            use std::collections::hash_map::Entry::*;
+            use scoped_hash_map::Entry::*;
             match entry {
-                Occupied(mut entry) => {
+                Occupied(entry) => {
-                    if domtree.dominates(*entry.get(), inst, pos.layout) {
+                    debug_assert!(domtree.dominates(*entry.get(), inst, pos.layout));
-                        func.dfg.replace_with_aliases(inst, *entry.get());
+                    // If the redundant instruction is representing the current
-                        pos.remove_inst_and_step_back();
+                    // scope, pick a new representative.
-                    } else {
+                    let old = scope_stack.last_mut().unwrap();
-                        // The prior instruction doesn't dominate inst, so it
+                    if *old == inst {
-                        // won't dominate any subsequent instructions we'll
+                        *old = pos.layout.next_inst(inst).unwrap();
-                        // visit, so just replace it.
-                        *entry.get_mut() = inst;
-                        continue;
                     }
+                    // Replace the redundant instruction and remove it.
+                    func.dfg.replace_with_aliases(inst, *entry.get());
+                    pos.remove_inst_and_step_back();
                 }
                 Vacant(entry) => {
                     entry.insert(inst);