Reduce cmp and cmp+br instructions (#1243)

* rename cmp exec handlers with 16-bit `rhs` value

* rename helper method

* add less-than (or equal) cmp instructions with 16-bit `lhs` immediate

* add new cmp+branch instrs with 16-bit lhs immediate value

* translate {gt,ge} cmp instrs as {lt,le} cmp instrs

* remove no longer needed cmp and cmp+br instrs

* adjust unit tests for removals

* add Instruction::result convenience method

* add some comments where missing

* add more br_if translation tests

* refactor and clean-up cmp+br instr fusion

* refactor + add more cmp+if_eqz instr fusion

* remove no longer needed Comparator variants

* add more translation tests for i32_eqz

crates/ir/src/enum.rs

@@ -79,6 +79,63 @@ macro_rules! define_enum {
 }
 for_each_op::for_each_op!(define_enum);
 
+/// Helper trait for [`Instruction::result`] method implementation.
+trait IntoReg: Sized {
+    /// Converts `self` into a [`Reg`] if possible.
+    fn into_reg(self) -> Option<Reg> {
+        None
+    }
+}
+
+impl IntoReg for Reg {
+    fn into_reg(self) -> Option<Reg> {
+        Some(self)
+    }
+}
+impl IntoReg for RegSpan {}
+impl<const N: u16> IntoReg for FixedRegSpan<N> {}
+impl IntoReg for () {}
+
+macro_rules! define_result {
+    (
+        $(
+            $( #[doc = $doc:literal] )*
+            #[snake_name($snake_name:ident)]
+            $name:ident
+            $(
+                {
+                    $(
+                        @ $result_name:ident: $result_ty:ty,
+                    )?
+                    $(
+                        $( #[$field_docs:meta] )*
+                        $field_name:ident: $field_ty:ty
+                    ),*
+                    $(,)?
+                }
+            )?
+        ),* $(,)?
+    ) => {
+        impl Instruction {
+            /// Returns the result [`Reg`] for `self`.
+            ///
+            /// Returns `None` if `self` does not statically return a single [`Reg`].
+            pub fn result(&self) -> Option<$crate::Reg> {
+                match *self {
+                    $(
+                        Self::$name { $( $( $result_name, )? )* .. } => {
+                            IntoReg::into_reg((
+                                $( $( $result_name )? )*
+                            ))
+                        }
+                    )*
+                }
+            }
+        }
+    };
+}
+for_each_op::for_each_op!(define_result);
+
 impl Instruction {
     /// Creates a new [`Instruction::ReturnReg2`] for the given [`Reg`] indices.
     pub fn return_reg2_ext(reg0: impl Into<Reg>, reg1: impl Into<Reg>) -> Self {

crates/ir/src/primitive.rs

@@ -244,10 +244,6 @@ macro_rules! for_each_comparator {
             I32LtU,
             I32LeS,
             I32LeU,
-            I32GtS,
-            I32GtU,
-            I32GeS,
-            I32GeU,
 
             I32And,
             I32Or,
@@ -262,23 +258,16 @@ macro_rules! for_each_comparator {
             I64LtU,
             I64LeS,
             I64LeU,
-            I64GtS,
-            I64GtU,
-            I64GeS,
-            I64GeU,
 
             F32Eq,
             F32Ne,
             F32Lt,
             F32Le,
-            F32Gt,
-            F32Ge,
+
             F64Eq,
             F64Ne,
             F64Lt,
             F64Le,
-            F64Gt,
-            F64Ge,
         }
     };
 }

crates/wasmi/src/engine/executor/instrs/binary.rs

@@ -73,7 +73,7 @@ macro_rules! impl_binary_imm16 {
         $(
             #[doc = concat!("Executes an [`Instruction::", stringify!($var_name), "`].")]
             pub fn $fn_name(&mut self, result: Reg, lhs: Reg, rhs: Const16<$ty>) {
-                self.execute_binary_imm16(result, lhs, rhs, $op)
+                self.execute_binary_imm16_rhs(result, lhs, rhs, $op)
             }
         )*
     };

crates/wasmi/src/engine/executor/instrs/branch.rs

@@ -214,7 +214,7 @@ impl Executor<'_> {
     }
 
     /// Executes a generic fused compare and branch instruction with immediate `rhs` operand.
-    fn execute_branch_binop_imm<T>(
+    fn execute_branch_binop_imm16_rhs<T>(
         &mut self,
         lhs: Reg,
         rhs: Const16<T>,
@@ -230,6 +230,24 @@ impl Executor<'_> {
         }
         self.next_instr()
     }
+
+    /// Executes a generic fused compare and branch instruction with immediate `rhs` operand.
+    fn execute_branch_binop_imm16_lhs<T>(
+        &mut self,
+        lhs: Const16<T>,
+        rhs: Reg,
+        offset: BranchOffset16,
+        f: fn(T, T) -> bool,
+    ) where
+        T: From<UntypedVal> + From<Const16<T>>,
+    {
+        let lhs = T::from(lhs);
+        let rhs: T = self.get_register_as(rhs);
+        if f(lhs, rhs) {
+            return self.branch_to16(offset);
+        }
+        self.next_instr()
+    }
 }
 
 fn cmp_eq<T>(a: T, b: T) -> bool
@@ -260,20 +278,6 @@ where
     a <= b
 }
 
-fn cmp_gt<T>(a: T, b: T) -> bool
-where
-    T: PartialOrd,
-{
-    a > b
-}
-
-fn cmp_ge<T>(a: T, b: T) -> bool
-where
-    T: PartialOrd,
-{
-    a >= b
-}
-
 fn cmp_i32_and(a: i32, b: i32) -> bool {
     (a & b) != 0
 }
@@ -323,50 +327,38 @@ impl_execute_branch_binop! {
     (u32, Instruction::BranchI32LtU, execute_branch_i32_lt_u, cmp_lt),
     (i32, Instruction::BranchI32LeS, execute_branch_i32_le_s, cmp_le),
     (u32, Instruction::BranchI32LeU, execute_branch_i32_le_u, cmp_le),
-    (i32, Instruction::BranchI32GtS, execute_branch_i32_gt_s, cmp_gt),
-    (u32, Instruction::BranchI32GtU, execute_branch_i32_gt_u, cmp_gt),
-    (i32, Instruction::BranchI32GeS, execute_branch_i32_ge_s, cmp_ge),
-    (u32, Instruction::BranchI32GeU, execute_branch_i32_ge_u, cmp_ge),
 
     (i64, Instruction::BranchI64Eq, execute_branch_i64_eq, cmp_eq),
     (i64, Instruction::BranchI64Ne, execute_branch_i64_ne, cmp_ne),
     (i64, Instruction::BranchI64LtS, execute_branch_i64_lt_s, cmp_lt),
     (u64, Instruction::BranchI64LtU, execute_branch_i64_lt_u, cmp_lt),
     (i64, Instruction::BranchI64LeS, execute_branch_i64_le_s, cmp_le),
     (u64, Instruction::BranchI64LeU, execute_branch_i64_le_u, cmp_le),
-    (i64, Instruction::BranchI64GtS, execute_branch_i64_gt_s, cmp_gt),
-    (u64, Instruction::BranchI64GtU, execute_branch_i64_gt_u, cmp_gt),
-    (i64, Instruction::BranchI64GeS, execute_branch_i64_ge_s, cmp_ge),
-    (u64, Instruction::BranchI64GeU, execute_branch_i64_ge_u, cmp_ge),
 
     (f32, Instruction::BranchF32Eq, execute_branch_f32_eq, cmp_eq),
     (f32, Instruction::BranchF32Ne, execute_branch_f32_ne, cmp_ne),
     (f32, Instruction::BranchF32Lt, execute_branch_f32_lt, cmp_lt),
     (f32, Instruction::BranchF32Le, execute_branch_f32_le, cmp_le),
-    (f32, Instruction::BranchF32Gt, execute_branch_f32_gt, cmp_gt),
-    (f32, Instruction::BranchF32Ge, execute_branch_f32_ge, cmp_ge),
 
     (f64, Instruction::BranchF64Eq, execute_branch_f64_eq, cmp_eq),
     (f64, Instruction::BranchF64Ne, execute_branch_f64_ne, cmp_ne),
     (f64, Instruction::BranchF64Lt, execute_branch_f64_lt, cmp_lt),
     (f64, Instruction::BranchF64Le, execute_branch_f64_le, cmp_le),
-    (f64, Instruction::BranchF64Gt, execute_branch_f64_gt, cmp_gt),
-    (f64, Instruction::BranchF64Ge, execute_branch_f64_ge, cmp_ge),
 }
 
-macro_rules! impl_execute_branch_binop_imm {
+macro_rules! impl_execute_branch_binop_imm16_rhs {
     ( $( ($ty:ty, Instruction::$op_name:ident, $fn_name:ident, $op:expr) ),* $(,)? ) => {
         impl<'engine> Executor<'engine> {
             $(
                 #[doc = concat!("Executes an [`Instruction::", stringify!($op_name), "`].")]
                 pub fn $fn_name(&mut self, lhs: Reg, rhs: Const16<$ty>, offset: BranchOffset16) {
-                    self.execute_branch_binop_imm::<$ty>(lhs, rhs, offset, $op)
+                    self.execute_branch_binop_imm16_rhs::<$ty>(lhs, rhs, offset, $op)
                 }
             )*
         }
     }
 }
-impl_execute_branch_binop_imm! {
+impl_execute_branch_binop_imm16_rhs! {
     (i32, Instruction::BranchI32AndImm16, execute_branch_i32_and_imm16, cmp_i32_and),
     (i32, Instruction::BranchI32OrImm16, execute_branch_i32_or_imm16, cmp_i32_or),
     (i32, Instruction::BranchI32XorImm16, execute_branch_i32_xor_imm16, cmp_i32_xor),
@@ -379,21 +371,37 @@ impl_execute_branch_binop_imm! {
     (u32, Instruction::BranchI32LtUImm16Rhs, execute_branch_i32_lt_u_imm16_rhs, cmp_lt),
     (i32, Instruction::BranchI32LeSImm16Rhs, execute_branch_i32_le_s_imm16_rhs, cmp_le),
     (u32, Instruction::BranchI32LeUImm16Rhs, execute_branch_i32_le_u_imm16_rhs, cmp_le),
-    (i32, Instruction::BranchI32GtSImm16Rhs, execute_branch_i32_gt_s_imm16_rhs, cmp_gt),
-    (u32, Instruction::BranchI32GtUImm16Rhs, execute_branch_i32_gt_u_imm16_rhs, cmp_gt),
-    (i32, Instruction::BranchI32GeSImm16Rhs, execute_branch_i32_ge_s_imm16_rhs, cmp_ge),
-    (u32, Instruction::BranchI32GeUImm16Rhs, execute_branch_i32_ge_u_imm16_rhs, cmp_ge),
 
     (i64, Instruction::BranchI64EqImm16, execute_branch_i64_eq_imm16, cmp_eq),
     (i64, Instruction::BranchI64NeImm16, execute_branch_i64_ne_imm16, cmp_ne),
     (i64, Instruction::BranchI64LtSImm16Rhs, execute_branch_i64_lt_s_imm16_rhs, cmp_lt),
     (u64, Instruction::BranchI64LtUImm16Rhs, execute_branch_i64_lt_u_imm16_rhs, cmp_lt),
     (i64, Instruction::BranchI64LeSImm16Rhs, execute_branch_i64_le_s_imm16_rhs, cmp_le),
     (u64, Instruction::BranchI64LeUImm16Rhs, execute_branch_i64_le_u_imm16_rhs, cmp_le),
-    (i64, Instruction::BranchI64GtSImm16Rhs, execute_branch_i64_gt_s_imm16_rhs, cmp_gt),
-    (u64, Instruction::BranchI64GtUImm16Rhs, execute_branch_i64_gt_u_imm16_rhs, cmp_gt),
-    (i64, Instruction::BranchI64GeSImm16Rhs, execute_branch_i64_ge_s_imm16_rhs, cmp_ge),
-    (u64, Instruction::BranchI64GeUImm16Rhs, execute_branch_i64_ge_u_imm16_rhs, cmp_ge),
+}
+
+macro_rules! impl_execute_branch_binop_imm16_lhs {
+    ( $( ($ty:ty, Instruction::$op_name:ident, $fn_name:ident, $op:expr) ),* $(,)? ) => {
+        impl<'engine> Executor<'engine> {
+            $(
+                #[doc = concat!("Executes an [`Instruction::", stringify!($op_name), "`].")]
+                pub fn $fn_name(&mut self, lhs: Const16<$ty>, rhs: Reg, offset: BranchOffset16) {
+                    self.execute_branch_binop_imm16_lhs::<$ty>(lhs, rhs, offset, $op)
+                }
+            )*
+        }
+    }
+}
+impl_execute_branch_binop_imm16_lhs! {
+    (i32, Instruction::BranchI32LtSImm16Lhs, execute_branch_i32_lt_s_imm16_lhs, cmp_lt),
+    (u32, Instruction::BranchI32LtUImm16Lhs, execute_branch_i32_lt_u_imm16_lhs, cmp_lt),
+    (i32, Instruction::BranchI32LeSImm16Lhs, execute_branch_i32_le_s_imm16_lhs, cmp_le),
+    (u32, Instruction::BranchI32LeUImm16Lhs, execute_branch_i32_le_u_imm16_lhs, cmp_le),
+
+    (i64, Instruction::BranchI64LtSImm16Lhs, execute_branch_i64_lt_s_imm16_lhs, cmp_lt),
+    (u64, Instruction::BranchI64LtUImm16Lhs, execute_branch_i64_lt_u_imm16_lhs, cmp_lt),
+    (i64, Instruction::BranchI64LeSImm16Lhs, execute_branch_i64_le_s_imm16_lhs, cmp_le),
+    (u64, Instruction::BranchI64LeUImm16Lhs, execute_branch_i64_le_u_imm16_lhs, cmp_le),
 }
 
 impl Executor<'_> {
@@ -412,10 +420,6 @@ impl Executor<'_> {
             C::I32LtU => self.execute_branch_binop::<u32>(lhs, rhs, offset, cmp_lt),
             C::I32LeS => self.execute_branch_binop::<i32>(lhs, rhs, offset, cmp_le),
             C::I32LeU => self.execute_branch_binop::<u32>(lhs, rhs, offset, cmp_le),
-            C::I32GtS => self.execute_branch_binop::<i32>(lhs, rhs, offset, cmp_gt),
-            C::I32GtU => self.execute_branch_binop::<u32>(lhs, rhs, offset, cmp_gt),
-            C::I32GeS => self.execute_branch_binop::<i32>(lhs, rhs, offset, cmp_ge),
-            C::I32GeU => self.execute_branch_binop::<u32>(lhs, rhs, offset, cmp_ge),
             C::I32And => self.execute_branch_binop::<i32>(lhs, rhs, offset, cmp_i32_and),
             C::I32Or => self.execute_branch_binop::<i32>(lhs, rhs, offset, cmp_i32_or),
             C::I32Xor => self.execute_branch_binop::<i32>(lhs, rhs, offset, cmp_i32_xor),
@@ -428,22 +432,14 @@ impl Executor<'_> {
             C::I64LtU => self.execute_branch_binop::<u64>(lhs, rhs, offset, cmp_lt),
             C::I64LeS => self.execute_branch_binop::<i64>(lhs, rhs, offset, cmp_le),
             C::I64LeU => self.execute_branch_binop::<u64>(lhs, rhs, offset, cmp_le),
-            C::I64GtS => self.execute_branch_binop::<i64>(lhs, rhs, offset, cmp_gt),
-            C::I64GtU => self.execute_branch_binop::<u64>(lhs, rhs, offset, cmp_gt),
-            C::I64GeS => self.execute_branch_binop::<i64>(lhs, rhs, offset, cmp_ge),
-            C::I64GeU => self.execute_branch_binop::<u64>(lhs, rhs, offset, cmp_ge),
             C::F32Eq => self.execute_branch_binop::<f32>(lhs, rhs, offset, cmp_eq),
             C::F32Ne => self.execute_branch_binop::<f32>(lhs, rhs, offset, cmp_ne),
             C::F32Lt => self.execute_branch_binop::<f32>(lhs, rhs, offset, cmp_lt),
             C::F32Le => self.execute_branch_binop::<f32>(lhs, rhs, offset, cmp_le),
-            C::F32Gt => self.execute_branch_binop::<f32>(lhs, rhs, offset, cmp_gt),
-            C::F32Ge => self.execute_branch_binop::<f32>(lhs, rhs, offset, cmp_ge),
             C::F64Eq => self.execute_branch_binop::<f64>(lhs, rhs, offset, cmp_eq),
             C::F64Ne => self.execute_branch_binop::<f64>(lhs, rhs, offset, cmp_ne),
             C::F64Lt => self.execute_branch_binop::<f64>(lhs, rhs, offset, cmp_lt),
             C::F64Le => self.execute_branch_binop::<f64>(lhs, rhs, offset, cmp_le),
-            C::F64Gt => self.execute_branch_binop::<f64>(lhs, rhs, offset, cmp_gt),
-            C::F64Ge => self.execute_branch_binop::<f64>(lhs, rhs, offset, cmp_ge),
         };
     }
 }