From ce94a3fa3918373f742e23030ba1232cc252f573 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Tue, 19 Sep 2017 21:05:53 -0700 Subject: [PATCH] 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 | 76 ++++++++++++++++++++++ lib/cretonne/src/simple_gvn.rs | 49 ++++++++++---- 2 files changed, 111 insertions(+), 14 deletions(-) create mode 100644 cranelift/filetests/simple_gvn/scopes.cton diff --git a/cranelift/filetests/simple_gvn/scopes.cton b/cranelift/filetests/simple_gvn/scopes.cton new file mode 100644 index 0000000000..85ea5583c2 --- /dev/null +++ b/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 +} diff --git a/lib/cretonne/src/simple_gvn.rs b/lib/cretonne/src/simple_gvn.rs index 20e2298440..c50f70a3fa 100644 --- a/lib/cretonne/src/simple_gvn.rs +++ b/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 = 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) => { - if domtree.dominates(*entry.get(), inst, pos.layout) { - func.dfg.replace_with_aliases(inst, *entry.get()); - pos.remove_inst_and_step_back(); - } else { - // The prior instruction doesn't dominate inst, so it - // won't dominate any subsequent instructions we'll - // visit, so just replace it. - *entry.get_mut() = inst; - continue; + Occupied(entry) => { + debug_assert!(domtree.dominates(*entry.get(), inst, pos.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.layout.next_inst(inst).unwrap(); } + // 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);