diff --git a/cranelift/codegen/src/peepmatic.rs b/cranelift/codegen/src/peepmatic.rs index 2fbd58d49c..bf0f440865 100644 --- a/cranelift/codegen/src/peepmatic.rs +++ b/cranelift/codegen/src/peepmatic.rs @@ -14,14 +14,13 @@ use peepmatic_runtime::{ cc::ConditionCode, instruction_set::InstructionSet, part::{Constant, Part}, - paths::Path, r#type::{BitWidth, Kind, Type}, PeepholeOptimizations, PeepholeOptimizer, }; -use peepmatic_traits::TypingRules; use std::borrow::Cow; use std::boxed::Box; use std::convert::{TryFrom, TryInto}; +use std::iter; use std::ptr; use std::sync::atomic::{AtomicPtr, Ordering}; @@ -573,35 +572,6 @@ fn intcc_to_peepmatic(cc: IntCC) -> ConditionCode { } } -fn get_immediate(dfg: &DataFlowGraph, inst: Inst, i: usize) -> Part { - return match dfg[inst] { - InstructionData::BinaryImm64 { imm, .. } if i == 0 => imm.into(), - InstructionData::BranchIcmp { cond, .. } if i == 0 => intcc_to_peepmatic(cond).into(), - InstructionData::BranchInt { cond, .. } if i == 0 => intcc_to_peepmatic(cond).into(), - InstructionData::IntCompare { cond, .. } if i == 0 => intcc_to_peepmatic(cond).into(), - InstructionData::IntCompareImm { cond, .. } if i == 0 => intcc_to_peepmatic(cond).into(), - InstructionData::IntCompareImm { imm, .. } if i == 1 => imm.into(), - InstructionData::IntCond { cond, .. } if i == 0 => intcc_to_peepmatic(cond).into(), - InstructionData::IntCondTrap { cond, .. } if i == 0 => intcc_to_peepmatic(cond).into(), - InstructionData::IntSelect { cond, .. } if i == 0 => intcc_to_peepmatic(cond).into(), - InstructionData::UnaryBool { imm, .. } if i == 0 => { - Constant::Bool(imm, BitWidth::Polymorphic).into() - } - InstructionData::UnaryImm { imm, .. } if i == 0 => imm.into(), - ref otherwise => unsupported(otherwise), - }; - - #[inline(never)] - #[cold] - fn unsupported(data: &InstructionData) -> ! { - panic!("unsupported instruction data: {:?}", data) - } -} - -fn get_argument(dfg: &DataFlowGraph, inst: Inst, i: usize) -> Option { - dfg.inst_args(inst).get(i).copied() -} - fn peepmatic_ty_to_ir_ty(ty: Type, dfg: &DataFlowGraph, root: Inst) -> types::Type { match (ty.kind, bit_width(dfg, ty.bit_width, root)) { (Kind::Int, 8) => types::I8, @@ -681,39 +651,290 @@ unsafe impl<'a, 'b> InstructionSet<'b> for &'a dyn TargetIsa { } } - fn get_part_at_path( + fn operator( &self, pos: &mut FuncCursor<'b>, - root: ValueOrInst, - path: Path, - ) -> Option> { - // The root is path [0]. - debug_assert!(!path.0.is_empty()); - debug_assert_eq!(path.0[0], 0); - - let mut part = Part::Instruction(root); - for p in path.0[1..].iter().copied() { - let inst = part.as_instruction()?.resolve_inst(&pos.func.dfg)?; - let operator = pos.func.dfg[inst].opcode(); - - if p < operator.immediates_arity() { - part = get_immediate(&pos.func.dfg, inst, p as usize); - continue; + value_or_inst: ValueOrInst, + operands: &mut E, + ) -> Option + where + E: Extend>, + { + let inst = value_or_inst.resolve_inst(&pos.func.dfg)?; + Some(match pos.func.dfg[inst] { + InstructionData::Binary { + opcode: opcode @ Opcode::Band, + args, + } + | InstructionData::Binary { + opcode: opcode @ Opcode::Bor, + args, + } + | InstructionData::Binary { + opcode: opcode @ Opcode::Bxor, + args, + } + | InstructionData::Binary { + opcode: opcode @ Opcode::Iadd, + args, + } + | InstructionData::Binary { + opcode: opcode @ Opcode::Ifcmp, + args, + } + | InstructionData::Binary { + opcode: opcode @ Opcode::Imul, + args, + } + | InstructionData::Binary { + opcode: opcode @ Opcode::Ishl, + args, + } + | InstructionData::Binary { + opcode: opcode @ Opcode::Isub, + args, + } + | InstructionData::Binary { + opcode: opcode @ Opcode::Rotl, + args, + } + | InstructionData::Binary { + opcode: opcode @ Opcode::Rotr, + args, + } + | InstructionData::Binary { + opcode: opcode @ Opcode::Sdiv, + args, + } + | InstructionData::Binary { + opcode: opcode @ Opcode::Srem, + args, + } + | InstructionData::Binary { + opcode: opcode @ Opcode::Sshr, + args, + } + | InstructionData::Binary { + opcode: opcode @ Opcode::Udiv, + args, + } + | InstructionData::Binary { + opcode: opcode @ Opcode::Urem, + args, + } + | InstructionData::Binary { + opcode: opcode @ Opcode::Ushr, + args, + } => { + operands.extend(args.iter().map(|v| Part::Instruction((*v).into()))); + opcode } - let arg = p - operator.immediates_arity(); - let arg = arg as usize; - let value = get_argument(&pos.func.dfg, inst, arg)?; - part = Part::Instruction(value.into()); - } + InstructionData::BinaryImm64 { + opcode: opcode @ Opcode::BandImm, + imm, + arg, + } + | InstructionData::BinaryImm64 { + opcode: opcode @ Opcode::BorImm, + imm, + arg, + } + | InstructionData::BinaryImm64 { + opcode: opcode @ Opcode::BxorImm, + imm, + arg, + } + | InstructionData::BinaryImm64 { + opcode: opcode @ Opcode::IaddImm, + imm, + arg, + } + | InstructionData::BinaryImm64 { + opcode: opcode @ Opcode::IfcmpImm, + imm, + arg, + } + | InstructionData::BinaryImm64 { + opcode: opcode @ Opcode::ImulImm, + imm, + arg, + } + | InstructionData::BinaryImm64 { + opcode: opcode @ Opcode::IrsubImm, + imm, + arg, + } + | InstructionData::BinaryImm64 { + opcode: opcode @ Opcode::IshlImm, + imm, + arg, + } + | InstructionData::BinaryImm64 { + opcode: opcode @ Opcode::RotlImm, + imm, + arg, + } + | InstructionData::BinaryImm64 { + opcode: opcode @ Opcode::RotrImm, + imm, + arg, + } + | InstructionData::BinaryImm64 { + opcode: opcode @ Opcode::SdivImm, + imm, + arg, + } + | InstructionData::BinaryImm64 { + opcode: opcode @ Opcode::SremImm, + imm, + arg, + } + | InstructionData::BinaryImm64 { + opcode: opcode @ Opcode::SshrImm, + imm, + arg, + } + | InstructionData::BinaryImm64 { + opcode: opcode @ Opcode::UdivImm, + imm, + arg, + } + | InstructionData::BinaryImm64 { + opcode: opcode @ Opcode::UremImm, + imm, + arg, + } + | InstructionData::BinaryImm64 { + opcode: opcode @ Opcode::UshrImm, + imm, + arg, + } => { + operands.extend( + iter::once(imm.into()).chain(iter::once(Part::Instruction(arg.into()))), + ); + opcode + } - log::trace!("get_part_at_path({:?}) = {:?}", path, part); - Some(part) - } + InstructionData::Branch { + opcode: opcode @ Opcode::Brnz, + ref args, + destination: _, + } + | InstructionData::Branch { + opcode: opcode @ Opcode::Brz, + ref args, + destination: _, + } => { + operands.extend( + args.as_slice(&pos.func.dfg.value_lists) + .iter() + .map(|v| Part::Instruction((*v).into())) + // NB: Peepmatic only knows about the condition, not any + // of the arguments to the block, which are special + // cased elsewhere, if/when we actually replace the + // instruction. + .take(1), + ); + opcode + } - fn operator(&self, pos: &mut FuncCursor<'b>, value_or_inst: ValueOrInst) -> Option { - let inst = value_or_inst.resolve_inst(&pos.func.dfg)?; - Some(pos.func.dfg[inst].opcode()) + InstructionData::CondTrap { + opcode: opcode @ Opcode::Trapnz, + arg, + code: _, + } + | InstructionData::CondTrap { + opcode: opcode @ Opcode::Trapz, + arg, + code: _, + } => { + operands.extend(iter::once(Part::Instruction(arg.into()))); + opcode + } + + InstructionData::IntCompare { + opcode: opcode @ Opcode::Icmp, + cond, + args, + } => { + operands.extend( + iter::once(intcc_to_peepmatic(cond).into()) + .chain(args.iter().map(|v| Part::Instruction((*v).into()))), + ); + opcode + } + + InstructionData::IntCompareImm { + opcode: opcode @ Opcode::IcmpImm, + cond, + imm, + arg, + } => { + operands.extend( + iter::once(intcc_to_peepmatic(cond).into()) + .chain(iter::once(Part::Constant(imm.into()))) + .chain(iter::once(Part::Instruction(arg.into()))), + ); + opcode + } + + InstructionData::Ternary { + opcode: opcode @ Opcode::Select, + ref args, + } => { + operands.extend(args.iter().map(|v| Part::Instruction((*v).into()))); + opcode + } + + InstructionData::Unary { + opcode: opcode @ Opcode::AdjustSpDown, + arg, + } + | InstructionData::Unary { + opcode: opcode @ Opcode::Bint, + arg, + } + | InstructionData::Unary { + opcode: opcode @ Opcode::Ireduce, + arg, + } + | InstructionData::Unary { + opcode: opcode @ Opcode::Sextend, + arg, + } + | InstructionData::Unary { + opcode: opcode @ Opcode::Uextend, + arg, + } => { + operands.extend(iter::once(Part::Instruction(arg.into()))); + opcode + } + + InstructionData::UnaryBool { opcode, imm } => { + operands.extend(iter::once(Part::Constant(Constant::Bool( + imm, + BitWidth::Polymorphic, + )))); + opcode + } + + InstructionData::UnaryImm { + opcode: opcode @ Opcode::AdjustSpDownImm, + imm, + } + | InstructionData::UnaryImm { + opcode: opcode @ Opcode::Iconst, + imm, + } => { + operands.extend(iter::once(imm.into())); + opcode + } + ref otherwise => { + log::trace!("Not supported by Peepmatic: {:?}", otherwise); + return None; + } + }) } fn make_inst_1( diff --git a/cranelift/codegen/src/preopt.serialized b/cranelift/codegen/src/preopt.serialized index 8765e4bb84..5669c99126 100644 Binary files a/cranelift/codegen/src/preopt.serialized and b/cranelift/codegen/src/preopt.serialized differ diff --git a/cranelift/peepmatic/crates/runtime/src/instruction_set.rs b/cranelift/peepmatic/crates/runtime/src/instruction_set.rs index d8b9129d6d..b49d71da71 100644 --- a/cranelift/peepmatic/crates/runtime/src/instruction_set.rs +++ b/cranelift/peepmatic/crates/runtime/src/instruction_set.rs @@ -1,7 +1,6 @@ //! Interfacing with actual instructions. use crate::part::{Constant, Part}; -use crate::paths::Path; use crate::r#type::Type; use std::fmt::Debug; use std::hash::Hash; @@ -54,26 +53,22 @@ pub unsafe trait InstructionSet<'a> { new: Part, ) -> Self::Instruction; - /// Get the instruction, constant, or condition code at the given path. - /// - /// If there is no such entity at the given path (e.g. we run into a - /// function parameter and can't traverse the path any further) then `None` - /// should be returned. - fn get_part_at_path( - &self, - context: &mut Self::Context, - root: Self::Instruction, - path: Path, - ) -> Option>; - /// Get the given instruction's operator. /// /// If the instruction isn't supported, then `None` should be returned. - fn operator( + /// + /// Additionally, if `Some` is returned, then the instruction's operands + /// must be pushed in order into `operands`. E.g. calling this method on + /// `(iadd $x $y)` would return `Some(iadd)` and extend `operands` with + /// `[$x, $y]`. + fn operator( &self, context: &mut Self::Context, instr: Self::Instruction, - ) -> Option; + operands: &mut E, + ) -> Option + where + E: Extend>; /// Make a unary instruction. /// diff --git a/cranelift/peepmatic/crates/runtime/src/lib.rs b/cranelift/peepmatic/crates/runtime/src/lib.rs index 0a0cf1012d..5689dd2d20 100644 --- a/cranelift/peepmatic/crates/runtime/src/lib.rs +++ b/cranelift/peepmatic/crates/runtime/src/lib.rs @@ -25,7 +25,6 @@ pub mod linear; pub mod optimizations; pub mod optimizer; pub mod part; -pub mod paths; pub mod r#type; pub mod unquote; diff --git a/cranelift/peepmatic/crates/runtime/src/linear.rs b/cranelift/peepmatic/crates/runtime/src/linear.rs index daa147e22b..575edde9e9 100644 --- a/cranelift/peepmatic/crates/runtime/src/linear.rs +++ b/cranelift/peepmatic/crates/runtime/src/linear.rs @@ -7,7 +7,6 @@ use crate::cc::ConditionCode; use crate::integer_interner::{IntegerId, IntegerInterner}; -use crate::paths::{PathId, PathInterner}; use crate::r#type::{BitWidth, Type}; use crate::unquote::UnquoteOperator; use serde::{Deserialize, Serialize}; @@ -24,9 +23,6 @@ where /// The linear optimizations. pub optimizations: Vec>, - /// The de-duplicated paths referenced by these optimizations. - pub paths: PathInterner, - /// The integer literals referenced by these optimizations. pub integers: IntegerInterner, } @@ -87,79 +83,54 @@ pub struct Match { #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, Deserialize, Serialize)] pub enum MatchOp { /// Switch on the opcode of an instruction. - Opcode { - /// The path to the instruction whose opcode we're switching on. - path: PathId, - }, + /// + /// Upon successfully matching an instruction's opcode, bind each of its + /// operands to a LHS temporary. + Opcode(LhsId), /// Does an instruction have a constant value? - IsConst { - /// The path to the instruction (or immediate) that we're checking - /// whether it is constant or not. - path: PathId, - }, + IsConst(LhsId), /// Is the constant value a power of two? - IsPowerOfTwo { - /// The path to the instruction (or immediate) that we are checking - /// whether it is a constant power of two or not. - path: PathId, - }, + IsPowerOfTwo(LhsId), /// Switch on the bit width of a value. - BitWidth { - /// The path to the instruction (or immediate) whose result's bit width - /// we are checking. - path: PathId, - }, + BitWidth(LhsId), /// Does the value fit in our target architecture's native word size? - FitsInNativeWord { - /// The path to the instruction (or immediate) whose result we are - /// checking whether it fits in a native word or not. - path: PathId, - }, + FitsInNativeWord(LhsId), - /// Are the instructions (or immediates) at the given paths the same? - Eq { - /// The path to the first instruction (or immediate). - path_a: PathId, - /// The path to the second instruction (or immediate). - path_b: PathId, - }, + /// Are the instructions (or immediates) the same? + Eq(LhsId, LhsId), /// Switch on the constant integer value of an instruction. - IntegerValue { - /// The path to the instruction. - path: PathId, - }, + IntegerValue(LhsId), /// Switch on the constant boolean value of an instruction. - BooleanValue { - /// The path to the instruction. - path: PathId, - }, + BooleanValue(LhsId), /// Switch on a condition code. - ConditionCode { - /// The path to the condition code. - path: PathId, - }, + ConditionCode(LhsId), - /// No operation. Always evaluates to `None`. + /// No operation. Always evaluates to `Else`. /// - /// Exceedingly rare in real optimizations; nonetheless required to support + /// Never appears in real optimizations; nonetheless required to support /// corner cases of the DSL, such as a LHS pattern that is nothing but a - /// variable pattern. + /// variable. Nop, } /// A canonicalized identifier for a left-hand side value that was bound in a /// pattern. +/// +/// These are defined in a pre-order traversal of the LHS pattern by successful +/// `MatchOp::Opcode` matches. #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord, Serialize, Deserialize)] pub struct LhsId(pub u16); /// A canonicalized identifier for a right-hand side value. +/// +/// These are defined by RHS actions. #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)] pub struct RhsId(pub u16); @@ -171,8 +142,8 @@ pub struct RhsId(pub u16); pub enum Action { /// Reuse something from the left-hand side. GetLhs { - /// The path to the instruction or value. - path: PathId, + /// The left-hand side instruction or value. + lhs: LhsId, }, /// Perform compile-time evaluation. diff --git a/cranelift/peepmatic/crates/runtime/src/optimizations.rs b/cranelift/peepmatic/crates/runtime/src/optimizations.rs index 3636b0e7b3..02ec18dc38 100644 --- a/cranelift/peepmatic/crates/runtime/src/optimizations.rs +++ b/cranelift/peepmatic/crates/runtime/src/optimizations.rs @@ -5,7 +5,6 @@ use crate::instruction_set::InstructionSet; use crate::integer_interner::IntegerInterner; use crate::linear::{Action, MatchOp, MatchResult}; use crate::optimizer::PeepholeOptimizer; -use crate::paths::PathInterner; use peepmatic_automata::Automaton; use serde::{Deserialize, Serialize}; use std::fmt::Debug; @@ -25,9 +24,6 @@ pub struct PeepholeOptimizations where TOperator: 'static + Copy + Debug + Eq + Hash, { - /// The instruction paths referenced by the peephole optimizations. - pub paths: PathInterner, - /// Not all integers we're matching on fit in the `u32` that we use as the /// result of match operations. So we intern them and refer to them by id. pub integers: IntegerInterner, @@ -88,6 +84,7 @@ where PeepholeOptimizer { peep_opt: self, instr_set, + left_hand_sides: vec![], right_hand_sides: vec![], actions: vec![], backtracking_states: vec![], diff --git a/cranelift/peepmatic/crates/runtime/src/optimizer.rs b/cranelift/peepmatic/crates/runtime/src/optimizer.rs index 48014adc4f..63a15c69f1 100644 --- a/cranelift/peepmatic/crates/runtime/src/optimizer.rs +++ b/cranelift/peepmatic/crates/runtime/src/optimizer.rs @@ -27,9 +27,10 @@ where { pub(crate) peep_opt: &'peep PeepholeOptimizations, pub(crate) instr_set: TInstructionSet, + pub(crate) left_hand_sides: Vec>, pub(crate) right_hand_sides: Vec>, pub(crate) actions: Vec>, - pub(crate) backtracking_states: Vec<(State, usize)>, + pub(crate) backtracking_states: Vec<(State, usize, usize)>, } impl<'peep, 'ctx, TInstructionSet> Debug for PeepholeOptimizer<'peep, 'ctx, TInstructionSet> @@ -40,6 +41,7 @@ where let PeepholeOptimizer { peep_opt, instr_set: _, + left_hand_sides, right_hand_sides, actions, backtracking_states, @@ -47,6 +49,7 @@ where f.debug_struct("PeepholeOptimizer") .field("peep_opt", peep_opt) .field("instr_set", &"_") + .field("left_hand_sides", left_hand_sides) .field("right_hand_sides", right_hand_sides) .field("actions", actions) .field("backtracking_states", backtracking_states) @@ -117,12 +120,8 @@ where for action in actions.drain(..) { log::trace!("Evaluating action: {:?}", action); match action { - Action::GetLhs { path } => { - let path = self.peep_opt.paths.lookup(path); - let lhs = self - .instr_set - .get_part_at_path(context, root, path) - .expect("should always get part at path OK by the time it is bound"); + Action::GetLhs { lhs } => { + let lhs = self.left_hand_sides[lhs.0 as usize]; self.right_hand_sides.push(lhs); } Action::UnaryUnquote { operator, operand } => { @@ -284,22 +283,17 @@ where log::trace!("Evaluating match operation: {:?}", match_op); let result: MatchResult = (|| match match_op { - Opcode { path } => { - let path = self.peep_opt.paths.lookup(path); - let part = self - .instr_set - .get_part_at_path(context, root, path) - .ok_or(Else)?; + Opcode(id) => { + let part = self.left_hand_sides[id.0 as usize]; let inst = part.as_instruction().ok_or(Else)?; - let op = self.instr_set.operator(context, inst).ok_or(Else)?; + let op = self + .instr_set + .operator(context, inst, &mut self.left_hand_sides) + .ok_or(Else)?; Ok(op.into()) } - IsConst { path } => { - let path = self.peep_opt.paths.lookup(path); - let part = self - .instr_set - .get_part_at_path(context, root, path) - .ok_or(Else)?; + IsConst(id) => { + let part = self.left_hand_sides[id.0 as usize]; let is_const = match part { Part::Instruction(i) => { self.instr_set.instruction_to_constant(context, i).is_some() @@ -308,12 +302,8 @@ where }; bool_to_match_result(is_const) } - IsPowerOfTwo { path } => { - let path = self.peep_opt.paths.lookup(path); - let part = self - .instr_set - .get_part_at_path(context, root, path) - .ok_or(Else)?; + IsPowerOfTwo(id) => { + let part = self.left_hand_sides[id.0 as usize]; match part { Part::Constant(c) => { let is_pow2 = c.as_int().unwrap().is_power_of_two(); @@ -327,18 +317,11 @@ where let is_pow2 = c.as_int().unwrap().is_power_of_two(); bool_to_match_result(is_pow2) } - Part::ConditionCode(_) => unreachable!( - "IsPowerOfTwo on a condition - code" - ), + Part::ConditionCode(_) => unreachable!("IsPowerOfTwo on a condition code"), } } - BitWidth { path } => { - let path = self.peep_opt.paths.lookup(path); - let part = self - .instr_set - .get_part_at_path(context, root, path) - .ok_or(Else)?; + BitWidth(id) => { + let part = self.left_hand_sides[id.0 as usize]; let bit_width = match part { Part::Instruction(i) => self.instr_set.instruction_result_bit_width(context, i), Part::Constant(Constant::Int(_, w)) | Part::Constant(Constant::Bool(_, w)) => { @@ -355,15 +338,11 @@ where ); Ok(unsafe { NonZeroU32::new_unchecked(bit_width as u32) }) } - FitsInNativeWord { path } => { + FitsInNativeWord(id) => { let native_word_size = self.instr_set.native_word_size_in_bits(context); debug_assert!(native_word_size.is_power_of_two()); - let path = self.peep_opt.paths.lookup(path); - let part = self - .instr_set - .get_part_at_path(context, root, path) - .ok_or(Else)?; + let part = self.left_hand_sides[id.0 as usize]; let fits = match part { Part::Instruction(i) => { let size = self.instr_set.instruction_result_bit_width(context, i); @@ -378,17 +357,9 @@ where }; bool_to_match_result(fits) } - Eq { path_a, path_b } => { - let path_a = self.peep_opt.paths.lookup(path_a); - let part_a = self - .instr_set - .get_part_at_path(context, root, path_a) - .ok_or(Else)?; - let path_b = self.peep_opt.paths.lookup(path_b); - let part_b = self - .instr_set - .get_part_at_path(context, root, path_b) - .ok_or(Else)?; + Eq(a, b) => { + let part_a = self.left_hand_sides[a.0 as usize]; + let part_b = self.left_hand_sides[b.0 as usize]; let eq = match (part_a, part_b) { (Part::Instruction(inst), Part::Constant(c1)) | (Part::Constant(c1), Part::Instruction(inst)) => { @@ -401,12 +372,8 @@ where }; bool_to_match_result(eq) } - IntegerValue { path } => { - let path = self.peep_opt.paths.lookup(path); - let part = self - .instr_set - .get_part_at_path(context, root, path) - .ok_or(Else)?; + IntegerValue(id) => { + let part = self.left_hand_sides[id.0 as usize]; match part { Part::Constant(c) => { let x = c.as_int().ok_or(Else)?; @@ -425,12 +392,8 @@ where Part::ConditionCode(_) => unreachable!("IntegerValue on condition code"), } } - BooleanValue { path } => { - let path = self.peep_opt.paths.lookup(path); - let part = self - .instr_set - .get_part_at_path(context, root, path) - .ok_or(Else)?; + BooleanValue(id) => { + let part = self.left_hand_sides[id.0 as usize]; match part { Part::Constant(c) => { let b = c.as_bool().ok_or(Else)?; @@ -447,12 +410,8 @@ where Part::ConditionCode(_) => unreachable!("IntegerValue on condition code"), } } - ConditionCode { path } => { - let path = self.peep_opt.paths.lookup(path); - let part = self - .instr_set - .get_part_at_path(context, root, path) - .ok_or(Else)?; + ConditionCode(id) => { + let part = self.left_hand_sides[id.0 as usize]; let cc = part.as_condition_code().ok_or(Else)?; let cc = cc as u32; debug_assert!(cc != 0); @@ -483,12 +442,20 @@ where self.backtracking_states.clear(); self.actions.clear(); self.right_hand_sides.clear(); + self.left_hand_sides.clear(); + + // `LhsId(0)` is always the root. + self.left_hand_sides.push(Part::Instruction(root)); let mut r#final = None; let mut query = self.peep_opt.automata.query(); loop { log::trace!("Current state: {:?}", query.current_state()); + log::trace!( + "self.left_hand_sides = {:#?}", + self.left_hand_sides.iter().enumerate().collect::>() + ); if query.is_in_final_state() { // If we're in a final state (which means an optimization is @@ -507,8 +474,11 @@ where // optimization, we want to be able to backtrack to this state and // then try taking the `Else` transition. if query.has_transition_on(&Err(Else)) { - self.backtracking_states - .push((query.current_state(), self.actions.len())); + self.backtracking_states.push(( + query.current_state(), + self.actions.len(), + self.left_hand_sides.len(), + )); } let match_op = match query.current_state_data() { @@ -522,9 +492,10 @@ where actions } else if r#final.is_some() { break; - } else if let Some((state, actions_len)) = self.backtracking_states.pop() { + } else if let Some((state, actions_len, lhs_len)) = self.backtracking_states.pop() { query.go_to_state(state); self.actions.truncate(actions_len); + self.left_hand_sides.truncate(lhs_len); query .next(&Err(Else)) .expect("backtracking states always have `Else` transitions") diff --git a/cranelift/peepmatic/crates/runtime/src/paths.rs b/cranelift/peepmatic/crates/runtime/src/paths.rs deleted file mode 100644 index f824f19a74..0000000000 --- a/cranelift/peepmatic/crates/runtime/src/paths.rs +++ /dev/null @@ -1,242 +0,0 @@ -//! Representing paths through the dataflow graph. -//! -//! Paths are relative from a *root* instruction, which is the instruction we -//! are determining which, if any, optimizations apply. -//! -//! Paths are series of indices through each instruction's children as we -//! traverse down the graph from the root. Children are immediates followed by -//! arguments: `[imm0, imm1, ..., immN, arg0, arg1, ..., argN]`. -//! -//! ## Examples -//! -//! * `[0]` is the path to the root. -//! * `[0, 0]` is the path to the root's first child. -//! * `[0, 1]` is the path to the root's second child. -//! * `[0, 1, 0]` is the path to the root's second child's first child. -//! -//! ## Interning -//! -//! To avoid extra allocations, de-duplicate paths, and reference them via a -//! fixed-length value, we intern paths inside a `PathInterner` and then -//! reference them via `PathId`. - -// TODO: Make `[]` the path to the root, and get rid of this redundant leading -// zero that is currently in every single path. - -use serde::de::{Deserializer, SeqAccess, Visitor}; -use serde::ser::{SerializeSeq, Serializer}; -use serde::{Deserialize, Serialize}; -use std::collections::HashMap; -use std::convert::TryInto; -use std::fmt; -use std::hash::{Hash, Hasher}; -use std::marker::PhantomData; - -/// A path through the data-flow graph from the root instruction. -#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)] -pub struct Path<'a>(pub &'a [u8]); - -impl Path<'_> { - /// Construct a new path through the data-flow graph from the root - /// instruction. - pub fn new(path: &impl AsRef<[u8]>) -> Path { - Path(path.as_ref()) - } -} - -/// An identifier for an interned path. -#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)] -pub struct PathId(u16); - -/// An interner and de-duplicator for `Path`s. -/// -/// Can be serialized and deserialized while maintaining the same id to interned -/// path mapping. -#[derive(Debug, Default)] -pub struct PathInterner { - /// A map from a path (whose owned data is inside `arena`) to the canonical - /// `PathId` we assigned it when interning it. - map: HashMap, - - /// A map from a `PathId` index to an unsafe, self-borrowed path pointing - /// into `arena`. It is safe to given these out as safe `Path`s, as long as - /// the lifetime is not longer than this `PathInterner`'s lifetime. - paths: Vec, - - /// Bump allocation arena for path data. The bump arena ensures that these - /// allocations never move, and are therefore safe for self-references. - arena: bumpalo::Bump, -} - -impl PathInterner { - /// Construct a new, empty `PathInterner`. - #[inline] - pub fn new() -> Self { - Self::default() - } - - /// Intern a path into this `PathInterner`, returning its canonical - /// `PathId`. - /// - /// If we've already interned this path before, then the existing id we - /// already assigned to it is returned. If we've never seen this path - /// before, then it is copied into this `PathInterner` and a new id is - /// assigned to it. - #[inline] - pub fn intern<'a>(&mut self, path: Path<'a>) -> PathId { - let unsafe_path = unsafe { UnsafePath::from_path(&path) }; - if let Some(id) = self.map.get(&unsafe_path) { - return *id; - } - self.intern_new(path) - } - - #[inline(never)] - fn intern_new<'a>(&mut self, path: Path<'a>) -> PathId { - let id: u16 = self - .paths - .len() - .try_into() - .expect("too many paths interned"); - let id = PathId(id); - - let our_path = self.arena.alloc_slice_copy(&path.0); - let unsafe_path = unsafe { UnsafePath::from_slice(&our_path) }; - - self.paths.push(unsafe_path.clone()); - let old = self.map.insert(unsafe_path, id); - - debug_assert!(old.is_none()); - debug_assert_eq!(self.lookup(id), path); - debug_assert_eq!(self.intern(path), id); - - id - } - - /// Lookup a previously interned path by id. - #[inline] - pub fn lookup<'a>(&'a self, id: PathId) -> Path<'a> { - let unsafe_path = self - .paths - .get(id.0 as usize) - .unwrap_or_else(|| Self::lookup_failure()); - unsafe { unsafe_path.as_path() } - } - - #[inline(never)] - fn lookup_failure() -> ! { - panic!( - "no path for the given id; this can only happen when mixing `PathId`s with different \ - `PathInterner`s" - ) - } -} - -impl Serialize for PathInterner { - fn serialize(&self, serializer: S) -> Result - where - S: Serializer, - { - let mut seq = serializer.serialize_seq(Some(self.paths.len()))?; - for p in &self.paths { - let p = unsafe { p.as_path() }; - seq.serialize_element(&p)?; - } - seq.end() - } -} - -impl<'de> Deserialize<'de> for PathInterner { - fn deserialize(deserializer: D) -> Result - where - D: Deserializer<'de>, - { - deserializer.deserialize_seq(PathInternerVisitor { - marker: PhantomData, - }) - } -} - -struct PathInternerVisitor { - marker: PhantomData PathInterner>, -} - -impl<'de> Visitor<'de> for PathInternerVisitor { - type Value = PathInterner; - - fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { - write!(formatter, "a `peepmatic_runtime::paths::PathInterner`") - } - - fn visit_seq(self, mut access: M) -> Result - where - M: SeqAccess<'de>, - { - const DEFAULT_CAPACITY: usize = 16; - let capacity = access.size_hint().unwrap_or(DEFAULT_CAPACITY); - - let mut interner = PathInterner { - map: HashMap::with_capacity(capacity), - paths: Vec::with_capacity(capacity), - arena: bumpalo::Bump::new(), - }; - - while let Some(path) = access.next_element::()? { - interner.intern(path); - } - - Ok(interner) - } -} - -/// An unsafe, unchecked borrow of a path. Not for use outside of -/// `PathInterner`! -#[derive(Clone, Debug)] -struct UnsafePath { - ptr: *const u8, - len: usize, -} - -impl PartialEq for UnsafePath { - fn eq(&self, rhs: &UnsafePath) -> bool { - unsafe { self.as_slice() == rhs.as_slice() } - } -} - -impl Eq for UnsafePath {} - -impl Hash for UnsafePath { - fn hash(&self, hasher: &mut H) - where - H: Hasher, - { - unsafe { self.as_slice().hash(hasher) } - } -} - -/// Safety: callers must ensure that the constructed values won't have unsafe -/// usages of `PartialEq`, `Eq`, or `Hash`. -impl UnsafePath { - unsafe fn from_path(p: &Path) -> Self { - Self::from_slice(&p.0) - } - - unsafe fn from_slice(s: &[u8]) -> Self { - UnsafePath { - ptr: s.as_ptr(), - len: s.len(), - } - } -} - -/// Safety: callers must ensure that `'a` does not outlive the lifetime of the -/// underlying data. -impl UnsafePath { - unsafe fn as_slice<'a>(&self) -> &'a [u8] { - std::slice::from_raw_parts(self.ptr, self.len) - } - - unsafe fn as_path<'a>(&self) -> Path<'a> { - Path(self.as_slice()) - } -} diff --git a/cranelift/peepmatic/crates/test/src/lib.rs b/cranelift/peepmatic/crates/test/src/lib.rs index 15055e2b1a..948c4abc80 100644 --- a/cranelift/peepmatic/crates/test/src/lib.rs +++ b/cranelift/peepmatic/crates/test/src/lib.rs @@ -6,7 +6,6 @@ use peepmatic_runtime::{ cc::ConditionCode, instruction_set::InstructionSet, part::{Constant, Part}, - paths::Path, r#type::{BitWidth, Kind, Type}, }; use peepmatic_test_operator::TestOperator; @@ -329,43 +328,23 @@ unsafe impl<'a> InstructionSet<'a> for TestIsa { new } - fn get_part_at_path( + fn operator( &self, program: &mut Program, - root: Instruction, - path: Path, - ) -> Option> { - log::debug!("get_part_at_path({:?})", path); - - assert!(!path.0.is_empty()); - assert_eq!(path.0[0], 0); - - let mut part = Part::Instruction(root); - for p in &path.0[1..] { - if let Part::Instruction(inst) = part { - let data = program.data(inst); - let p = *p as usize; - - if p < data.immediates.len() { - part = data.immediates[p].into(); - continue; - } - - if let Some(inst) = data.arguments.get(p - data.immediates.len()).copied() { - part = Part::Instruction(inst); - continue; - } - } - - return None; - } - - Some(part) - } - - fn operator(&self, program: &mut Program, instr: Instruction) -> Option { + instr: Self::Instruction, + operands: &mut E, + ) -> Option + where + E: Extend>, + { log::debug!("operator({:?})", instr); let data = program.data(instr); + operands.extend( + data.immediates + .iter() + .map(|imm| Part::from(*imm)) + .chain(data.arguments.iter().map(|inst| Part::Instruction(*inst))), + ); Some(data.operator) } diff --git a/cranelift/peepmatic/src/dot_fmt.rs b/cranelift/peepmatic/src/dot_fmt.rs index e92c3468ba..71a9533417 100644 --- a/cranelift/peepmatic/src/dot_fmt.rs +++ b/cranelift/peepmatic/src/dot_fmt.rs @@ -7,7 +7,6 @@ use peepmatic_runtime::{ cc::ConditionCode, integer_interner::{IntegerId, IntegerInterner}, linear, - paths::{PathId, PathInterner}, }; use std::convert::{TryFrom, TryInto}; use std::fmt::Debug; @@ -15,7 +14,7 @@ use std::io::{self, Write}; use std::num::{NonZeroU16, NonZeroU32}; #[derive(Debug)] -pub(crate) struct PeepholeDotFmt<'a>(pub(crate) &'a PathInterner, pub(crate) &'a IntegerInterner); +pub(crate) struct PeepholeDotFmt<'a>(pub(crate) &'a IntegerInterner); impl DotFmt]>> for PeepholeDotFmt<'_> @@ -44,7 +43,7 @@ where write!(w, "{}", cc) } linear::MatchOp::IntegerValue { .. } => { - let x = self.1.lookup(IntegerId( + let x = self.0.lookup(IntegerId( NonZeroU16::new(x.get().try_into().unwrap()).unwrap(), )); write!(w, "{}", x) @@ -61,17 +60,16 @@ where write!(w, r#""#)?; - let p = p(self.0); match op { - Opcode { path } => write!(w, "opcode @ {}", p(path))?, - IsConst { path } => write!(w, "is-const? @ {}", p(path))?, - IsPowerOfTwo { path } => write!(w, "is-power-of-two? @ {}", p(path))?, - BitWidth { path } => write!(w, "bit-width @ {}", p(path))?, - FitsInNativeWord { path } => write!(w, "fits-in-native-word @ {}", p(path))?, - Eq { path_a, path_b } => write!(w, "{} == {}", p(path_a), p(path_b))?, - IntegerValue { path } => write!(w, "integer-value @ {}", p(path))?, - BooleanValue { path } => write!(w, "boolean-value @ {}", p(path))?, - ConditionCode { path } => write!(w, "condition-code @ {}", p(path))?, + Opcode(id) => write!(w, "opcode $lhs{}", id.0)?, + IsConst(id) => write!(w, "is-const? $lhs{}", id.0)?, + IsPowerOfTwo(id) => write!(w, "is-power-of-two? $lhs{}", id.0)?, + BitWidth(id) => write!(w, "bit-width $lhs{}", id.0)?, + FitsInNativeWord(id) => write!(w, "fits-in-native-word $lhs{}", id.0)?, + Eq(a, b) => write!(w, "$lhs{} == $lhs{}", a.0, b.0)?, + IntegerValue(id) => write!(w, "integer-value $lhs{}", id.0)?, + BooleanValue(id) => write!(w, "boolean-value $lhs{}", id.0)?, + ConditionCode(id) => write!(w, "condition-code $lhs{}", id.0)?, Nop => write!(w, "nop")?, } @@ -91,11 +89,9 @@ where write!(w, r#""#)?; - let p = p(self.0); - for a in actions.iter() { match a { - GetLhs { path } => write!(w, "get-lhs @ {}
", p(path))?, + GetLhs { lhs } => write!(w, "get-lhs $lhs{}
", lhs.0)?, UnaryUnquote { operator, operand } => { write!(w, "eval {:?} $rhs{}
", operator, operand.0)? } @@ -107,7 +103,7 @@ where MakeIntegerConst { value, bit_width: _, - } => write!(w, "make {}
", self.1.lookup(*value))?, + } => write!(w, "make {}
", self.0.lookup(*value))?, MakeBooleanConst { value, bit_width: _, @@ -142,13 +138,3 @@ where writeln!(w, "") } } - -fn p<'a>(paths: &'a PathInterner) -> impl Fn(&PathId) -> String + 'a { - move |path: &PathId| { - let mut s = vec![]; - for b in paths.lookup(*path).0 { - s.push(b.to_string()); - } - s.join(".") - } -} diff --git a/cranelift/peepmatic/src/lib.rs b/cranelift/peepmatic/src/lib.rs index 86f28f41d8..9b32e06f44 100644 --- a/cranelift/peepmatic/src/lib.rs +++ b/cranelift/peepmatic/src/lib.rs @@ -142,11 +142,10 @@ where sort_lexicographically(&mut opts); let automata = automatize(&opts); - let paths = opts.paths; let integers = opts.integers; if let Ok(path) = std::env::var("PEEPMATIC_DOT") { - let f = dot_fmt::PeepholeDotFmt(&paths, &integers); + let f = dot_fmt::PeepholeDotFmt(&integers); if let Err(e) = automata.write_dot_file(&f, &path) { panic!( "failed to write GraphViz Dot file to PEEPMATIC_DOT={}; error: {}", @@ -155,11 +154,7 @@ where } } - Ok(PeepholeOptimizations { - paths, - integers, - automata, - }) + Ok(PeepholeOptimizations { integers, automata }) } #[cfg(test)] diff --git a/cranelift/peepmatic/src/linear_passes.rs b/cranelift/peepmatic/src/linear_passes.rs index 5eee0d1070..143e1a19c1 100644 --- a/cranelift/peepmatic/src/linear_passes.rs +++ b/cranelift/peepmatic/src/linear_passes.rs @@ -1,9 +1,6 @@ //! Passes over the linear IR. -use peepmatic_runtime::{ - linear, - paths::{PathId, PathInterner}, -}; +use peepmatic_runtime::linear; use std::cmp::Ordering; use std::fmt::Debug; use std::hash::Hash; @@ -33,13 +30,12 @@ where { let linear::Optimizations { ref mut optimizations, - ref paths, .. } = opts; // NB: we *cannot* use an unstable sort here, because we want deterministic // compilation of optimizations to automata. - optimizations.sort_by(|a, b| compare_optimization_generality(paths, a, b)); + optimizations.sort_by(compare_optimization_generality); debug_assert!(is_sorted_by_generality(opts)); } @@ -52,17 +48,14 @@ where { let linear::Optimizations { ref mut optimizations, - ref paths, .. } = opts; // NB: we *cannot* use an unstable sort here, same as above. - optimizations - .sort_by(|a, b| compare_optimizations(paths, a, b, |a_len, b_len| a_len.cmp(&b_len))); + optimizations.sort_by(|a, b| compare_optimizations(a, b, |a_len, b_len| a_len.cmp(&b_len))); } fn compare_optimizations( - paths: &PathInterner, a: &linear::Optimization, b: &linear::Optimization, compare_lengths: impl Fn(usize, usize) -> Ordering, @@ -71,7 +64,7 @@ where TOperator: Copy + Debug + Eq + Hash, { for (a, b) in a.matches.iter().zip(b.matches.iter()) { - let c = compare_match_op_generality(paths, a.operation, b.operation); + let c = compare_match_op_generality(a.operation, b.operation); if c != Ordering::Equal { return c; } @@ -91,86 +84,62 @@ where } fn compare_optimization_generality( - paths: &PathInterner, a: &linear::Optimization, b: &linear::Optimization, ) -> Ordering where TOperator: Copy + Debug + Eq + Hash, { - compare_optimizations(paths, a, b, |a_len, b_len| { + compare_optimizations(a, b, |a_len, b_len| { // If they shared equivalent prefixes, then compare lengths and invert the // result because longer patterns are less general than shorter patterns. a_len.cmp(&b_len).reverse() }) } -fn compare_match_op_generality( - paths: &PathInterner, - a: linear::MatchOp, - b: linear::MatchOp, -) -> Ordering { +fn compare_match_op_generality(a: linear::MatchOp, b: linear::MatchOp) -> Ordering { use linear::MatchOp::*; match (a, b) { - (Opcode { path: a }, Opcode { path: b }) => compare_paths(paths, a, b), - (Opcode { .. }, _) => Ordering::Less, - (_, Opcode { .. }) => Ordering::Greater, + (Opcode(a), Opcode(b)) => a.cmp(&b), + (Opcode(_), _) => Ordering::Less, + (_, Opcode(_)) => Ordering::Greater, - (IntegerValue { path: a }, IntegerValue { path: b }) => compare_paths(paths, a, b), - (IntegerValue { .. }, _) => Ordering::Less, - (_, IntegerValue { .. }) => Ordering::Greater, + (IntegerValue(a), IntegerValue(b)) => a.cmp(&b), + (IntegerValue(_), _) => Ordering::Less, + (_, IntegerValue(_)) => Ordering::Greater, - (BooleanValue { path: a }, BooleanValue { path: b }) => compare_paths(paths, a, b), - (BooleanValue { .. }, _) => Ordering::Less, - (_, BooleanValue { .. }) => Ordering::Greater, + (BooleanValue(a), BooleanValue(b)) => a.cmp(&b), + (BooleanValue(_), _) => Ordering::Less, + (_, BooleanValue(_)) => Ordering::Greater, - (ConditionCode { path: a }, ConditionCode { path: b }) => compare_paths(paths, a, b), - (ConditionCode { .. }, _) => Ordering::Less, - (_, ConditionCode { .. }) => Ordering::Greater, + (ConditionCode(a), ConditionCode(b)) => a.cmp(&b), + (ConditionCode(_), _) => Ordering::Less, + (_, ConditionCode(_)) => Ordering::Greater, - (IsConst { path: a }, IsConst { path: b }) => compare_paths(paths, a, b), - (IsConst { .. }, _) => Ordering::Less, - (_, IsConst { .. }) => Ordering::Greater, + (IsConst(a), IsConst(b)) => a.cmp(&b), + (IsConst(_), _) => Ordering::Less, + (_, IsConst(_)) => Ordering::Greater, - ( - Eq { - path_a: pa1, - path_b: pb1, - }, - Eq { - path_a: pa2, - path_b: pb2, - }, - ) => compare_paths(paths, pa1, pa2).then(compare_paths(paths, pb1, pb2)), - (Eq { .. }, _) => Ordering::Less, - (_, Eq { .. }) => Ordering::Greater, + (Eq(a1, b1), Eq(a2, b2)) => a1.cmp(&a2).then(b1.cmp(&b2)), + (Eq(..), _) => Ordering::Less, + (_, Eq(..)) => Ordering::Greater, - (IsPowerOfTwo { path: a }, IsPowerOfTwo { path: b }) => compare_paths(paths, a, b), - (IsPowerOfTwo { .. }, _) => Ordering::Less, - (_, IsPowerOfTwo { .. }) => Ordering::Greater, + (IsPowerOfTwo(a), IsPowerOfTwo(b)) => a.cmp(&b), + (IsPowerOfTwo(_), _) => Ordering::Less, + (_, IsPowerOfTwo(_)) => Ordering::Greater, - (BitWidth { path: a }, BitWidth { path: b }) => compare_paths(paths, a, b), - (BitWidth { .. }, _) => Ordering::Less, - (_, BitWidth { .. }) => Ordering::Greater, + (BitWidth(a), BitWidth(b)) => a.cmp(&b), + (BitWidth(_), _) => Ordering::Less, + (_, BitWidth(_)) => Ordering::Greater, - (FitsInNativeWord { path: a }, FitsInNativeWord { path: b }) => compare_paths(paths, a, b), - (FitsInNativeWord { .. }, _) => Ordering::Less, - (_, FitsInNativeWord { .. }) => Ordering::Greater, + (FitsInNativeWord(a), FitsInNativeWord(b)) => a.cmp(&b), + (FitsInNativeWord(_), _) => Ordering::Less, + (_, FitsInNativeWord(_)) => Ordering::Greater, (Nop, Nop) => Ordering::Equal, } } -fn compare_paths(paths: &PathInterner, a: PathId, b: PathId) -> Ordering { - if a == b { - Ordering::Equal - } else { - let a = paths.lookup(a); - let b = paths.lookup(b); - a.0.cmp(&b.0) - } -} - /// Are the given optimizations sorted from least to most general? pub(crate) fn is_sorted_by_generality(opts: &linear::Optimizations) -> bool where @@ -178,7 +147,7 @@ where { opts.optimizations .windows(2) - .all(|w| compare_optimization_generality(&opts.paths, &w[0], &w[1]) <= Ordering::Equal) + .all(|w| compare_optimization_generality(&w[0], &w[1]) <= Ordering::Equal) } /// Are the given optimizations sorted lexicographically? @@ -189,8 +158,7 @@ where TOperator: Copy + Debug + Eq + Hash, { opts.optimizations.windows(2).all(|w| { - compare_optimizations(&opts.paths, &w[0], &w[1], |a_len, b_len| a_len.cmp(&b_len)) - <= Ordering::Equal + compare_optimizations(&w[0], &w[1], |a_len, b_len| a_len.cmp(&b_len)) <= Ordering::Equal }) } @@ -309,10 +277,7 @@ where mod tests { use super::*; use crate::ast::*; - use peepmatic_runtime::{ - linear::{bool_to_match_result, Else, MatchOp::*, MatchResult}, - paths::*, - }; + use peepmatic_runtime::linear::{bool_to_match_result, Else, LhsId, MatchOp::*, MatchResult}; use peepmatic_test_operator::TestOperator; use std::num::NonZeroU32; @@ -374,7 +339,6 @@ mod tests { eprintln!("after = {:#?}", before); let linear::Optimizations { - mut paths, mut integers, optimizations, } = opts; @@ -389,9 +353,8 @@ mod tests { }) .collect(); - let mut p = |p: &[u8]| paths.intern(Path::new(&p)); let mut i = |i: u64| Ok(integers.intern(i).into()); - let expected = $make_expected(&mut p, &mut i); + let expected = $make_expected(&mut i); assert_eq!(expected, actual); } @@ -452,7 +415,6 @@ mod tests { eprintln!("after = {:#?}", before); let linear::Optimizations { - mut paths, mut integers, optimizations, } = opts; @@ -467,9 +429,8 @@ mod tests { }) .collect(); - let mut p = |p: &[u8]| paths.intern(Path::new(&p)); let mut i = |i: u64| Ok(integers.intern(i).into()); - let expected = $make_expected(&mut p, &mut i); + let expected = $make_expected(&mut i); assert_eq!(expected, actual); } @@ -488,55 +449,43 @@ mod tests { (=> (iadd $x 42) 0) (=> (iadd $x (iadd $y $z)) 0) ", - |p: &mut dyn FnMut(&[u8]) -> PathId, i: &mut dyn FnMut(u64) -> MatchResult| vec![ + |i: &mut dyn FnMut(u64) -> MatchResult| vec![ vec![ - (Opcode { path: p(&[0]) }, Ok(TestOperator::Iadd.into())), + (Opcode(LhsId(0)), Ok(TestOperator::Iadd.into())), (Nop, Err(Else)), - (Opcode { path: p(&[0, 1]) }, Ok(TestOperator::Iadd.into())), + (Opcode(LhsId(2)), Ok(TestOperator::Iadd.into())), (Nop, Err(Else)), (Nop, Err(Else)), ], vec![ - (Opcode { path: p(&[0]) }, Ok(TestOperator::Iadd.into())), + (Opcode(LhsId(0)), Ok(TestOperator::Iadd.into())), (Nop, Err(Else)), - (IntegerValue { path: p(&[0, 1]) }, i(42)) + (IntegerValue(LhsId(2)), i(42)) ], vec![ - (Opcode { path: p(&[0]) }, Ok(TestOperator::Iadd.into())), + (Opcode(LhsId(0)), Ok(TestOperator::Iadd.into())), (Nop, Err(Else)), - (IsConst { path: p(&[0, 1]) }, bool_to_match_result(true)), - ( - IsPowerOfTwo { path: p(&[0, 1]) }, - bool_to_match_result(true) - ) + (IsConst(LhsId(2)), bool_to_match_result(true)), + (IsPowerOfTwo(LhsId(2)), bool_to_match_result(true)) ], vec![ - (Opcode { path: p(&[0]) }, Ok(TestOperator::Iadd.into())), + (Opcode(LhsId(0)), Ok(TestOperator::Iadd.into())), (Nop, Err(Else)), - (IsConst { path: p(&[0, 1]) }, bool_to_match_result(true)), - ( - BitWidth { path: p(&[0, 0]) }, - Ok(NonZeroU32::new(32).unwrap()) - ) + (IsConst(LhsId(2)), bool_to_match_result(true)), + (BitWidth(LhsId(1)), Ok(NonZeroU32::new(32).unwrap())) ], vec![ - (Opcode { path: p(&[0]) }, Ok(TestOperator::Iadd.into())), + (Opcode(LhsId(0)), Ok(TestOperator::Iadd.into())), (Nop, Err(Else)), - (IsConst { path: p(&[0, 1]) }, bool_to_match_result(true)) + (IsConst(LhsId(2)), bool_to_match_result(true)) ], vec![ - (Opcode { path: p(&[0]) }, Ok(TestOperator::Iadd.into())), + (Opcode(LhsId(0)), Ok(TestOperator::Iadd.into())), (Nop, Err(Else)), - ( - Eq { - path_a: p(&[0, 1]), - path_b: p(&[0, 0]), - }, - bool_to_match_result(true) - ) + (Eq(LhsId(2), LhsId(1)), bool_to_match_result(true)) ], vec![ - (Opcode { path: p(&[0]) }, Ok(TestOperator::Iadd.into())), + (Opcode(LhsId(0)), Ok(TestOperator::Iadd.into())), (Nop, Err(Else)), (Nop, Err(Else)), ], @@ -554,36 +503,36 @@ mod tests { (=> (imul 2 $x) (ishl $x 1)) (=> (imul $x 2) (ishl $x 1)) ", - |p: &mut dyn FnMut(&[u8]) -> PathId, i: &mut dyn FnMut(u64) -> MatchResult| vec![ + |i: &mut dyn FnMut(u64) -> MatchResult| vec![ vec![ - (Opcode { path: p(&[0]) }, Ok(TestOperator::Imul.into())), - (IntegerValue { path: p(&[0, 0]) }, i(2)), + (Opcode(LhsId(0)), Ok(TestOperator::Imul.into())), + (IntegerValue(LhsId(1)), i(2)), (Nop, Err(Else)) ], vec![ - (Opcode { path: p(&[0]) }, Ok(TestOperator::Imul.into())), - (IntegerValue { path: p(&[0, 0]) }, i(1)), + (Opcode(LhsId(0)), Ok(TestOperator::Imul.into())), + (IntegerValue(LhsId(1)), i(1)), (Nop, Err(Else)) ], vec![ - (Opcode { path: p(&[0]) }, Ok(TestOperator::Imul.into())), + (Opcode(LhsId(0)), Ok(TestOperator::Imul.into())), (Nop, Err(Else)), - (IntegerValue { path: p(&[0, 1]) }, i(2)) + (IntegerValue(LhsId(2)), i(2)) ], vec![ - (Opcode { path: p(&[0]) }, Ok(TestOperator::Imul.into())), + (Opcode(LhsId(0)), Ok(TestOperator::Imul.into())), (Nop, Err(Else)), - (IntegerValue { path: p(&[0, 1]) }, i(1)) + (IntegerValue(LhsId(2)), i(1)) ], vec![ - (Opcode { path: p(&[0]) }, Ok(TestOperator::Iadd.into())), - (IntegerValue { path: p(&[0, 0]) }, i(0)), + (Opcode(LhsId(0)), Ok(TestOperator::Iadd.into())), + (IntegerValue(LhsId(1)), i(0)), (Nop, Err(Else)) ], vec![ - (Opcode { path: p(&[0]) }, Ok(TestOperator::Iadd.into())), + (Opcode(LhsId(0)), Ok(TestOperator::Iadd.into())), (Nop, Err(Else)), - (IntegerValue { path: p(&[0, 1]) }, i(0)) + (IntegerValue(LhsId(2)), i(0)) ] ] ); @@ -597,32 +546,20 @@ mod tests { (=> (bor (bor $x $y) $y) (bor $x $y)) ", - |p: &mut dyn FnMut(&[u8]) -> PathId, i: &mut dyn FnMut(u64) -> MatchResult| vec![ + |i: &mut dyn FnMut(u64) -> MatchResult| vec![ vec![ - (Opcode { path: p(&[0]) }, Ok(TestOperator::Bor.into())), - (Opcode { path: p(&[0, 0]) }, Ok(TestOperator::Bor.into())), + (Opcode(LhsId(0)), Ok(TestOperator::Bor.into())), + (Opcode(LhsId(1)), Ok(TestOperator::Bor.into())), (Nop, Err(Else)), + (Eq(LhsId(3), LhsId(2)), bool_to_match_result(true)), (Nop, Err(Else)), - ( - Eq { - path_a: p(&[0, 1]), - path_b: p(&[0, 0, 0]), - }, - bool_to_match_result(true) - ), ], vec![ - (Opcode { path: p(&[0]) }, Ok(TestOperator::Bor.into())), - (Opcode { path: p(&[0, 0]) }, Ok(TestOperator::Bor.into())), + (Opcode(LhsId(0)), Ok(TestOperator::Bor.into())), + (Opcode(LhsId(1)), Ok(TestOperator::Bor.into())), (Nop, Err(Else)), (Nop, Err(Else)), - ( - Eq { - path_a: p(&[0, 1]), - path_b: p(&[0, 0, 1]), - }, - bool_to_match_result(true) - ), + (Eq(LhsId(4), LhsId(2)), bool_to_match_result(true)), ], ] ); @@ -636,39 +573,21 @@ mod tests { (=> (bor (bor $x $y) $y) (bor $x $y)) ", - |p: &mut dyn FnMut(&[u8]) -> PathId, i: &mut dyn FnMut(u64) -> MatchResult| vec![ + |i: &mut dyn FnMut(u64) -> MatchResult| vec![ vec![ - (Opcode { path: p(&[0]) }, Ok(TestOperator::Bor.into())), - (Opcode { path: p(&[0, 0]) }, Ok(TestOperator::Bor.into())), + (Opcode(LhsId(0)), Ok(TestOperator::Bor.into())), + (Opcode(LhsId(1)), Ok(TestOperator::Bor.into())), (Nop, Err(Else)), + (Eq(LhsId(3), LhsId(2)), bool_to_match_result(true)), (Nop, Err(Else)), - ( - Eq { - path_a: p(&[0, 1]), - path_b: p(&[0, 0, 0]), - }, - bool_to_match_result(true) - ), ], vec![ - (Opcode { path: p(&[0]) }, Ok(TestOperator::Bor.into())), - (Opcode { path: p(&[0, 0]) }, Ok(TestOperator::Bor.into())), + (Opcode(LhsId(0)), Ok(TestOperator::Bor.into())), + (Opcode(LhsId(1)), Ok(TestOperator::Bor.into())), (Nop, Err(Else)), + (Eq(LhsId(3), LhsId(2)), Err(Else)), (Nop, Err(Else)), - ( - Eq { - path_a: p(&[0, 1]), - path_b: p(&[0, 0, 0]), - }, - Err(Else), - ), - ( - Eq { - path_a: p(&[0, 1]), - path_b: p(&[0, 0, 1]), - }, - bool_to_match_result(true) - ), + (Eq(LhsId(4), LhsId(2)), bool_to_match_result(true)), ], ] ); diff --git a/cranelift/peepmatic/src/linearize.rs b/cranelift/peepmatic/src/linearize.rs index 1ab8e9475d..6d205a7c42 100644 --- a/cranelift/peepmatic/src/linearize.rs +++ b/cranelift/peepmatic/src/linearize.rs @@ -49,14 +49,8 @@ //! //! Here are the general principles that linearization should adhere to: //! -//! * Actions should be pushed as early in the optimization's match chain as -//! they can be. This means the tail has fewer side effects, and is therefore -//! more likely to be share-able with other optimizations in the automata that -//! we build. -//! -//! * RHS actions cannot reference matches from the LHS until they've been -//! defined. And finally, an RHS operation's operands must be defined before -//! the RHS operation itself. In general, definitions must come before uses! +//! * Don't match on a subtree until we know it exists. That is, match on +//! parents before matching on children. //! //! * Shorter match chains are better! This means fewer tests when matching //! left-hand sides, and a more-compact, more-cache-friendly automata, and @@ -76,13 +70,15 @@ //! precondition. //! //! Within matching the pattern structure, we emit matching operations in a -//! pre-order traversal of the pattern. This ensures that we've already matched -//! an operation before we consider its operands, and therefore we already know -//! the operands exist. See `PatternPreOrder` for details. +//! breadth-first traversal of the pattern. This ensures that we've already +//! matched an operation before we consider its operands, and therefore we +//! already know the operands exist. It also lets us fuse "what opcode does this +//! instruction have?" and "define temporary variables for this instruction's +//! operands" into a single operation. See `PatternBfs` for details. //! //! As we define the match operations for a pattern, we remember the path where //! each LHS id first occurred. These will later be reused when building the RHS -//! actions. See `LhsIdToPath` for details. +//! actions. See `LhsCanonicalizer` for details. //! //! After we've generated the match operations and expected result of those //! match operations, then we generate the right-hand side actions. The @@ -94,12 +90,8 @@ //! linear optimization translation function. use crate::ast::*; -use crate::traversals::Dfs; -use peepmatic_runtime::{ - integer_interner::IntegerInterner, - linear, - paths::{Path, PathId, PathInterner}, -}; +use crate::traversals::{Bfs, Dfs}; +use peepmatic_runtime::{integer_interner::IntegerInterner, linear}; use std::collections::BTreeMap; use std::convert::TryInto; use std::fmt::Debug; @@ -114,22 +106,19 @@ where TOperator: Copy + Debug + Eq + Hash + Into, { let mut optimizations = vec![]; - let mut paths = PathInterner::new(); let mut integers = IntegerInterner::new(); for opt in &opts.optimizations { - let lin_opt = linearize_optimization(&mut paths, &mut integers, opt); + let lin_opt = linearize_optimization(&mut integers, opt); optimizations.push(lin_opt); } linear::Optimizations { optimizations, - paths, integers, } } /// Translate an AST optimization into a linear optimization! fn linearize_optimization( - paths: &mut PathInterner, integers: &mut IntegerInterner, opt: &Optimization, ) -> linear::Optimization @@ -138,22 +127,21 @@ where { let mut matches: Vec = vec![]; - let mut lhs_id_to_path = LhsIdToPath::new(); + let mut lhs_canonicalizer = LhsCanonicalizer::new(); - // We do a pre-order traversal of the LHS because we don't know whether a - // child actually exists to match on until we've matched its parent, and we - // don't want to emit matching operations on things that might not exist! - let mut patterns = PatternPreOrder::new(&opt.lhs.pattern); - while let Some((path, pattern)) = patterns.next(paths) { + // We do a breadth-first traversal of the LHS because we don't know whether + // a child actually exists to match on until we've matched its parent, and + // we don't want to emit matching operations on things that might not exist! + for (id, pattern) in PatternBfs::new(&opt.lhs.pattern) { // Create the matching parts of an `Match` for this part of the // pattern. - let (operation, expected) = pattern.to_linear_match_op(integers, &lhs_id_to_path, path); + let (operation, expected) = pattern.to_linear_match_op(integers, &lhs_canonicalizer, id); matches.push(linear::Match { operation, expected, }); - lhs_id_to_path.remember_path_to_pattern_ids(pattern, path); + lhs_canonicalizer.remember_linear_id(pattern, id); // Some operations require type ascriptions for us to infer the correct // bit width of their results: `ireduce`, `sextend`, `uextend`, etc. @@ -165,7 +153,7 @@ where let expected = Ok(unsafe { NonZeroU32::new_unchecked(w as u32) }); matches.push(linear::Match { - operation: linear::MatchOp::BitWidth { path }, + operation: linear::MatchOp::BitWidth(id), expected, }); } @@ -175,7 +163,7 @@ where // Now that we've added all the matches for the LHS pattern, add the // matches for its preconditions. for pre in &opt.lhs.preconditions { - matches.push(pre.to_linear_match(&lhs_id_to_path)); + matches.push(pre.to_linear_match(&lhs_canonicalizer)); } assert!(!matches.is_empty()); @@ -183,7 +171,7 @@ where // Finally, generate the RHS-building actions and attach them to the first match. let mut rhs_builder = RhsBuilder::new(&opt.rhs); let mut actions = vec![]; - rhs_builder.add_rhs_build_actions(integers, &lhs_id_to_path, &mut actions); + rhs_builder.add_rhs_build_actions(integers, &lhs_canonicalizer, &mut actions); linear::Optimization { matches, actions } } @@ -222,55 +210,46 @@ where } } -/// A pre-order, depth-first traversal of left-hand side patterns. +/// A breadth-first traversal of left-hand side patterns. /// -/// Keeps track of the path to each pattern, and yields it along side the +/// Keeps track of the `LhsId` of each pattern, and yields it along side the /// pattern AST node. -struct PatternPreOrder<'a, TOperator> { - last_child: Option, - path: Vec, - dfs: Dfs<'a, TOperator>, +/// +/// We use a breadth-first traversal because we fuse "which opcode is this?" and +/// "assign operands to temporaries" into a single linear match operation. A +/// breadth-first traversal aligns with "match this opcode, and on success bind +/// all of its operands to temporaries". Fusing these operations into one is +/// important for attaining similar performance as an open-coded Rust `match` +/// expression, which would also fuse these operations via pattern matching. +struct PatternBfs<'a, TOperator> { + next_id: u16, + bfs: Bfs<'a, TOperator>, } -impl<'a, TOperator> PatternPreOrder<'a, TOperator> +impl<'a, TOperator> PatternBfs<'a, TOperator> where TOperator: Copy + Debug + Eq + Hash, { fn new(pattern: &'a Pattern<'a, TOperator>) -> Self { Self { - last_child: None, - path: vec![], - dfs: Dfs::new(pattern), + next_id: 0, + bfs: Bfs::new(pattern), } } +} - fn next(&mut self, paths: &mut PathInterner) -> Option<(PathId, &'a Pattern<'a, TOperator>)> { - use crate::traversals::TraversalEvent as TE; +impl<'a, TOperator> Iterator for PatternBfs<'a, TOperator> +where + TOperator: 'a + Copy + Debug + Eq + Hash, +{ + type Item = (linear::LhsId, &'a Pattern<'a, TOperator>); + + fn next(&mut self) -> Option { loop { - match self.dfs.next()? { - (TE::Enter, DynAstRef::Pattern(pattern)) => { - let last_child = self.last_child.take(); - self.path.push(match last_child { - None => 0, - Some(c) => { - assert!( - c < std::u8::MAX, - "operators must have less than or equal u8::MAX arity" - ); - c + 1 - } - }); - let path = paths.intern(Path(&self.path)); - return Some((path, pattern)); - } - (TE::Exit, DynAstRef::Pattern(_)) => { - self.last_child = Some( - self.path - .pop() - .expect("should always have a non-empty path during traversal"), - ); - } - _ => {} + if let DynAstRef::Pattern(pattern) = self.bfs.next()? { + let id = linear::LhsId(self.next_id); + self.next_id = self.next_id.checked_add(1).unwrap(); + return Some((id, pattern)); } } } @@ -278,42 +257,37 @@ where /// A map from left-hand side identifiers to the path in the left-hand side /// where they first occurred. -struct LhsIdToPath<'a, TOperator> { - id_to_path: BTreeMap<&'a str, PathId>, +struct LhsCanonicalizer<'a, TOperator> { + id_to_linear: BTreeMap<&'a str, linear::LhsId>, _marker: PhantomData<&'a TOperator>, } -impl<'a, TOperator> LhsIdToPath<'a, TOperator> { - /// Construct a new, empty `LhsIdToPath`. +impl<'a, TOperator> LhsCanonicalizer<'a, TOperator> { + /// Construct a new, empty `LhsCanonicalizer`. fn new() -> Self { Self { - id_to_path: Default::default(), + id_to_linear: Default::default(), _marker: PhantomData, } } - /// Have we already seen the given identifier? - fn get_first_occurrence(&self, id: &Id) -> Option { - self.id_to_path.get(id.name()).copied() + /// Get the canonical `linear::LhsId` for the given variable, if any. + fn get(&self, id: &Id) -> Option { + self.id_to_linear.get(id.name()).copied() } - /// Get the path within the left-hand side pattern where we first saw the - /// given AST id. - /// - /// ## Panics - /// - /// Panics if the given AST id has not already been canonicalized. - fn unwrap_first_occurrence(&self, id: &Id) -> PathId { - self.id_to_path[id.name()] - } - - /// Remember the path to any LHS ids used in the given pattern. - fn remember_path_to_pattern_ids(&mut self, pattern: &'a Pattern<'a, TOperator>, path: PathId) { + /// Remember the canonical `linear::LhsId` for any variables or constants + /// used in the given pattern. + fn remember_linear_id( + &mut self, + pattern: &'a Pattern<'a, TOperator>, + linear_id: linear::LhsId, + ) { match pattern { // If this is the first time we've seen an identifier defined on the // left-hand side, remember it. Pattern::Variable(Variable { id, .. }) | Pattern::Constant(Constant { id, .. }) => { - self.id_to_path.entry(id.name()).or_insert(path); + self.id_to_linear.entry(id.name()).or_insert(linear_id); } _ => {} } @@ -366,11 +340,11 @@ where fn add_rhs_build_actions( &mut self, integers: &mut IntegerInterner, - lhs_id_to_path: &LhsIdToPath, + lhs_canonicalizer: &LhsCanonicalizer, actions: &mut Vec>, ) { while let Some(rhs) = self.rhs_post_order.next() { - actions.push(self.rhs_to_linear_action(integers, lhs_id_to_path, rhs)); + actions.push(self.rhs_to_linear_action(integers, lhs_canonicalizer, rhs)); let id = linear::RhsId(self.rhs_span_to_id.len().try_into().unwrap()); self.rhs_span_to_id.insert(rhs.span(), id); } @@ -379,7 +353,7 @@ where fn rhs_to_linear_action( &self, integers: &mut IntegerInterner, - lhs_id_to_path: &LhsIdToPath, + lhs_canonicalizer: &LhsCanonicalizer, rhs: &Rhs, ) -> linear::Action { match rhs { @@ -401,8 +375,8 @@ where linear::Action::MakeConditionCode { cc: *cc } } Rhs::Variable(Variable { id, .. }) | Rhs::Constant(Constant { id, .. }) => { - let path = lhs_id_to_path.unwrap_first_occurrence(id); - linear::Action::GetLhs { path } + let lhs = lhs_canonicalizer.get(id).unwrap(); + linear::Action::GetLhs { lhs } } Rhs::Unquote(unq) => match unq.operands.len() { 1 => linear::Action::UnaryUnquote { @@ -461,16 +435,16 @@ where TOperator: Copy + Debug + Eq + Hash + Into, { /// Convert this precondition into a `linear::Match`. - fn to_linear_match(&self, lhs_id_to_path: &LhsIdToPath) -> linear::Match { + fn to_linear_match(&self, lhs_canonicalizer: &LhsCanonicalizer) -> linear::Match { match self.constraint { Constraint::IsPowerOfTwo => { let id = match &self.operands[0] { ConstraintOperand::Constant(Constant { id, .. }) => id, _ => unreachable!("checked in verification"), }; - let path = lhs_id_to_path.unwrap_first_occurrence(&id); + let id = lhs_canonicalizer.get(&id).unwrap(); linear::Match { - operation: linear::MatchOp::IsPowerOfTwo { path }, + operation: linear::MatchOp::IsPowerOfTwo(id), expected: linear::bool_to_match_result(true), } } @@ -480,7 +454,7 @@ where | ConstraintOperand::Variable(Variable { id, .. }) => id, _ => unreachable!("checked in verification"), }; - let path = lhs_id_to_path.unwrap_first_occurrence(&id); + let id = lhs_canonicalizer.get(&id).unwrap(); let width = match &self.operands[1] { ConstraintOperand::ValueLiteral(ValueLiteral::Integer(Integer { @@ -495,7 +469,7 @@ where let expected = Ok(unsafe { NonZeroU32::new_unchecked(width as u32) }); linear::Match { - operation: linear::MatchOp::BitWidth { path }, + operation: linear::MatchOp::BitWidth(id), expected, } } @@ -505,9 +479,9 @@ where | ConstraintOperand::Variable(Variable { id, .. }) => id, _ => unreachable!("checked in verification"), }; - let path = lhs_id_to_path.unwrap_first_occurrence(&id); + let id = lhs_canonicalizer.get(&id).unwrap(); linear::Match { - operation: linear::MatchOp::FitsInNativeWord { path }, + operation: linear::MatchOp::FitsInNativeWord(id), expected: linear::bool_to_match_result(true), } } @@ -527,52 +501,46 @@ where fn to_linear_match_op( &self, integers: &mut IntegerInterner, - lhs_id_to_path: &LhsIdToPath, - path: PathId, + lhs_canonicalizer: &LhsCanonicalizer, + linear_lhs_id: linear::LhsId, ) -> (linear::MatchOp, linear::MatchResult) where TOperator: Into, { match self { Pattern::ValueLiteral(ValueLiteral::Integer(Integer { value, .. })) => ( - linear::MatchOp::IntegerValue { path }, + linear::MatchOp::IntegerValue(linear_lhs_id), Ok(integers.intern(*value as u64).into()), ), Pattern::ValueLiteral(ValueLiteral::Boolean(Boolean { value, .. })) => ( - linear::MatchOp::BooleanValue { path }, + linear::MatchOp::BooleanValue(linear_lhs_id), linear::bool_to_match_result(*value), ), Pattern::ValueLiteral(ValueLiteral::ConditionCode(ConditionCode { cc, .. })) => { let cc = *cc as u32; debug_assert!(cc != 0, "no `ConditionCode` variants are zero"); let expected = Ok(unsafe { NonZeroU32::new_unchecked(cc) }); - (linear::MatchOp::ConditionCode { path }, expected) + (linear::MatchOp::ConditionCode(linear_lhs_id), expected) } Pattern::Constant(Constant { id, .. }) => { - if let Some(path_b) = lhs_id_to_path.get_first_occurrence(id) { - debug_assert!(path != path_b); + if let Some(linear_lhs_id2) = lhs_canonicalizer.get(id) { + debug_assert!(linear_lhs_id != linear_lhs_id2); ( - linear::MatchOp::Eq { - path_a: path, - path_b, - }, + linear::MatchOp::Eq(linear_lhs_id, linear_lhs_id2), linear::bool_to_match_result(true), ) } else { ( - linear::MatchOp::IsConst { path }, + linear::MatchOp::IsConst(linear_lhs_id), linear::bool_to_match_result(true), ) } } Pattern::Variable(Variable { id, .. }) => { - if let Some(path_b) = lhs_id_to_path.get_first_occurrence(id) { - debug_assert!(path != path_b); + if let Some(linear_lhs_id2) = lhs_canonicalizer.get(id) { + debug_assert!(linear_lhs_id != linear_lhs_id2); ( - linear::MatchOp::Eq { - path_a: path, - path_b, - }, + linear::MatchOp::Eq(linear_lhs_id, linear_lhs_id2), linear::bool_to_match_result(true), ) } else { @@ -581,7 +549,7 @@ where } Pattern::Operation(op) => { let expected = Ok(op.operator.into()); - (linear::MatchOp::Opcode { path }, expected) + (linear::MatchOp::Opcode(linear_lhs_id), expected) } } } @@ -592,7 +560,7 @@ mod tests { use super::*; use peepmatic_runtime::{ integer_interner::IntegerId, - linear::{bool_to_match_result, Action::*, Else, MatchOp::*}, + linear::{bool_to_match_result, Action::*, Else, LhsId, MatchOp::*, RhsId}, r#type::{BitWidth, Kind, Type}, unquote::UnquoteOperator, }; @@ -628,20 +596,16 @@ mod tests { panic!("should verify OK") } - let mut paths = PathInterner::new(); - let mut p = |p: &[u8]| paths.intern(Path::new(&p)); - let mut integers = IntegerInterner::new(); let mut i = |i: u64| integers.intern(i); #[allow(unused_variables)] let make_expected: fn( - &mut dyn FnMut(&[u8]) -> PathId, &mut dyn FnMut(u64) -> IntegerId, ) -> (Vec, Vec>) = $make_expected; - let expected = make_expected(&mut p, &mut i); - let actual = linearize_optimization(&mut paths, &mut integers, &opts.optimizations[0]); + let expected = make_expected(&mut i); + let actual = linearize_optimization(&mut integers, &opts.optimizations[0]); assert_eq!(expected.0, actual.matches); assert_eq!(expected.1, actual.actions); } @@ -655,10 +619,10 @@ mod tests { (is-power-of-two $C)) (ishl $x $(log2 $C))) ", - |p, i| ( + |i| ( vec![ linear::Match { - operation: Opcode { path: p(&[0]) }, + operation: Opcode(LhsId(0)), expected: Ok(TestOperator::Imul.into()), }, linear::Match { @@ -666,20 +630,20 @@ mod tests { expected: Err(Else), }, linear::Match { - operation: IsConst { path: p(&[0, 1]) }, + operation: IsConst(LhsId(2)), expected: bool_to_match_result(true), }, linear::Match { - operation: IsPowerOfTwo { path: p(&[0, 1]) }, + operation: IsPowerOfTwo(LhsId(2)), expected: bool_to_match_result(true), }, ], vec![ - GetLhs { path: p(&[0, 0]) }, - GetLhs { path: p(&[0, 1]) }, + GetLhs { lhs: LhsId(1) }, + GetLhs { lhs: LhsId(2) }, UnaryUnquote { operator: UnquoteOperator::Log2, - operand: linear::RhsId(1) + operand: RhsId(1) }, MakeBinaryInst { operator: TestOperator::Ishl, @@ -687,31 +651,31 @@ mod tests { kind: Kind::Int, bit_width: BitWidth::Polymorphic }, - operands: [linear::RhsId(0), linear::RhsId(2)] + operands: [RhsId(0), RhsId(2)] } ], ), ); - linearizes_to!(variable_pattern_id_optimization, "(=> $x $x)", |p, i| ( + linearizes_to!(variable_pattern_id_optimization, "(=> $x $x)", |i| ( vec![linear::Match { operation: Nop, expected: Err(Else), }], - vec![GetLhs { path: p(&[0]) }], + vec![GetLhs { lhs: LhsId(0) }], )); - linearizes_to!(constant_pattern_id_optimization, "(=> $C $C)", |p, i| ( + linearizes_to!(constant_pattern_id_optimization, "(=> $C $C)", |i| ( vec![linear::Match { - operation: IsConst { path: p(&[0]) }, + operation: IsConst(LhsId(0)), expected: bool_to_match_result(true), }], - vec![GetLhs { path: p(&[0]) }], + vec![GetLhs { lhs: LhsId(0) }], )); - linearizes_to!(boolean_literal_id_optimization, "(=> true true)", |p, i| ( + linearizes_to!(boolean_literal_id_optimization, "(=> true true)", |i| ( vec![linear::Match { - operation: BooleanValue { path: p(&[0]) }, + operation: BooleanValue(LhsId(0)), expected: bool_to_match_result(true), }], vec![MakeBooleanConst { @@ -720,9 +684,9 @@ mod tests { }], )); - linearizes_to!(number_literal_id_optimization, "(=> 5 5)", |p, i| ( + linearizes_to!(number_literal_id_optimization, "(=> 5 5)", |i| ( vec![linear::Match { - operation: IntegerValue { path: p(&[0]) }, + operation: IntegerValue(LhsId(0)), expected: Ok(i(5).into()), }], vec![MakeIntegerConst { @@ -734,26 +698,26 @@ mod tests { linearizes_to!( operation_id_optimization, "(=> (iconst $C) (iconst $C))", - |p, i| ( + |i| ( vec![ linear::Match { - operation: Opcode { path: p(&[0]) }, + operation: Opcode(LhsId(0)), expected: Ok(TestOperator::Iconst.into()), }, linear::Match { - operation: IsConst { path: p(&[0, 0]) }, + operation: IsConst(LhsId(1)), expected: bool_to_match_result(true), }, ], vec![ - GetLhs { path: p(&[0, 0]) }, + GetLhs { lhs: LhsId(1) }, MakeUnaryInst { operator: TestOperator::Iconst, r#type: Type { kind: Kind::Int, bit_width: BitWidth::Polymorphic, }, - operand: linear::RhsId(0), + operand: RhsId(0), }, ], ), @@ -762,10 +726,10 @@ mod tests { linearizes_to!( redundant_bor, "(=> (bor $x (bor $x $y)) (bor $x $y))", - |p, i| ( + |i| ( vec![ linear::Match { - operation: Opcode { path: p(&[0]) }, + operation: Opcode(LhsId(0)), expected: Ok(TestOperator::Bor.into()), }, linear::Match { @@ -773,14 +737,11 @@ mod tests { expected: Err(Else), }, linear::Match { - operation: Opcode { path: p(&[0, 1]) }, + operation: Opcode(LhsId(2)), expected: Ok(TestOperator::Bor.into()), }, linear::Match { - operation: Eq { - path_a: p(&[0, 1, 0]), - path_b: p(&[0, 0]), - }, + operation: Eq(LhsId(3), LhsId(1)), expected: bool_to_match_result(true), }, linear::Match { @@ -789,17 +750,15 @@ mod tests { }, ], vec![ - GetLhs { path: p(&[0, 0]) }, - GetLhs { - path: p(&[0, 1, 1]), - }, + GetLhs { lhs: LhsId(1) }, + GetLhs { lhs: LhsId(4) }, MakeBinaryInst { operator: TestOperator::Bor, r#type: Type { kind: Kind::Int, bit_width: BitWidth::Polymorphic, }, - operands: [linear::RhsId(0), linear::RhsId(1)], + operands: [RhsId(0), RhsId(1)], }, ], ), @@ -809,9 +768,9 @@ mod tests { large_integers, // u64::MAX "(=> 18446744073709551615 0)", - |p, i| ( + |i| ( vec![linear::Match { - operation: IntegerValue { path: p(&[0]) }, + operation: IntegerValue(LhsId(0)), expected: Ok(i(std::u64::MAX).into()), }], vec![MakeIntegerConst { @@ -824,14 +783,14 @@ mod tests { linearizes_to!( ireduce_with_type_ascription, "(=> (ireduce{i32} $x) 0)", - |p, i| ( + |i| ( vec![ linear::Match { - operation: Opcode { path: p(&[0]) }, + operation: Opcode(LhsId(0)), expected: Ok(TestOperator::Ireduce.into()), }, linear::Match { - operation: linear::MatchOp::BitWidth { path: p(&[0]) }, + operation: linear::MatchOp::BitWidth(LhsId(0)), expected: Ok(NonZeroU32::new(32).unwrap()), }, linear::Match { diff --git a/cranelift/peepmatic/src/traversals.rs b/cranelift/peepmatic/src/traversals.rs index f0e147d1f1..1477f6b961 100644 --- a/cranelift/peepmatic/src/traversals.rs +++ b/cranelift/peepmatic/src/traversals.rs @@ -1,6 +1,7 @@ //! Traversals over the AST. use crate::ast::*; +use std::collections::VecDeque; use std::fmt::Debug; use std::hash::Hash; @@ -103,6 +104,44 @@ where } } +/// A breadth-first traversal of an AST +/// +/// This implementation is not recursive, and exposes an `Iterator` interface +/// that yields `DynAstRef` items. +/// +/// The traversal can walk a whole set of `Optimization`s or just a subtree of +/// the AST, because the `new` constructor takes anything that can convert into +/// a `DynAstRef`. +#[derive(Clone, Debug)] +pub struct Bfs<'a, TOperator> { + queue: VecDeque>, +} + +impl<'a, TOperator> Bfs<'a, TOperator> +where + TOperator: Copy + Debug + Eq + Hash, +{ + /// Construct a new `Bfs` traversal starting at the given `start` AST node. + pub fn new(start: impl Into>) -> Self { + let mut queue = VecDeque::with_capacity(16); + queue.push_back(start.into()); + Bfs { queue } + } +} + +impl<'a, TOperator> Iterator for Bfs<'a, TOperator> +where + TOperator: Copy + Debug + Eq + Hash, +{ + type Item = DynAstRef<'a, TOperator>; + + fn next(&mut self) -> Option { + let node = self.queue.pop_front()?; + node.child_nodes(&mut self.queue); + Some(node) + } +} + #[cfg(test)] mod tests { use super::*;