Remove conditional return instructions (#1486)

* comment-out or remove conditional return tests

* remove conditional return instructions

* remove conditional return instruction execution

* adjust Wasmi translation for conditional return removal

* fix bug

* remove broken doc links

* fix found fuzz bug

* update fuzz tests

* update some of the commented-out tests

* convert another translation test

* update the remaining translation tests

* convert missing translation test

Author: Robbepop

crates/ir/src/enum.rs

@@ -166,24 +166,6 @@ impl Instruction {
         Self::return_many([reg0.into(), reg1.into(), reg2.into()])
     }
 
-    /// Creates a new [`Instruction::ReturnNezReg2`] for the given `condition` and `value`.
-    pub fn return_nez_reg2_ext(
-        condition: impl Into<Reg>,
-        value0: impl Into<Reg>,
-        value1: impl Into<Reg>,
-    ) -> Self {
-        Self::return_nez_reg2(condition, [value0.into(), value1.into()])
-    }
-
-    /// Creates a new [`Instruction::ReturnNezMany`] for the given `condition` and `value`.
-    pub fn return_nez_many_ext(
-        condition: impl Into<Reg>,
-        head0: impl Into<Reg>,
-        head1: impl Into<Reg>,
-    ) -> Self {
-        Self::return_nez_many(condition, [head0.into(), head1.into()])
-    }
-
     /// Creates a new [`Instruction::Copy2`].
     pub fn copy2_ext(results: RegSpan, value0: impl Into<Reg>, value1: impl Into<Reg>) -> Self {
         let span = FixedRegSpan::new(results).unwrap_or_else(|_| {

crates/ir/src/for_each_op.rs

@@ -122,117 +122,6 @@ macro_rules! for_each_op_grouped {
                     values: [Reg; 3],
                 },
 
-                /// A conditional `return` instruction.
-                ///
-                /// # Note
-                ///
-                /// This is used to translate certain conditional Wasm branches such as `br_if`.
-                /// Returns back to the caller if and only if the `condition` value is non zero.
-                #[snake_name(return_nez)]
-                ReturnNez {
-                    /// The register holding the condition to evaluate against zero.
-                    condition: Reg,
-                },
-                /// A conditional `return` instruction.
-                ///
-                /// # Note
-                ///
-                /// Variant of [`Instruction::ReturnNez`] returning a single
-                /// [`Reg`] value if the `condition` evaluates to `true`.
-                #[snake_name(return_nez_reg)]
-                ReturnNezReg {
-                    /// The register holding the condition to evaluate against zero.
-                    condition: Reg,
-                    /// The returned value.
-                    value: Reg,
-                },
-                /// A conditional `return` instruction.
-                ///
-                /// # Note
-                ///
-                /// Variant of [`Instruction::ReturnNez`] returning two
-                /// [`Reg`] value if the `condition` evaluates to `true`.
-                #[snake_name(return_nez_reg2)]
-                ReturnNezReg2 {
-                    /// The register holding the condition to evaluate against zero.
-                    condition: Reg,
-                    /// The returned value.
-                    values: [Reg; 2],
-                },
-                /// A conditional `return` instruction.
-                ///
-                /// # Note
-                ///
-                /// Variant of [`Instruction::ReturnNez`] returning a single
-                /// [`AnyConst32`] value if the `condition` evaluates to `true`.
-                #[snake_name(return_nez_imm32)]
-                ReturnNezImm32 {
-                    /// The register holding the condition to evaluate against zero.
-                    condition: Reg,
-                    /// The returned value.
-                    value: AnyConst32,
-                },
-                /// A conditional `return` instruction.
-                ///
-                /// # Note
-                ///
-                /// Variant of [`Instruction::ReturnNez`] returning a single
-                /// 32-bit encoded [`i64`] value if the `condition` evaluates to `true`.
-                #[snake_name(return_nez_i64imm32)]
-                ReturnNezI64Imm32 {
-                    /// The register holding the condition to evaluate against zero.
-                    condition: Reg,
-                    /// The returned value.
-                    value: Const32<i64>,
-                },
-                /// A conditional `return` instruction.
-                ///
-                /// # Note
-                ///
-                /// Variant of [`Instruction::ReturnNez`] returning a single
-                /// 32-bit encoded [`f64`] value if the `condition` evaluates to `true`.
-                #[snake_name(return_nez_f64imm32)]
-                ReturnNezF64Imm32 {
-                    /// The register holding the condition to evaluate against zero.
-                    condition: Reg,
-                    /// The returned value.
-                    value: Const32<f64>,
-                },
-                /// A conditional `return` instruction.
-                ///
-                /// # Note
-                ///
-                /// Variant of [`Instruction::ReturnNez`] returning two or more values.
-                #[snake_name(return_nez_span)]
-                ReturnNezSpan {
-                    /// The register holding the condition to evaluate against zero.
-                    condition: Reg,
-                    /// The returned values.
-                    values: BoundedRegSpan,
-                },
-                /// A conditional `return` instruction.
-                ///
-                /// # Note
-                ///
-                /// Variant of [`Instruction::ReturnNez`] returning multiple register values.
-                ///
-                /// # Encoding
-                ///
-                /// Must be followed by
-                ///
-                /// 1. Zero or more [`Instruction::RegisterList`]
-                /// 2. Followed by one of
-                ///     - [`Instruction::Register`]
-                ///     - [`Instruction::Register2`]
-                ///     - [`Instruction::Register3`]
-                #[snake_name(return_nez_many)]
-                ReturnNezMany {
-                    /// The register holding the condition to evaluate against zero.
-                    condition: Reg,
-                    /// The first returned value.
-                    values: [Reg; 2],
-                },
-
                 /// A Wasm `br` instruction.
                 #[snake_name(branch)]
                 Branch {

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

@@ -153,58 +153,6 @@ impl<'engine> Executor<'engine> {
                 Instr::ReturnMany { values } => {
                     forward_return!(self.execute_return_many(store.inner_mut(), values))
                 }
-                Instr::ReturnNez { condition } => {
-                    forward_return!(self.execute_return_nez(store.inner_mut(), condition))
-                }
-                Instr::ReturnNezReg { condition, value } => {
-                    forward_return!(self.execute_return_nez_reg(
-                        store.inner_mut(),
-                        condition,
-                        value
-                    ))
-                }
-                Instr::ReturnNezReg2 { condition, values } => {
-                    forward_return!(self.execute_return_nez_reg2(
-                        store.inner_mut(),
-                        condition,
-                        values
-                    ))
-                }
-                Instr::ReturnNezImm32 { condition, value } => {
-                    forward_return!(self.execute_return_nez_imm32(
-                        store.inner_mut(),
-                        condition,
-                        value
-                    ))
-                }
-                Instr::ReturnNezI64Imm32 { condition, value } => {
-                    forward_return!(self.execute_return_nez_i64imm32(
-                        store.inner_mut(),
-                        condition,
-                        value
-                    ))
-                }
-                Instr::ReturnNezF64Imm32 { condition, value } => {
-                    forward_return!(self.execute_return_nez_f64imm32(
-                        store.inner_mut(),
-                        condition,
-                        value
-                    ))
-                }
-                Instr::ReturnNezSpan { condition, values } => {
-                    forward_return!(self.execute_return_nez_span(
-                        store.inner_mut(),
-                        condition,
-                        values
-                    ))
-                }
-                Instr::ReturnNezMany { condition, values } => {
-                    forward_return!(self.execute_return_nez_many(
-                        store.inner_mut(),
-                        condition,
-                        values
-                    ))
-                }
                 Instr::Branch { offset } => self.execute_branch(offset),
                 Instr::BranchTable0 { index, len_targets } => {
                     self.execute_branch_table_0(index, len_targets)

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

@@ -187,7 +187,6 @@ impl Executor<'_> {
     /// Used by the execution logic for
     ///
     /// - [`Instruction::ReturnMany`]
-    /// - [`Instruction::ReturnNezMany`]
     /// - [`Instruction::BranchTableMany`]
     pub fn copy_many_return_values(&mut self, ip: InstructionPtr, values: &[Reg]) {
         let (mut caller_sp, results) = self.return_caller_results();
@@ -224,110 +223,4 @@ impl Executor<'_> {
         };
         copy_results(values);
     }
-
-    /// Execute a generic conditional return [`Instruction`].
-    fn execute_return_nez_impl<T>(
-        &mut self,
-        store: &mut StoreInner,
-        condition: Reg,
-        value: T,
-        f: fn(&mut Self, &mut StoreInner, T) -> ControlFlow,
-    ) -> ControlFlow {
-        let condition = self.get_register(condition);
-        match bool::from(condition) {
-            true => f(self, store, value),
-            false => {
-                self.next_instr();
-                ControlFlow::Continue(())
-            }
-        }
-    }
-
-    /// Execute an [`Instruction::ReturnNez`].
-    pub fn execute_return_nez(&mut self, store: &mut StoreInner, condition: Reg) -> ControlFlow {
-        self.execute_return_nez_impl(store, condition, (), |this, store, _| {
-            this.execute_return(store)
-        })
-    }
-
-    /// Execute an [`Instruction::ReturnNezReg`] returning a single [`Reg`] value.
-    pub fn execute_return_nez_reg(
-        &mut self,
-        store: &mut StoreInner,
-        condition: Reg,
-        value: Reg,
-    ) -> ControlFlow {
-        self.execute_return_nez_impl(store, condition, value, Self::execute_return_reg)
-    }
-
-    /// Execute an [`Instruction::ReturnNezReg2`] returning two [`Reg`] values.
-    pub fn execute_return_nez_reg2(
-        &mut self,
-        store: &mut StoreInner,
-        condition: Reg,
-        value: [Reg; 2],
-    ) -> ControlFlow {
-        self.execute_return_nez_impl(store, condition, value, Self::execute_return_reg2)
-    }
-
-    /// Execute an [`Instruction::ReturnNezImm32`] returning a single 32-bit immediate value.
-    pub fn execute_return_nez_imm32(
-        &mut self,
-        store: &mut StoreInner,
-        condition: Reg,
-        value: AnyConst32,
-    ) -> ControlFlow {
-        self.execute_return_nez_impl(store, condition, value, Self::execute_return_imm32)
-    }
-
-    /// Execute an [`Instruction::ReturnNezI64Imm32`] returning a single 32-bit encoded immediate `i64` value.
-    pub fn execute_return_nez_i64imm32(
-        &mut self,
-        store: &mut StoreInner,
-        condition: Reg,
-        value: Const32<i64>,
-    ) -> ControlFlow {
-        self.execute_return_nez_impl(store, condition, value, Self::execute_return_i64imm32)
-    }
-
-    /// Execute an [`Instruction::ReturnNezF64Imm32`] returning a single 32-bit encoded immediate `f64` value.
-    pub fn execute_return_nez_f64imm32(
-        &mut self,
-        store: &mut StoreInner,
-        condition: Reg,
-        value: Const32<f64>,
-    ) -> ControlFlow {
-        self.execute_return_nez_impl(store, condition, value, Self::execute_return_f64imm32)
-    }
-
-    /// Execute an [`Instruction::ReturnNezSpan`] returning many values.
-    pub fn execute_return_nez_span(
-        &mut self,
-        store: &mut StoreInner,
-        condition: Reg,
-        values: BoundedRegSpan,
-    ) -> ControlFlow {
-        self.execute_return_nez_impl(store, condition, values, Self::execute_return_span)
-    }
-
-    /// Execute an [`Instruction::ReturnNezMany`] returning many values.
-    pub fn execute_return_nez_many(
-        &mut self,
-        store: &mut StoreInner,
-        condition: Reg,
-        values: [Reg; 2],
-    ) -> ControlFlow {
-        let condition = self.get_register(condition);
-        self.ip.add(1);
-        match bool::from(condition) {
-            true => {
-                self.copy_many_return_values(self.ip, &values);
-                self.return_impl(store)
-            }
-            false => {
-                self.ip = Self::skip_register_list(self.ip);
-                ControlFlow::Continue(())
-            }
-        }
-    }
 }

crates/wasmi/src/engine/translator/instr_encoder.rs

@@ -641,75 +641,13 @@ impl InstrEncoder {
         Ok(())
     }
 
-    /// Encodes an conditional `return` instruction.
-    pub fn encode_return_nez(
-        &mut self,
-        stack: &mut ValueStack,
-        condition: Reg,
-        values: &[TypedProvider],
-        fuel_info: FuelInfo,
-    ) -> Result<(), Error> {
-        // Note: We bump fuel unconditionally even if the conditional return is not taken.
-        //       This is very conservative and may lead to more fuel costs than
-        //       actually needed for the computation. We might revisit this decision
-        //       later. An alternative solution would consume fuel during execution
-        //       time only when the return is taken.
-        let instr = match values {
-            [] => Instruction::return_nez(condition),
-            [TypedProvider::Register(reg)] => Instruction::return_nez_reg(condition, *reg),
-            [TypedProvider::Const(value)] => match value.ty() {
-                ValType::I32 => Instruction::return_nez_imm32(condition, i32::from(*value)),
-                ValType::I64 => match <Const32<i64>>::try_from(i64::from(*value)).ok() {
-                    Some(value) => Instruction::return_nez_i64imm32(condition, value),
-                    None => Instruction::return_nez_reg(condition, stack.alloc_const(*value)?),
-                },
-                ValType::F32 => Instruction::return_nez_imm32(condition, f32::from(*value)),
-                ValType::F64 => match <Const32<f64>>::try_from(f64::from(*value)).ok() {
-                    Some(value) => Instruction::return_nez_f64imm32(condition, value),
-                    None => Instruction::return_nez_reg(condition, stack.alloc_const(*value)?),
-                },
-                ValType::V128 | ValType::FuncRef | ValType::ExternRef => {
-                    Instruction::return_nez_reg(condition, stack.alloc_const(*value)?)
-                }
-            },
-            [v0, v1] => {
-                let reg0 = stack.provider2reg(v0)?;
-                let reg1 = stack.provider2reg(v1)?;
-                Instruction::return_nez_reg2_ext(condition, reg0, reg1)
-            }
-            [v0, v1, rest @ ..] => {
-                debug_assert!(!rest.is_empty());
-                // Note: The fuel for return values might result in 0 charges if there aren't
-                //       enough return values to account for at least 1 fuel. Therefore we need
-                //       to also bump by `FuelCosts::base` to charge at least 1 fuel.
-                self.bump_fuel_consumption(&fuel_info, FuelCostsProvider::base)?;
-                self.bump_fuel_consumption(&fuel_info, |costs| {
-                    costs.fuel_for_copying_values(rest.len() as u64 + 3)
-                })?;
-                if let Some(span) = BoundedRegSpan::from_providers(values) {
-                    self.push_instr(Instruction::return_nez_span(condition, span))?;
-                    return Ok(());
-                }
-                let reg0 = stack.provider2reg(v0)?;
-                let reg1 = stack.provider2reg(v1)?;
-                self.push_instr(Instruction::return_nez_many_ext(condition, reg0, reg1))?;
-                self.encode_register_list(stack, rest)?;
-                return Ok(());
-            }
-        };
-        self.bump_fuel_consumption(&fuel_info, FuelCostsProvider::base)?;
-        self.push_instr(instr)?;
-        Ok(())
-    }
-
     /// Encode the given slice of [`TypedProvider`] as a list of [`Reg`].
     ///
     /// # Note
     ///
     /// This is used for the following n-ary instructions:
     ///
     /// - [`Instruction::ReturnMany`]
-    /// - [`Instruction::ReturnNezMany`]
     /// - [`Instruction::CopyMany`]
     /// - [`Instruction::CallInternal`]
     /// - [`Instruction::CallImported`]