peepmatic: Make the results of match operations a smaller and more cache friendly

cranelift/codegen/src/peepmatic.rs

@@ -422,7 +422,10 @@ fn peepmatic_ty_to_ir_ty(ty: Type, dfg: &DataFlowGraph, root: Inst) -> types::Ty
     }
 }
 
-impl<'a, 'b> InstructionSet<'b> for &'a dyn TargetIsa {
+// NB: the unsafe contract we must uphold here is that our implementation of
+// `instruction_result_bit_width` must always return a valid, non-zero bit
+// width.
+unsafe impl<'a, 'b> InstructionSet<'b> for &'a dyn TargetIsa {
     type Context = FuncCursor<'b>;
 
     type Instruction = ValueOrInst;

cranelift/peepmatic/crates/runtime/src/cc.rs

@@ -12,6 +12,8 @@ use std::fmt;
 #[repr(u32)]
 pub enum ConditionCode {
     /// Equal.
+    // NB: We convert `ConditionCode` into `NonZeroU32`s with unchecked
+    // conversions; memory safety relies on no variant being zero.
     Eq = 1,
 
     /// Not equal.

cranelift/peepmatic/crates/runtime/src/instruction_set.rs

@@ -21,7 +21,11 @@ use std::fmt::Debug;
 /// new `MachInst` and vcode backend easier, since all that needs to be done is
 /// "just" implementing this trait. (And probably add/modify some
 /// `peepmatic_runtime::operation::Operation`s as well).
-pub trait InstructionSet<'a> {
+///
+/// ## Safety
+///
+/// See doc comment for `instruction_result_bit_width`.
+pub unsafe trait InstructionSet<'a> {
     /// Mutable context passed into all trait methods. Can be whatever you want!
     ///
     /// In practice, this is a `FuncCursor` for `cranelift-codegen`'s trait
@@ -124,7 +128,10 @@ pub trait InstructionSet<'a> {
 
     /// Get the bit width of the given instruction's result.
     ///
-    /// Must be one of 1, 8, 16, 32, 64, or 128.
+    /// ## Safety
+    ///
+    /// There is code that makes memory-safety assumptions that the result is
+    /// always one of 1, 8, 16, 32, 64, or 128. Implementors must uphold this.
     fn instruction_result_bit_width(
         &self,
         context: &mut Self::Context,

cranelift/peepmatic/crates/runtime/src/integer_interner.rs

@@ -7,11 +7,11 @@
 
 use serde::{Deserialize, Serialize};
 use std::collections::BTreeMap;
-use std::convert::TryInto;
+use std::num::NonZeroU32;
 
 /// An identifier for an interned integer.
 #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)]
-pub struct IntegerId(#[doc(hidden)] pub u32);
+pub struct IntegerId(#[doc(hidden)] pub NonZeroU32);
 
 /// An interner for integer values.
 #[derive(Debug, Default, Serialize, Deserialize)]
@@ -40,7 +40,8 @@ impl IntegerInterner {
             return *id;
         }
 
-        let id = IntegerId(self.values.len().try_into().unwrap());
+        assert!((self.values.len() as u64) < (std::u32::MAX as u64));
+        let id = IntegerId(unsafe { NonZeroU32::new_unchecked(self.values.len() as u32 + 1) });
 
         self.values.push(value);
         self.map.insert(value, id);
@@ -59,13 +60,21 @@ impl IntegerInterner {
     /// Lookup a previously interned integer by id.
     #[inline]
     pub fn lookup(&self, id: IntegerId) -> u64 {
-        self.values[id.0 as usize]
+        let index = id.0.get() as usize - 1;
+        self.values[index]
     }
 }
 
 impl From<IntegerId> for u32 {
     #[inline]
     fn from(id: IntegerId) -> u32 {
+        id.0.get()
+    }
+}
+
+impl From<IntegerId> for NonZeroU32 {
+    #[inline]
+    fn from(id: IntegerId) -> NonZeroU32 {
         id.0
     }
 }

cranelift/peepmatic/crates/runtime/src/linear.rs

@@ -11,6 +11,7 @@ use crate::operator::{Operator, UnquoteOperator};
 use crate::paths::{PathId, PathInterner};
 use crate::r#type::{BitWidth, Type};
 use serde::{Deserialize, Serialize};
+use std::num::NonZeroU32;
 
 /// A set of linear optimizations.
 #[derive(Debug)]
@@ -32,6 +33,26 @@ pub struct Optimization {
     pub increments: Vec<Increment>,
 }
 
+/// Match any value.
+///
+/// This can be used to create fallback, wildcard-style transitions between
+/// states.
+#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)]
+pub struct Else;
+
+/// The result of evaluating a `MatchOp`.
+///
+/// This is either a specific non-zero `u32`, or a fallback that matches
+/// everything.
+pub type MatchResult = Result<NonZeroU32, Else>;
+
+/// Convert a boolean to a `MatchResult`.
+#[inline]
+pub fn bool_to_match_result(b: bool) -> MatchResult {
+    let b = b as u32;
+    unsafe { Ok(NonZeroU32::new_unchecked(b + 1)) }
+}
+
 /// An increment is a matching operation, the expected result from that
 /// operation to continue to the next increment, and the actions to take to
 /// build up the LHS scope and RHS instructions given that we got the expected
@@ -44,9 +65,9 @@ pub struct Increment {
     pub operation: MatchOp,
 
     /// The expected result of our matching operation, that enables us to
-    /// continue to the next increment. `None` is used for wildcard-style "else"
+    /// continue to the next increment, or `Else` for "don't care"
-    /// transitions.
+    /// wildcard-style matching.
-    pub expected: Option<u32>,
+    pub expected: MatchResult,
 
     /// Actions to perform, given that the operation resulted in the expected
     /// value.
@@ -217,3 +238,23 @@ pub enum Action {
         operands: [RhsId; 3],
     },
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn match_result_is_4_bytes_in_size() {
+        assert_eq!(std::mem::size_of::<MatchResult>(), 4);
+    }
+
+    #[test]
+    fn match_op_is_12_bytes_in_size() {
+        assert_eq!(std::mem::size_of::<MatchOp>(), 12);
+    }
+
+    #[test]
+    fn action_is_20_bytes_in_size() {
+        assert_eq!(std::mem::size_of::<Action>(), 20);
+    }
+}

cranelift/peepmatic/crates/runtime/src/operator.rs

@@ -20,6 +20,9 @@ use serde::{Deserialize, Serialize};
 pub enum Operator {
     /// `adjust_sp_down`
     #[peepmatic(params(iNN), result(void))]
+    // NB: We convert `Operator`s into `NonZeroU32`s with unchecked casts;
+    // memory safety relies on `Operator` starting at `1` and no variant ever
+    // being zero.
     AdjustSpDown = 1,
 
     /// `adjust_sp_down_imm`

cranelift/peepmatic/crates/runtime/src/optimizations.rs

@@ -3,7 +3,7 @@
 use crate::error::Result;
 use crate::instruction_set::InstructionSet;
 use crate::integer_interner::IntegerInterner;
-use crate::linear::{Action, MatchOp};
+use crate::linear::{Action, MatchOp, MatchResult};
 use crate::optimizer::PeepholeOptimizer;
 use crate::paths::PathInterner;
 use peepmatic_automata::Automaton;
@@ -29,7 +29,7 @@ pub struct PeepholeOptimizations {
 
     /// The underlying automata for matching optimizations' left-hand sides, and
     /// building up the corresponding right-hand side.
-    pub automata: Automaton<Option<u32>, MatchOp, Vec<Action>>,
+    pub automata: Automaton<MatchResult, MatchOp, Vec<Action>>,
 }
 
 impl PeepholeOptimizations {

cranelift/peepmatic/crates/runtime/src/optimizer.rs

@@ -1,7 +1,7 @@
 //! An optimizer for a set of peephole optimizations.
 
 use crate::instruction_set::InstructionSet;
-use crate::linear::{Action, MatchOp};
+use crate::linear::{bool_to_match_result, Action, Else, MatchOp, MatchResult};
 use crate::operator::UnquoteOperator;
 use crate::optimizations::PeepholeOptimizations;
 use crate::part::{Constant, Part};
@@ -10,6 +10,7 @@ use peepmatic_automata::State;
 use std::convert::TryFrom;
 use std::fmt::{self, Debug};
 use std::mem;
+use std::num::NonZeroU32;
 
 /// A peephole optimizer instance that can apply a set of peephole
 /// optimizations to instructions.
@@ -275,43 +276,72 @@ where
         context: &mut I::Context,
         root: I::Instruction,
         match_op: MatchOp,
-    ) -> Option<u32> {
+    ) -> MatchResult {
         use crate::linear::MatchOp::*;
 
         log::trace!("Evaluating match operation: {:?}", match_op);
-        let result = match 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)?;
+                let part = self
-                let inst = part.as_instruction()?;
+                    .instr_set
-                self.instr_set.operator(context, inst).map(|op| op as u32)
+                    .get_part_at_path(context, root, path)
+                    .ok_or(Else)?;
+                let inst = part.as_instruction().ok_or(Else)?;
+                let op = self.instr_set.operator(context, inst).ok_or(Else)?;
+                let op = op as u32;
+                debug_assert!(
+                    op != 0,
+                    "`Operator` doesn't have any variant represented
+        with zero"
+                );
+                Ok(unsafe { NonZeroU32::new_unchecked(op as u32) })
             }
             IsConst { path } => {
                 let path = self.peep_opt.paths.lookup(path);
-                let part = self.instr_set.get_part_at_path(context, root, path)?;
+                let part = self
+                    .instr_set
+                    .get_part_at_path(context, root, path)
+                    .ok_or(Else)?;
                 let is_const = match part {
                     Part::Instruction(i) => {
                         self.instr_set.instruction_to_constant(context, i).is_some()
                     }
                     Part::ConditionCode(_) | Part::Constant(_) => true,
                 };
-                Some(is_const as u32)
+                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)?;
+                let part = self
+                    .instr_set
+                    .get_part_at_path(context, root, path)
+                    .ok_or(Else)?;
                 match part {
-                    Part::Constant(c) => Some(c.as_int().unwrap().is_power_of_two() as u32),
+                    Part::Constant(c) => {
-                    Part::Instruction(i) => {
+                        let is_pow2 = c.as_int().unwrap().is_power_of_two();
-                        let c = self.instr_set.instruction_to_constant(context, i)?;
+                        bool_to_match_result(is_pow2)
-                        Some(c.as_int().unwrap().is_power_of_two() as u32)
                     }
-                    Part::ConditionCode(_) => panic!("IsPowerOfTwo on a condition code"),
+                    Part::Instruction(i) => {
+                        let c = self
+                            .instr_set
+                            .instruction_to_constant(context, i)
+                            .ok_or(Else)?;
+                        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"
+                    ),
                 }
             }
             BitWidth { path } => {
                 let path = self.peep_opt.paths.lookup(path);
-                let part = self.instr_set.get_part_at_path(context, root, path)?;
+                let part = self
+                    .instr_set
+                    .get_part_at_path(context, root, path)
+                    .ok_or(Else)?;
                 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)) => {
@@ -321,14 +351,22 @@ where
                     }
                     Part::ConditionCode(_) => panic!("BitWidth on condition code"),
                 };
-                Some(bit_width as u32)
+                debug_assert!(
+                    bit_width != 0,
+                    "`InstructionSet` implementors must uphold the contract that \
+                     `instruction_result_bit_width` returns one of 1, 8, 16, 32, 64, or 128"
+                );
+                Ok(unsafe { NonZeroU32::new_unchecked(bit_width as u32) })
             }
             FitsInNativeWord { path } => {
                 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)?;
+                let part = self
+                    .instr_set
+                    .get_part_at_path(context, root, path)
+                    .ok_or(Else)?;
                 let fits = match part {
                     Part::Instruction(i) => {
                         let size = self.instr_set.instruction_result_bit_width(context, i);
@@ -341,13 +379,19 @@ where
                     }
                     Part::ConditionCode(_) => panic!("FitsInNativeWord on condition code"),
                 };
-                Some(fits as u32)
+                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)?;
+                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)?;
+                let part_b = self
+                    .instr_set
+                    .get_part_at_path(context, root, path_b)
+                    .ok_or(Else)?;
                 let eq = match (part_a, part_b) {
                     (Part::Instruction(inst), Part::Constant(c1))
                     | (Part::Constant(c1), Part::Instruction(inst)) => {
@@ -358,43 +402,67 @@ where
                     }
                     (a, b) => a == b,
                 };
-                Some(eq as _)
+                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)?;
+                let part = self
+                    .instr_set
+                    .get_part_at_path(context, root, path)
+                    .ok_or(Else)?;
                 match part {
                     Part::Constant(c) => {
-                        let x = c.as_int()?;
+                        let x = c.as_int().ok_or(Else)?;
-                        self.peep_opt.integers.already_interned(x).map(|id| id.0)
+                        let id = self.peep_opt.integers.already_interned(x).ok_or(Else)?;
+                        Ok(id.0)
                     }
                     Part::Instruction(i) => {
-                        let c = self.instr_set.instruction_to_constant(context, i)?;
+                        let c = self
-                        let x = c.as_int()?;
+                            .instr_set
-                        self.peep_opt.integers.already_interned(x).map(|id| id.0)
+                            .instruction_to_constant(context, i)
+                            .ok_or(Else)?;
+                        let x = c.as_int().ok_or(Else)?;
+                        let id = self.peep_opt.integers.already_interned(x).ok_or(Else)?;
+                        Ok(id.0)
                     }
-                    Part::ConditionCode(_) => panic!("IntegerValue on condition code"),
+                    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)?;
+                let part = self
+                    .instr_set
+                    .get_part_at_path(context, root, path)
+                    .ok_or(Else)?;
                 match part {
-                    Part::Constant(c) => c.as_bool().map(|b| b as u32),
+                    Part::Constant(c) => {
-                    Part::Instruction(i) => {
+                        let b = c.as_bool().ok_or(Else)?;
-                        let c = self.instr_set.instruction_to_constant(context, i)?;
+                        bool_to_match_result(b)
-                        c.as_bool().map(|b| b as u32)
                     }
-                    Part::ConditionCode(_) => panic!("IntegerValue on condition code"),
+                    Part::Instruction(i) => {
+                        let c = self
+                            .instr_set
+                            .instruction_to_constant(context, i)
+                            .ok_or(Else)?;
+                        let b = c.as_bool().ok_or(Else)?;
+                        bool_to_match_result(b)
+                    }
+                    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)?;
+                let part = self
-                part.as_condition_code().map(|cc| cc as u32)
+                    .instr_set
+                    .get_part_at_path(context, root, path)
+                    .ok_or(Else)?;
+                let cc = part.as_condition_code().ok_or(Else)?;
+                let cc = cc as u32;
+                debug_assert!(cc != 0);
+                Ok(unsafe { NonZeroU32::new_unchecked(cc) })
             }
-            MatchOp::Nop => None,
+            MatchOp::Nop => Err(Else),
-        };
+        })();
         log::trace!("Evaluated match operation: {:?} = {:?}", match_op, result);
         result
     }
@@ -437,12 +505,12 @@ where
                 r#final = Some((query.current_state(), self.actions.len()));
             }
 
-            // Anything following a `None` transition doesn't care about the
+            // Anything following a `Else` transition doesn't care about the
             // result of this match operation, so if we partially follow the
-            // current non-`None` path, but don't ultimately find a matching
+            // current non-`Else` path, but don't ultimately find a matching
             // optimization, we want to be able to backtrack to this state and
-            // then try taking the `None` transition.
+            // then try taking the `Else` transition.
-            if query.has_transition_on(&None) {
+            if query.has_transition_on(&Err(Else)) {
                 self.backtracking_states
                     .push((query.current_state(), self.actions.len()));
             }
@@ -462,8 +530,8 @@ where
                 query.go_to_state(state);
                 self.actions.truncate(actions_len);
                 query
-                    .next(&None)
+                    .next(&Err(Else))
-                    .expect("backtracking states always have `None` transitions")
+                    .expect("backtracking states always have `Else` transitions")
             } else {
                 break;
             };

cranelift/peepmatic/crates/test/src/lib.rs

@@ -307,7 +307,9 @@ pub struct TestIsa {
     pub native_word_size_in_bits: u8,
 }
 
-impl<'a> InstructionSet<'a> for TestIsa {
+// Unsafe because we must ensure that `instruction_result_bit_width` never
+// returns zero.
+unsafe impl<'a> InstructionSet<'a> for TestIsa {
     type Context = Program;
 
     type Instruction = Instruction;
@@ -521,7 +523,9 @@ impl<'a> InstructionSet<'a> for TestIsa {
     fn instruction_result_bit_width(&self, program: &mut Program, inst: Instruction) -> u8 {
         log::debug!("instruction_result_bit_width({:?})", inst);
         let ty = program.data(inst).r#type;
-        ty.bit_width.fixed_width().unwrap()
+        let width = ty.bit_width.fixed_width().unwrap();
+        assert!(width != 0);
+        width
     }
 
     fn native_word_size_in_bits(&self, _program: &mut Program) -> u8 {

cranelift/peepmatic/src/automatize.rs

@@ -6,10 +6,10 @@ use peepmatic_runtime::linear;
 /// Construct an automaton from a set of linear optimizations.
 pub fn automatize(
     opts: &linear::Optimizations,
-) -> Automaton<Option<u32>, linear::MatchOp, Vec<linear::Action>> {
+) -> Automaton<linear::MatchResult, linear::MatchOp, Vec<linear::Action>> {
     debug_assert!(crate::linear_passes::is_sorted_lexicographically(opts));
 
-    let mut builder = Builder::<Option<u32>, linear::MatchOp, Vec<linear::Action>>::new();
+    let mut builder = Builder::<linear::MatchResult, linear::MatchOp, Vec<linear::Action>>::new();
 
     for opt in &opts.optimizations {
         let mut insertion = builder.insert();

cranelift/peepmatic/src/dot_fmt.rs

@@ -12,36 +12,37 @@ use peepmatic_runtime::{
 };
 use std::convert::TryFrom;
 use std::io::{self, Write};
+use std::num::NonZeroU32;
 
 #[derive(Debug)]
 pub(crate) struct PeepholeDotFmt<'a>(pub(crate) &'a PathInterner, pub(crate) &'a IntegerInterner);
 
-impl DotFmt<Option<u32>, linear::MatchOp, Vec<linear::Action>> for PeepholeDotFmt<'_> {
+impl DotFmt<linear::MatchResult, linear::MatchOp, Vec<linear::Action>> for PeepholeDotFmt<'_> {
     fn fmt_transition(
         &self,
         w: &mut impl Write,
         from: Option<&linear::MatchOp>,
-        input: &Option<u32>,
+        input: &linear::MatchResult,
         _to: Option<&linear::MatchOp>,
     ) -> io::Result<()> {
         let from = from.expect("we should have match op for every state");
-        if let Some(x) = input {
+        if let Some(x) = input.ok().map(|x| x.get()) {
             match from {
                 linear::MatchOp::Opcode { .. } => {
                     let opcode =
-                        Operator::try_from(*x).expect("we shouldn't generate non-opcode edges");
+                        Operator::try_from(x).expect("we shouldn't generate non-opcode edges");
                     write!(w, "{}", opcode)
                 }
                 linear::MatchOp::ConditionCode { .. } => {
                     let cc =
-                        ConditionCode::try_from(*x).expect("we shouldn't generate non-CC edges");
+                        ConditionCode::try_from(x).expect("we shouldn't generate non-CC edges");
                     write!(w, "{}", cc)
                 }
                 linear::MatchOp::IntegerValue { .. } => {
-                    let x = self.1.lookup(IntegerId(*x));
+                    let x = self.1.lookup(IntegerId(NonZeroU32::new(x).unwrap()));
                     write!(w, "{}", x)
                 }
-                _ => write!(w, "{}", x),
+                _ => write!(w, "Ok({})", x),
             }
         } else {
             write!(w, "(else)")

cranelift/peepmatic/src/linear_passes.rs

@@ -65,7 +65,12 @@ fn compare_optimizations(
             return c;
         }
 
-        let c = a.expected.cmp(&b.expected).reverse();
+        let c = match (a.expected, b.expected) {
+            (Ok(a), Ok(b)) => a.cmp(&b).reverse(),
+            (Err(_), Ok(_)) => Ordering::Greater,
+            (Ok(_), Err(_)) => Ordering::Less,
+            (Err(linear::Else), Err(linear::Else)) => Ordering::Equal,
+        };
         if c != Ordering::Equal {
             return c;
         }
@@ -228,10 +233,10 @@ pub fn match_in_same_order(opts: &mut linear::Optimizations) {
                 Some(_) => {
                     new_increments.push(linear::Increment {
                         operation: *last_op,
-                        expected: None,
+                        expected: Err(linear::Else),
                         actions: vec![],
                     });
-                    if last_expected.is_some() {
+                    if last_expected.is_ok() {
                         break;
                     }
                 }
@@ -282,12 +287,99 @@ pub fn remove_unnecessary_nops(opts: &mut linear::Optimizations) {
 mod tests {
     use super::*;
     use crate::ast::*;
-    use linear::MatchOp::*;
+    use peepmatic_runtime::{
-    use peepmatic_runtime::{operator::Operator, paths::*};
+        linear::{bool_to_match_result, Else, MatchOp::*, MatchResult},
+        operator::Operator,
+        paths::*,
+    };
+    use std::num::NonZeroU32;
+
+    #[test]
+    fn ok_non_zero_less_than_err_else() {
+        assert!(Ok(NonZeroU32::new(1).unwrap()) < Err(Else));
+    }
 
     macro_rules! sorts_to {
         ($test_name:ident, $source:expr, $make_expected:expr) => {
             #[test]
+            #[allow(unused_variables)]
+            fn $test_name() {
+                let buf = wast::parser::ParseBuffer::new($source).expect("should lex OK");
+
+                let opts = match wast::parser::parse::<Optimizations>(&buf) {
+                    Ok(opts) => opts,
+                    Err(mut e) => {
+                        e.set_path(std::path::Path::new(stringify!($test_name)));
+                        e.set_text($source);
+                        eprintln!("{}", e);
+                        panic!("should parse OK")
+                    }
+                };
+
+                if let Err(mut e) = crate::verify(&opts) {
+                    e.set_path(std::path::Path::new(stringify!($test_name)));
+                    e.set_text($source);
+                    eprintln!("{}", e);
+                    panic!("should verify OK")
+                }
+
+                let mut opts = crate::linearize(&opts);
+
+                let before = opts
+                    .optimizations
+                    .iter()
+                    .map(|o| {
+                        o.increments
+                            .iter()
+                            .map(|i| format!("{:?} == {:?}", i.operation, i.expected))
+                            .collect::<Vec<_>>()
+                    })
+                    .collect::<Vec<_>>();
+                eprintln!("before = {:#?}", before);
+
+                sort_least_to_most_general(&mut opts);
+
+                let after = opts
+                    .optimizations
+                    .iter()
+                    .map(|o| {
+                        o.increments
+                            .iter()
+                            .map(|i| format!("{:?} == {:?}", i.operation, i.expected))
+                            .collect::<Vec<_>>()
+                    })
+                    .collect::<Vec<_>>();
+                eprintln!("after = {:#?}", before);
+
+                let linear::Optimizations {
+                    mut paths,
+                    mut integers,
+                    optimizations,
+                } = opts;
+
+                let actual: Vec<Vec<_>> = optimizations
+                    .iter()
+                    .map(|o| {
+                        o.increments
+                            .iter()
+                            .map(|i| (i.operation, i.expected))
+                            .collect()
+                    })
+                    .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);
+
+                assert_eq!(expected, actual);
+            }
+        };
+    }
+
+    macro_rules! match_in_same_order {
+        ($test_name:ident, $source:expr, $make_expected:expr) => {
+            #[test]
+            #[allow(unused_variables)]
             fn $test_name() {
                 let buf = wast::parser::ParseBuffer::new($source).expect("should lex OK");
 
@@ -311,6 +403,32 @@ mod tests {
                 let mut opts = crate::linearize(&opts);
                 sort_least_to_most_general(&mut opts);
 
+                let before = opts
+                    .optimizations
+                    .iter()
+                    .map(|o| {
+                        o.increments
+                            .iter()
+                            .map(|i| format!("{:?} == {:?}", i.operation, i.expected))
+                            .collect::<Vec<_>>()
+                    })
+                    .collect::<Vec<_>>();
+                eprintln!("before = {:#?}", before);
+
+                match_in_same_order(&mut opts);
+
+                let after = opts
+                    .optimizations
+                    .iter()
+                    .map(|o| {
+                        o.increments
+                            .iter()
+                            .map(|i| format!("{:?} == {:?}", i.operation, i.expected))
+                            .collect::<Vec<_>>()
+                    })
+                    .collect::<Vec<_>>();
+                eprintln!("after = {:#?}", before);
+
                 let linear::Optimizations {
                     mut paths,
                     mut integers,
@@ -328,7 +446,7 @@ mod tests {
                     .collect();
 
                 let mut p = |p: &[u8]| paths.intern(Path::new(&p));
-                let mut i = |i: u64| Some(integers.intern(i).into());
+                let mut i = |i: u64| Ok(integers.intern(i).into());
                 let expected = $make_expected(&mut p, &mut i);
 
                 assert_eq!(expected, actual);
@@ -348,53 +466,83 @@ mod tests {
 (=>       (iadd $x 42)                       0)
 (=>       (iadd $x (iadd $y $z))             0)
 ",
-        |p: &mut dyn FnMut(&[u8]) -> PathId, i: &mut dyn FnMut(u64) -> Option<u32>| vec![
+        |p: &mut dyn FnMut(&[u8]) -> PathId, i: &mut dyn FnMut(u64) -> MatchResult| vec![
             vec![
-                (Opcode { path: p(&[0]) }, Some(Operator::Iadd as _)),
+                (
-                (Nop, None),
+                    Opcode { path: p(&[0]) },
-                (Opcode { path: p(&[0, 1]) }, Some(Operator::Iadd as _)),
+                    Ok(NonZeroU32::new(Operator::Iadd as _).unwrap())
-                (Nop, None),
+                ),
-                (Nop, None),
+                (Nop, Err(Else)),
+                (
+                    Opcode { path: p(&[0, 1]) },
+                    Ok(NonZeroU32::new(Operator::Iadd as _).unwrap())
+                ),
+                (Nop, Err(Else)),
+                (Nop, Err(Else)),
             ],
             vec![
-                (Opcode { path: p(&[0]) }, Some(Operator::Iadd as _)),
+                (
-                (Nop, None),
+                    Opcode { path: p(&[0]) },
+                    Ok(NonZeroU32::new(Operator::Iadd as _).unwrap())
+                ),
+                (Nop, Err(Else)),
                 (IntegerValue { path: p(&[0, 1]) }, i(42))
             ],
             vec![
-                (Opcode { path: p(&[0]) }, Some(Operator::Iadd as _)),
+                (
-                (Nop, None),
+                    Opcode { path: p(&[0]) },
-                (IsConst { path: p(&[0, 1]) }, Some(1)),
+                    Ok(NonZeroU32::new(Operator::Iadd as _).unwrap())
-                (IsPowerOfTwo { path: p(&[0, 1]) }, Some(1))
+                ),
+                (Nop, Err(Else)),
+                (IsConst { path: p(&[0, 1]) }, bool_to_match_result(true)),
+                (
+                    IsPowerOfTwo { path: p(&[0, 1]) },
+                    bool_to_match_result(true)
+                )
             ],
             vec![
-                (Opcode { path: p(&[0]) }, Some(Operator::Iadd as _)),
+                (
-                (Nop, None),
+                    Opcode { path: p(&[0]) },
-                (IsConst { path: p(&[0, 1]) }, Some(1)),
+                    Ok(NonZeroU32::new(Operator::Iadd as _).unwrap())
-                (BitWidth { path: p(&[0, 0]) }, Some(32))
+                ),
+                (Nop, Err(Else)),
+                (IsConst { path: p(&[0, 1]) }, bool_to_match_result(true)),
+                (
+                    BitWidth { path: p(&[0, 0]) },
+                    Ok(NonZeroU32::new(32).unwrap())
+                )
             ],
             vec![
-                (Opcode { path: p(&[0]) }, Some(Operator::Iadd as _)),
+                (
-                (Nop, None),
+                    Opcode { path: p(&[0]) },
-                (IsConst { path: p(&[0, 1]) }, Some(1))
+                    Ok(NonZeroU32::new(Operator::Iadd as _).unwrap())
+                ),
+                (Nop, Err(Else)),
+                (IsConst { path: p(&[0, 1]) }, bool_to_match_result(true))
             ],
             vec![
-                (Opcode { path: p(&[0]) }, Some(Operator::Iadd as _)),
+                (
-                (Nop, None),
+                    Opcode { path: p(&[0]) },
+                    Ok(NonZeroU32::new(Operator::Iadd as _).unwrap())
+                ),
+                (Nop, Err(Else)),
                 (
                     Eq {
                         path_a: p(&[0, 1]),
                         path_b: p(&[0, 0]),
                     },
-                    Some(1)
+                    bool_to_match_result(true)
                 )
             ],
             vec![
-                (Opcode { path: p(&[0]) }, Some(Operator::Iadd as _)),
+                (
-                (Nop, None),
+                    Opcode { path: p(&[0]) },
-                (Nop, None),
+                    Ok(NonZeroU32::new(Operator::Iadd as _).unwrap())
+                ),
+                (Nop, Err(Else)),
+                (Nop, Err(Else)),
             ],
-            vec![(Nop, None)]
+            vec![(Nop, Err(Else))]
         ]
     );
 
@@ -408,37 +556,164 @@ 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) -> Option<u32>| vec![
+        |p: &mut dyn FnMut(&[u8]) -> PathId, i: &mut dyn FnMut(u64) -> MatchResult| vec![
             vec![
-                (Opcode { path: p(&[0]) }, Some(Operator::Imul as _)),
+                (
+                    Opcode { path: p(&[0]) },
+                    Ok(NonZeroU32::new(Operator::Imul as _).unwrap())
+                ),
                 (IntegerValue { path: p(&[0, 0]) }, i(2)),
-                (Nop, None)
+                (Nop, Err(Else))
             ],
             vec![
-                (Opcode { path: p(&[0]) }, Some(Operator::Imul as _)),
+                (
+                    Opcode { path: p(&[0]) },
+                    Ok(NonZeroU32::new(Operator::Imul as _).unwrap())
+                ),
                 (IntegerValue { path: p(&[0, 0]) }, i(1)),
-                (Nop, None)
+                (Nop, Err(Else))
             ],
             vec![
-                (Opcode { path: p(&[0]) }, Some(Operator::Imul as _)),
+                (
-                (Nop, None),
+                    Opcode { path: p(&[0]) },
+                    Ok(NonZeroU32::new(Operator::Imul as _).unwrap())
+                ),
+                (Nop, Err(Else)),
                 (IntegerValue { path: p(&[0, 1]) }, i(2))
             ],
             vec![
-                (Opcode { path: p(&[0]) }, Some(Operator::Imul as _)),
+                (
-                (Nop, None),
+                    Opcode { path: p(&[0]) },
+                    Ok(NonZeroU32::new(Operator::Imul as _).unwrap())
+                ),
+                (Nop, Err(Else)),
                 (IntegerValue { path: p(&[0, 1]) }, i(1))
             ],
             vec![
-                (Opcode { path: p(&[0]) }, Some(Operator::Iadd as _)),
+                (
+                    Opcode { path: p(&[0]) },
+                    Ok(NonZeroU32::new(Operator::Iadd as _).unwrap())
+                ),
                 (IntegerValue { path: p(&[0, 0]) }, i(0)),
-                (Nop, None)
+                (Nop, Err(Else))
             ],
             vec![
-                (Opcode { path: p(&[0]) }, Some(Operator::Iadd as _)),
+                (
-                (Nop, None),
+                    Opcode { path: p(&[0]) },
+                    Ok(NonZeroU32::new(Operator::Iadd as _).unwrap())
+                ),
+                (Nop, Err(Else)),
                 (IntegerValue { path: p(&[0, 1]) }, i(0))
             ]
         ]
     );
+
+    sorts_to!(
+        sort_redundant_bor,
+        "
+        (=> (bor (bor $x $y) $x)
+            (bor $x $y))
+
+        (=> (bor (bor $x $y) $y)
+            (bor $x $y))
+        ",
+        |p: &mut dyn FnMut(&[u8]) -> PathId, i: &mut dyn FnMut(u64) -> MatchResult| vec![
+            vec![
+                (
+                    Opcode { path: p(&[0]) },
+                    Ok(NonZeroU32::new(Operator::Bor as _).unwrap())
+                ),
+                (
+                    Opcode { path: p(&[0, 0]) },
+                    Ok(NonZeroU32::new(Operator::Bor as _).unwrap())
+                ),
+                (Nop, Err(Else)),
+                (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(NonZeroU32::new(Operator::Bor as _).unwrap())
+                ),
+                (
+                    Opcode { path: p(&[0, 0]) },
+                    Ok(NonZeroU32::new(Operator::Bor as _).unwrap())
+                ),
+                (Nop, Err(Else)),
+                (Nop, Err(Else)),
+                (
+                    Eq {
+                        path_a: p(&[0, 1]),
+                        path_b: p(&[0, 0, 1]),
+                    },
+                    bool_to_match_result(true)
+                ),
+            ],
+        ]
+    );
+
+    match_in_same_order!(
+        match_in_same_order_redundant_bor,
+        "
+        (=> (bor (bor $x $y) $x)
+            (bor $x $y))
+
+        (=> (bor (bor $x $y) $y)
+            (bor $x $y))
+        ",
+        |p: &mut dyn FnMut(&[u8]) -> PathId, i: &mut dyn FnMut(u64) -> MatchResult| vec![
+            vec![
+                (
+                    Opcode { path: p(&[0]) },
+                    Ok(NonZeroU32::new(Operator::Bor as _).unwrap())
+                ),
+                (
+                    Opcode { path: p(&[0, 0]) },
+                    Ok(NonZeroU32::new(Operator::Bor as _).unwrap())
+                ),
+                (Nop, Err(Else)),
+                (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(NonZeroU32::new(Operator::Bor as _).unwrap())
+                ),
+                (
+                    Opcode { path: p(&[0, 0]) },
+                    Ok(NonZeroU32::new(Operator::Bor as _).unwrap())
+                ),
+                (Nop, 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)
+                ),
+            ],
+        ]
+    );
 }

cranelift/peepmatic/src/linearize.rs

@@ -92,6 +92,7 @@ use peepmatic_runtime::{
     paths::{Path, PathId, PathInterner},
 };
 use std::collections::BTreeMap;
+use std::num::NonZeroU32;
 use wast::Id;
 
 /// Translate the given AST optimizations into linear optimizations.
@@ -142,9 +143,12 @@ fn linearize_optimization(
         // increment that checks the instruction-being-matched's bit width.
         if let Pattern::Operation(Operation { r#type, .. }) = pattern {
             if let Some(w) = r#type.get().and_then(|ty| ty.bit_width.fixed_width()) {
+                debug_assert!(w != 0, "All fixed-width bit widths are non-zero");
+                let expected = Ok(unsafe { NonZeroU32::new_unchecked(w as u32) });
+
                 increments.push(linear::Increment {
                     operation: linear::MatchOp::BitWidth { path },
-                    expected: Some(w as u32),
+                    expected,
                     actions: vec![],
                 });
             }
@@ -432,7 +436,7 @@ impl Precondition<'_> {
                 let path = lhs_id_to_path.unwrap_first_occurrence(&id);
                 linear::Increment {
                     operation: linear::MatchOp::IsPowerOfTwo { path },
-                    expected: Some(1),
+                    expected: linear::bool_to_match_result(true),
                     actions: vec![],
                 }
             }
@@ -451,12 +455,14 @@ impl Precondition<'_> {
                     })) => *value,
                     _ => unreachable!("checked in verification"),
                 };
-                debug_assert!(width <= 128);
+
-                debug_assert!((width as u8).is_power_of_two());
+                assert!(0 < width && width <= 128);
+                assert!((width as u8).is_power_of_two());
+                let expected = Ok(unsafe { NonZeroU32::new_unchecked(width as u32) });
 
                 linear::Increment {
                     operation: linear::MatchOp::BitWidth { path },
-                    expected: Some(width as u32),
+                    expected,
                     actions: vec![],
                 }
             }
@@ -469,7 +475,7 @@ impl Precondition<'_> {
                 let path = lhs_id_to_path.unwrap_first_occurrence(&id);
                 linear::Increment {
                     operation: linear::MatchOp::FitsInNativeWord { path },
-                    expected: Some(1),
+                    expected: linear::bool_to_match_result(true),
                     actions: vec![],
                 }
             }
@@ -488,17 +494,21 @@ impl Pattern<'_> {
         integers: &mut IntegerInterner,
         lhs_id_to_path: &LhsIdToPath,
         path: PathId,
-    ) -> (linear::MatchOp, Option<u32>) {
+    ) -> (linear::MatchOp, linear::MatchResult) {
         match self {
             Pattern::ValueLiteral(ValueLiteral::Integer(Integer { value, .. })) => (
                 linear::MatchOp::IntegerValue { path },
-                Some(integers.intern(*value as u64).into()),
+                Ok(integers.intern(*value as u64).into()),
+            ),
+            Pattern::ValueLiteral(ValueLiteral::Boolean(Boolean { value, .. })) => (
+                linear::MatchOp::BooleanValue { path },
+                linear::bool_to_match_result(*value),
             ),
-            Pattern::ValueLiteral(ValueLiteral::Boolean(Boolean { value, .. })) => {
-                (linear::MatchOp::BooleanValue { path }, Some(*value as u32))
-            }
             Pattern::ValueLiteral(ValueLiteral::ConditionCode(ConditionCode { cc, .. })) => {
-                (linear::MatchOp::ConditionCode { path }, Some(*cc as u32))
+                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)
             }
             Pattern::Constant(Constant { id, .. }) => {
                 if let Some(path_b) = lhs_id_to_path.get_first_occurrence(id) {
@@ -508,10 +518,13 @@ impl Pattern<'_> {
                             path_a: path,
                             path_b,
                         },
-                        Some(1),
+                        linear::bool_to_match_result(true),
                     )
                 } else {
-                    (linear::MatchOp::IsConst { path }, Some(1))
+                    (
+                        linear::MatchOp::IsConst { path },
+                        linear::bool_to_match_result(true),
+                    )
                 }
             }
             Pattern::Variable(Variable { id, .. }) => {
@@ -522,13 +535,18 @@ impl Pattern<'_> {
                             path_a: path,
                             path_b,
                         },
-                        Some(1),
+                        linear::bool_to_match_result(true),
                     )
                 } else {
-                    (linear::MatchOp::Nop, None)
+                    (linear::MatchOp::Nop, Err(linear::Else))
                 }
             }
-            Pattern::Operation(op) => (linear::MatchOp::Opcode { path }, Some(op.operator as u32)),
+            Pattern::Operation(op) => {
+                let op = op.operator as u32;
+                debug_assert!(op != 0, "no `Operator` variants are zero");
+                let expected = Ok(unsafe { NonZeroU32::new_unchecked(op) });
+                (linear::MatchOp::Opcode { path }, expected)
+            }
         }
     }
 }
@@ -538,7 +556,7 @@ mod tests {
     use super::*;
     use peepmatic_runtime::{
         integer_interner::IntegerId,
-        linear::{Action::*, MatchOp::*},
+        linear::{bool_to_match_result, Action::*, Else, MatchOp::*},
         operator::Operator,
         r#type::{BitWidth, Kind, Type},
     };
@@ -603,7 +621,7 @@ mod tests {
                 increments: vec![
                     linear::Increment {
                         operation: Opcode { path: p(&[0]) },
-                        expected: Some(Operator::Imul as _),
+                        expected: Ok(NonZeroU32::new(Operator::Imul as _).unwrap()),
                         actions: vec![
                             GetLhs { path: p(&[0, 0]) },
                             GetLhs { path: p(&[0, 1]) },
@@ -619,17 +637,17 @@ mod tests {
                     },
                     linear::Increment {
                         operation: Nop,
-                        expected: None,
+                        expected: Err(Else),
                         actions: vec![],
                     },
                     linear::Increment {
                         operation: IsConst { path: p(&[0, 1]) },
-                        expected: Some(1),
+                        expected: bool_to_match_result(true),
                         actions: vec![],
                     },
                     linear::Increment {
                         operation: IsPowerOfTwo { path: p(&[0, 1]) },
-                        expected: Some(1),
+                        expected: bool_to_match_result(true),
                         actions: vec![],
                     },
                 ],
@@ -644,7 +662,7 @@ mod tests {
             linear::Optimization {
                 increments: vec![linear::Increment {
                     operation: Nop,
-                    expected: None,
+                    expected: Err(Else),
                     actions: vec![GetLhs { path: p(&[0]) }],
                 }],
             }
@@ -658,7 +676,7 @@ mod tests {
             linear::Optimization {
                 increments: vec![linear::Increment {
                     operation: IsConst { path: p(&[0]) },
-                    expected: Some(1),
+                    expected: bool_to_match_result(true),
                     actions: vec![GetLhs { path: p(&[0]) }],
                 }],
             }
@@ -672,7 +690,7 @@ mod tests {
             linear::Optimization {
                 increments: vec![linear::Increment {
                     operation: BooleanValue { path: p(&[0]) },
-                    expected: Some(1),
+                    expected: bool_to_match_result(true),
                     actions: vec![MakeBooleanConst {
                         value: true,
                         bit_width: BitWidth::Polymorphic,
@@ -689,7 +707,7 @@ mod tests {
             linear::Optimization {
                 increments: vec![linear::Increment {
                     operation: IntegerValue { path: p(&[0]) },
-                    expected: Some(i(5).into()),
+                    expected: Ok(i(5).into()),
                     actions: vec![MakeIntegerConst {
                         value: i(5),
                         bit_width: BitWidth::Polymorphic,
@@ -707,7 +725,7 @@ mod tests {
                 increments: vec![
                     linear::Increment {
                         operation: Opcode { path: p(&[0]) },
-                        expected: Some(Operator::Iconst as _),
+                        expected: Ok(NonZeroU32::new(Operator::Iconst as _).unwrap()),
                         actions: vec![
                             GetLhs { path: p(&[0, 0]) },
                             MakeUnaryInst {
@@ -722,7 +740,7 @@ mod tests {
                     },
                     linear::Increment {
                         operation: IsConst { path: p(&[0, 0]) },
-                        expected: Some(1),
+                        expected: bool_to_match_result(true),
                         actions: vec![],
                     },
                 ],
@@ -738,7 +756,7 @@ mod tests {
                 increments: vec![
                     linear::Increment {
                         operation: Opcode { path: p(&[0]) },
-                        expected: Some(Operator::Bor as _),
+                        expected: Ok(NonZeroU32::new(Operator::Bor as _).unwrap()),
                         actions: vec![
                             GetLhs { path: p(&[0, 0]) },
                             GetLhs {
@@ -756,12 +774,12 @@ mod tests {
                     },
                     linear::Increment {
                         operation: Nop,
-                        expected: None,
+                        expected: Err(Else),
                         actions: vec![],
                     },
                     linear::Increment {
                         operation: Opcode { path: p(&[0, 1]) },
-                        expected: Some(Operator::Bor as _),
+                        expected: Ok(NonZeroU32::new(Operator::Bor as _).unwrap()),
                         actions: vec![],
                     },
                     linear::Increment {
@@ -769,12 +787,12 @@ mod tests {
                             path_a: p(&[0, 1, 0]),
                             path_b: p(&[0, 0]),
                         },
-                        expected: Some(1),
+                        expected: bool_to_match_result(true),
                         actions: vec![],
                     },
                     linear::Increment {
                         operation: Nop,
-                        expected: None,
+                        expected: Err(Else),
                         actions: vec![],
                     },
                 ],
@@ -790,7 +808,7 @@ mod tests {
             linear::Optimization {
                 increments: vec![linear::Increment {
                     operation: IntegerValue { path: p(&[0]) },
-                    expected: Some(i(std::u64::MAX).into()),
+                    expected: Ok(i(std::u64::MAX).into()),
                     actions: vec![MakeIntegerConst {
                         value: i(0),
                         bit_width: BitWidth::Polymorphic,
@@ -808,7 +826,7 @@ mod tests {
                 increments: vec![
                     linear::Increment {
                         operation: Opcode { path: p(&[0]) },
-                        expected: Some(Operator::Ireduce as _),
+                        expected: Ok(NonZeroU32::new(Operator::Ireduce as _).unwrap()),
                         actions: vec![MakeIntegerConst {
                             value: i(0),
                             bit_width: BitWidth::ThirtyTwo,
@@ -816,12 +834,12 @@ mod tests {
                     },
                     linear::Increment {
                         operation: linear::MatchOp::BitWidth { path: p(&[0]) },
-                        expected: Some(32),
+                        expected: Ok(NonZeroU32::new(32).unwrap()),
                         actions: vec![],
                     },
                     linear::Increment {
                         operation: Nop,
-                        expected: None,
+                        expected: Err(Else),
                         actions: vec![],
                     },
                 ],