diff --git a/lib/codegen/meta/gen_instr.py b/lib/codegen/meta/gen_instr.py index 0b25cc1835..da9699ee97 100644 --- a/lib/codegen/meta/gen_instr.py +++ b/lib/codegen/meta/gen_instr.py @@ -114,7 +114,7 @@ def gen_instruction_data(fmt): store the additional information out of line. """ - fmt.line('#[derive(Clone, Debug, Hash, PartialEq, Eq)]') + fmt.line('#[derive(Clone, Debug)]') fmt.line('#[allow(missing_docs)]') with fmt.indented('pub enum InstructionData {', '}'): for f in InstructionFormat.all_formats: @@ -147,6 +147,8 @@ def gen_instruction_data_impl(fmt): - `pub fn arguments_mut(&mut self, &pool) -> &mut [Value]` - `pub fn take_value_list(&mut self) -> Option` - `pub fn put_value_list(&mut self, args: ir::ValueList>` + - `pub fn eq(&self, &other: Self, &pool) -> bool` + - `pub fn hash(&self, state: &mut H, &pool)` """ # The `opcode` method simply reads the `opcode` members. This is really a @@ -243,6 +245,93 @@ def gen_instruction_data_impl(fmt): fmt.line( 'debug_assert!(args.is_empty(), "Value list already in use");') fmt.line('*args = vlist;') + fmt.line() + + fmt.doc_comment( + """ + Compare two `InstructionData` for equality. + + This operation requires a reference to a `ValueListPool` to + determine if the contents of any `ValueLists` are equal. + """) + with fmt.indented( + 'pub fn eq(&self, other: &Self, pool: &ir::ValueListPool)' + ' -> bool {', + '}'): + with fmt.indented('if ::std::mem::discriminant(self) != ' + '::std::mem::discriminant(other) {', '}'): + fmt.line('return false;') + with fmt.indented('match (self, other) {', '}'): + for f in InstructionFormat.all_formats: + n = '&InstructionData::' + f.name + members = ['opcode'] + if f.typevar_operand is None: + args_eq = 'true' + elif f.has_value_list: + members.append('args') + args_eq = 'args1.as_slice(pool) == ' \ + 'args2.as_slice(pool)' + elif f.num_value_operands == 1: + members.append('arg') + args_eq = 'arg1 == arg2' + else: + members.append('args') + args_eq = 'args1 == args2' + for field in f.imm_fields: + members.append(field.member) + pat1 = ', '.join('{}: ref {}1'.format(x, x) + for x in members) + pat2 = ', '.join('{}: ref {}2'.format(x, x) + for x in members) + with fmt.indented('({} {{ {} }}, {} {{ {} }}) => {{' + .format(n, pat1, n, pat2), '}'): + fmt.line('opcode1 == opcode2 &&') + for field in f.imm_fields: + fmt.line('{}1 == {}2 &&' + .format(field.member, field.member)) + fmt.line(args_eq) + fmt.line('_ => unsafe { ' + '::std::hint::unreachable_unchecked() }') + fmt.line() + + fmt.doc_comment( + """ + Hash an `InstructionData`. + + This operation requires a reference to a `ValueListPool` to + hash the contents of any `ValueLists`. + """) + with fmt.indented( + 'pub fn hash' + '(&self, state: &mut H, pool: &ir::ValueListPool) {', + '}'): + with fmt.indented('match self {', '}'): + for f in InstructionFormat.all_formats: + n = 'InstructionData::' + f.name + members = ['opcode'] + if f.typevar_operand is None: + args = '&()' + elif f.has_value_list: + members.append('args') + args = 'args.as_slice(pool)' + elif f.num_value_operands == 1: + members.append('arg') + args = 'arg' + else: + members.append('args') + args = 'args' + for field in f.imm_fields: + members.append(field.member) + pat = n + ' { ' + ', '.join(members) + ' }' + with fmt.indented(pat + ' => {', '}'): + fmt.line('::std::hash::Hash::hash( ' + '&::std::mem::discriminant(self), state);') + fmt.line('::std::hash::Hash::hash(opcode, state);') + for field in f.imm_fields: + fmt.line('::std::hash::Hash::hash({}, state);' + .format(field.member)) + fmt.line('::std::hash::Hash::hash({}, state);' + .format(args)) def collect_instr_groups(isas): diff --git a/lib/codegen/src/simple_gvn.rs b/lib/codegen/src/simple_gvn.rs index cc8e7c37c5..79560af674 100644 --- a/lib/codegen/src/simple_gvn.rs +++ b/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>, +} +impl<'a, 'f: 'a> Hash for HashKey<'a, 'f> { + fn hash(&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 = ScopedHashMap::new(); let mut scope_stack: Vec = 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(); } diff --git a/lib/entity/src/list.rs b/lib/entity/src/list.rs index e48a839759..3c5bd25a13 100644 --- a/lib/entity/src/list.rs +++ b/lib/entity/src/list.rs @@ -1,5 +1,4 @@ //! Small lists of entity references. -use std::hash::{Hash, Hasher}; use std::marker::PhantomData; use std::mem; use std::vec::Vec; @@ -34,9 +33,8 @@ use EntityRef; /// function they belong to. *Cloning an entity list does not allocate new memory for the clone*. /// It creates an alias of the same memory. /// -/// Entity lists can also be hashed and compared for equality, but those operations just panic if -/// they're ever actually called, because it's not possible to compare the contents of the list -/// without the pool reference. +/// Entity lists cannot be hashed and compared for equality because it's not possible to compare the +/// contents of the list without the pool reference. /// /// # Implementation /// @@ -76,19 +74,6 @@ impl Default for EntityList { } } -impl Hash for EntityList { - fn hash(&self, _: &mut H) { - panic!("hash called on EntityList"); - } -} - -impl PartialEq for EntityList { - fn eq(&self, _: &Self) -> bool { - panic!("eq called on EntityList"); - } -} -impl Eq for EntityList {} - /// A memory pool for storing lists of `T`. #[derive(Clone, Debug)] pub struct ListPool {