remove superseeded branch+cmp instructions

wasmi-labs · Nov 25, 2023 · 2b9ff82 · 2b9ff82
1 parent a5432b0
commit 2b9ff82
Show file tree

Hide file tree

Showing 12 changed files with 72 additions and 171 deletions.
diff --git a/crates/wasmi/src/engine/regmach/bytecode/construct.rs b/crates/wasmi/src/engine/regmach/bytecode/construct.rs
@@ -224,8 +224,8 @@ macro_rules! constructor_for_branch_binop_imm {
         impl Instruction {
             $(
                 #[doc = concat!("Creates a new [`Instruction::", stringify!($op_code), "`].")]
-                pub fn $name(lhs: Register, rhs: Const16<$ty>, offset: BranchOffset16) -> Self {
-                    Self::$op_code(BranchBinOpInstrImm16::new(lhs, rhs, offset))
+                pub fn $name(lhs: Register, rhs: impl Into<Const16<$ty>>, offset: BranchOffset16) -> Self {
+                    Self::$op_code(BranchBinOpInstrImm16::new(lhs, rhs.into(), offset))
                 }
             )*
         }
@@ -412,24 +412,24 @@ impl Instruction {
         Self::Branch { offset }
     }
 
-    /// Creates a new [`Instruction::BranchI32Eqz`] for the given `condition` and `offset`.
-    pub fn branch_i32_eqz(condition: Register, offset: BranchOffset) -> Self {
-        Self::BranchI32Eqz { condition, offset }
+    /// Convenience constructor to create a new [`Instruction::BranchI32Eqz`] with a zero immediate value.
+    pub fn branch_i32_eqz(condition: Register, offset: BranchOffset16) -> Self {
+        Self::branch_i32_eq_imm(condition, 0_i16, offset)
     }
 
-    /// Creates a new [`Instruction::BranchI32Nez`] for the given `condition` and `offset`.
-    pub fn branch_i32_nez(condition: Register, offset: BranchOffset) -> Self {
-        Self::BranchI32Nez { condition, offset }
+    /// Convenience constructor to create a new [`Instruction::BranchI32Nez`] with a zero immediate value.
+    pub fn branch_i32_nez(condition: Register, offset: BranchOffset16) -> Self {
+        Self::branch_i32_ne_imm(condition, 0_i16, offset)
     }
 
-    /// Creates a new [`Instruction::BranchI64Eqz`] for the given `condition` and `offset`.
-    pub fn branch_i64_eqz(condition: Register, offset: BranchOffset) -> Self {
-        Self::BranchI64Eqz { condition, offset }
+    /// Convenience constructor to create a new [`Instruction::BranchI64Eqz`] with a zero immediate value.
+    pub fn branch_i64_eqz(condition: Register, offset: BranchOffset16) -> Self {
+        Self::branch_i64_eq_imm(condition, 0_i16, offset)
     }
 
-    /// Creates a new [`Instruction::BranchI64Nez`] for the given `condition` and `offset`.
-    pub fn branch_i64_nez(condition: Register, offset: BranchOffset) -> Self {
-        Self::BranchI64Nez { condition, offset }
+    /// Convenience constructor to create a new [`Instruction::BranchI64Nez`] with a zero immediate value.
+    pub fn branch_i64_nez(condition: Register, offset: BranchOffset16) -> Self {
+        Self::branch_i64_ne_imm(condition, 0_i16, offset)
     }
 
     /// Creates a new [`Instruction::BranchTable`] for the given `index` and `len_targets`.

diff --git a/crates/wasmi/src/engine/regmach/bytecode/mod.rs b/crates/wasmi/src/engine/regmach/bytecode/mod.rs
@@ -367,51 +367,6 @@ pub enum Instruction {
         /// The branching offset for the instruction pointer.
         offset: BranchOffset,
     },
-    /// A conditional branch instruction.
-    ///
-    /// # Note
-    ///
-    /// - The branch is taken if `condition` evaluates to zero.
-    /// - Partially translated from negated Wasm `br_if` instructions.
-    BranchI32Eqz {
-        /// The register holding the condition to evaluate against zero.
-        condition: Register,
-        /// The branching offset for the instruction pointer.
-        offset: BranchOffset,
-    },
-    /// A Wasm `br_if` instruction.
-    ///
-    /// # Note
-    ///
-    /// The branch is taken if `condition` evaluates to zero.
-    BranchI32Nez {
-        /// The register holding the condition to evaluate against zero.
-        condition: Register,
-        /// The branching offset for the instruction pointer.
-        offset: BranchOffset,
-    },
-    /// A fused Wasm `i64.eqz` + `if` instruction.
-    ///
-    /// # Note
-    ///
-    /// - The branch is taken if `condition` evaluates to zero.
-    BranchI64Eqz {
-        /// The register holding the condition to evaluate against zero.
-        condition: Register,
-        /// The branching offset for the instruction pointer.
-        offset: BranchOffset,
-    },
-    /// A fused Wasm `i64.eqz` + `br_if` instruction.
-    ///
-    /// # Note
-    ///
-    /// The branch is taken if `condition` evaluates to zero.
-    BranchI64Nez {
-        /// The register holding the condition to evaluate against zero.
-        condition: Register,
-        /// The branching offset for the instruction pointer.
-        offset: BranchOffset,
-    },
 
     /// A fused [`Instruction::I32And`] and [`Instruction::BranchI32Nez`] instruction.
     BranchI32And(BranchBinOpInstr),

diff --git a/crates/wasmi/src/engine/regmach/bytecode/utils.rs b/crates/wasmi/src/engine/regmach/bytecode/utils.rs
@@ -496,7 +496,9 @@ impl TryFrom<BranchOffset> for BranchOffset16 {
 
     fn try_from(offset: BranchOffset) -> Result<Self, Self::Error> {
         let Ok(offset16) = i16::try_from(offset.to_i32()) else {
-            return Err(TranslationError::new(TranslationErrorInner::BranchOffsetOutOfBounds))
+            return Err(TranslationError::new(
+                TranslationErrorInner::BranchOffsetOutOfBounds,
+            ));
         };
         Ok(Self(offset16))
     }

diff --git a/crates/wasmi/src/engine/regmach/executor/instrs.rs b/crates/wasmi/src/engine/regmach/executor/instrs.rs
@@ -245,18 +245,6 @@ impl<'ctx, 'engine> Executor<'ctx, 'engine> {
                     forward_return!(self.execute_return_nez_many(condition, values))
                 }
                 Instr::Branch { offset } => self.execute_branch(offset),
-                Instr::BranchI32Eqz { condition, offset } => {
-                    self.execute_branch_i32_eqz(condition, offset)
-                }
-                Instr::BranchI32Nez { condition, offset } => {
-                    self.execute_branch_i32_nez(condition, offset)
-                }
-                Instr::BranchI64Eqz { condition, offset } => {
-                    self.execute_branch_i64_eqz(condition, offset)
-                }
-                Instr::BranchI64Nez { condition, offset } => {
-                    self.execute_branch_i64_nez(condition, offset)
-                }
                 Instr::BranchTable { index, len_targets } => {
                     self.execute_branch_table(index, len_targets)
                 }

diff --git a/crates/wasmi/src/engine/regmach/executor/instrs/branch.rs b/crates/wasmi/src/engine/regmach/executor/instrs/branch.rs
@@ -15,42 +15,6 @@ impl<'ctx, 'engine> Executor<'ctx, 'engine> {
         self.branch_to(offset)
     }
 
-    #[inline(always)]
-    pub fn execute_branch_i32_nez(&mut self, condition: Register, offset: BranchOffset) {
-        let value: i32 = self.get_register_as(condition);
-        if value != 0 {
-            return self.branch_to(offset);
-        }
-        self.next_instr();
-    }
-
-    #[inline(always)]
-    pub fn execute_branch_i32_eqz(&mut self, condition: Register, offset: BranchOffset) {
-        let value: i32 = self.get_register_as(condition);
-        if value == 0 {
-            return self.branch_to(offset);
-        }
-        self.next_instr();
-    }
-
-    #[inline(always)]
-    pub fn execute_branch_i64_nez(&mut self, condition: Register, offset: BranchOffset) {
-        let value: i64 = self.get_register_as(condition);
-        if value != 0 {
-            return self.branch_to(offset);
-        }
-        self.next_instr();
-    }
-
-    #[inline(always)]
-    pub fn execute_branch_i64_eqz(&mut self, condition: Register, offset: BranchOffset) {
-        let value: i64 = self.get_register_as(condition);
-        if value == 0 {
-            return self.branch_to(offset);
-        }
-        self.next_instr();
-    }
-
     #[inline(always)]
     pub fn execute_branch_table(&mut self, index: Register, len_targets: Const32<u32>) {
         let index: u32 = self.get_register_as(index);

diff --git a/crates/wasmi/src/engine/regmach/tests/op/block.rs b/crates/wasmi/src/engine/regmach/tests/op/block.rs
@@ -2,7 +2,7 @@ use super::*;
 use crate::engine::{
     bytecode::BranchOffset,
     regmach::{
-        bytecode::RegisterSpan,
+        bytecode::{BranchOffset16, RegisterSpan},
         tests::{display_wasm::DisplayValueType, wasm_type::WasmType},
     },
 };
@@ -346,7 +346,7 @@ fn branch_if_block_0() {
     );
     TranslationTest::new(wasm)
         .expect_func_instrs([
-            Instruction::branch_i32_nez(Register::from_i16(0), BranchOffset::from(1)),
+            Instruction::branch_i32_nez(Register::from_i16(0), BranchOffset16::from(1)),
             Instruction::Return,
         ])
         .run()
@@ -369,7 +369,7 @@ fn branch_if_block_1() {
     );
     TranslationTest::new(wasm)
         .expect_func_instrs([
-            Instruction::branch_i32_eqz(Register::from_i16(1), BranchOffset::from(3)),
+            Instruction::branch_i32_eqz(Register::from_i16(1), BranchOffset16::from(3)),
             Instruction::copy(Register::from_i16(2), Register::from_i16(0)),
             Instruction::branch(BranchOffset::from(2)),
             Instruction::copy(Register::from_i16(2), Register::from_i16(0)),

diff --git a/crates/wasmi/src/engine/regmach/tests/op/br_if.rs b/crates/wasmi/src/engine/regmach/tests/op/br_if.rs
@@ -2,7 +2,7 @@ use super::*;
 use crate::engine::{
     bytecode::BranchOffset,
     regmach::{
-        bytecode::RegisterSpan,
+        bytecode::{BranchOffset16, RegisterSpan},
         tests::{display_wasm::DisplayValueType, driver::ExpectedFunc, wasm_type::WasmType},
     },
 };
@@ -844,7 +844,7 @@ fn branch_if_results_0() {
     );
     TranslationTest::new(wasm)
         .expect_func_instrs([
-            Instruction::branch_i32_nez(Register::from_i16(0), BranchOffset::from(1)),
+            Instruction::branch_i32_nez(Register::from_i16(0), BranchOffset16::from(1)),
             Instruction::Return,
         ])
         .run()
@@ -867,7 +867,7 @@ fn branch_if_results_1() {
     );
     TranslationTest::new(wasm)
         .expect_func_instrs([
-            Instruction::branch_i32_eqz(Register::from_i16(1), BranchOffset::from(3)),
+            Instruction::branch_i32_eqz(Register::from_i16(1), BranchOffset16::from(3)),
             Instruction::copy(Register::from_i16(2), Register::from_i16(0)),
             Instruction::branch(BranchOffset::from(2)),
             Instruction::copy(Register::from_i16(2), Register::from_i16(0)),
@@ -902,7 +902,7 @@ fn branch_if_results_1_avoid_copy() {
         .expect_func_instrs([
             Instruction::i32_clz(Register::from_i16(2), Register::from_i16(0)),
             Instruction::i32_ctz(Register::from_i16(3), Register::from_i16(1)),
-            Instruction::branch_i32_nez(Register::from_i16(3), BranchOffset::from(1)),
+            Instruction::branch_i32_nez(Register::from_i16(3), BranchOffset16::from(1)),
             Instruction::return_reg(Register::from_i16(2)),
         ])
         .run()
@@ -927,7 +927,7 @@ fn branch_if_results_2() {
     );
     TranslationTest::new(wasm)
         .expect_func_instrs([
-            Instruction::branch_i32_eqz(Register::from_i16(2), BranchOffset::from(3)),
+            Instruction::branch_i32_eqz(Register::from_i16(2), BranchOffset16::from(3)),
             Instruction::copy2(RegisterSpan::new(Register::from_i16(3)), 0, 1),
             Instruction::branch(BranchOffset::from(2)),
             Instruction::copy2(RegisterSpan::new(Register::from_i16(3)), 0, 1),
@@ -968,7 +968,7 @@ fn branch_if_results_2_avoid_copy() {
         .expect_func_instrs([
             Instruction::i32_clz(Register::from_i16(3), Register::from_i16(0)),
             Instruction::i32_ctz(Register::from_i16(4), Register::from_i16(1)),
-            Instruction::branch_i32_nez(Register::from_i16(2), BranchOffset::from(1)),
+            Instruction::branch_i32_nez(Register::from_i16(2), BranchOffset16::from(1)),
             Instruction::i32_add(
                 Register::from_i16(3),
                 Register::from_i16(3),
@@ -1002,7 +1002,7 @@ fn branch_if_results_4_mixed_1() {
     TranslationTest::new(wasm)
         .expect_func(
             ExpectedFunc::new([
-                Instruction::branch_i32_eqz(Register::from_i16(2), BranchOffset::from(4)),
+                Instruction::branch_i32_eqz(Register::from_i16(2), BranchOffset16::from(4)),
                 Instruction::copy_many_non_overlapping(
                     RegisterSpan::new(Register::from_i16(3)),
                     -1,
@@ -1045,7 +1045,7 @@ fn branch_if_results_4_mixed_2() {
     );
     TranslationTest::new(wasm)
         .expect_func_instrs([
-            Instruction::branch_i32_eqz(Register::from_i16(2), BranchOffset::from(4)),
+            Instruction::branch_i32_eqz(Register::from_i16(2), BranchOffset16::from(4)),
             Instruction::copy_many_non_overlapping(RegisterSpan::new(Register::from_i16(3)), 0, 0),
             Instruction::register2(1, 1),
             Instruction::branch(BranchOffset::from(3)),

diff --git a/crates/wasmi/src/engine/regmach/tests/op/cmp_br.rs b/crates/wasmi/src/engine/regmach/tests/op/cmp_br.rs
@@ -150,7 +150,7 @@ fn loop_backward_imm() {
 #[test]
 #[cfg_attr(miri, ignore)]
 fn loop_backward_imm_eqz() {
-    fn test_for(op: &str, expect_instr: fn(Register, BranchOffset) -> Instruction) {
+    fn test_for(op: &str, expect_instr: fn(Register, BranchOffset16) -> Instruction) {
         let wasm = wat2wasm(&format!(
             r"
             (module
@@ -166,7 +166,7 @@ fn loop_backward_imm_eqz() {
         ));
         TranslationTest::new(wasm)
             .expect_func_instrs([
-                expect_instr(Register::from_i16(0), BranchOffset::from(0_i32)),
+                expect_instr(Register::from_i16(0), BranchOffset16::from(0_i16)),
                 Instruction::Return,
             ])
             .run()
@@ -539,7 +539,7 @@ fn block_i64_eqz_fuse() {
     );
     TranslationTest::new(wasm)
         .expect_func_instrs([
-            Instruction::branch_i64_eqz(Register::from_i16(0), BranchOffset::from(1)),
+            Instruction::branch_i64_eqz(Register::from_i16(0), BranchOffset16::from(1)),
             Instruction::Return,
         ])
         .run()
@@ -561,7 +561,7 @@ fn if_i64_eqz_fuse() {
     );
     TranslationTest::new(wasm)
         .expect_func_instrs([
-            Instruction::branch_i64_nez(Register::from_i16(0), BranchOffset::from(1)),
+            Instruction::branch_i64_nez(Register::from_i16(0), BranchOffset16::from(1)),
             Instruction::Return,
         ])
         .run()