Skip to content

Commit

Permalink
Remove superseeded conditional branch instructions (#805)
Browse files Browse the repository at this point in the history
* add TryFrom<BranchOffset> for BranchOffset16

* move From impl

* remove superseeded branch+cmp instructions

* fix intra doc links

* refactor implementation of cmp+br instructions

* apply rustfmt

* reduce column noise
  • Loading branch information
Robbepop authored Nov 25, 2023
1 parent 13dd961 commit 834c3fb
Show file tree
Hide file tree
Showing 12 changed files with 246 additions and 333 deletions.
28 changes: 14 additions & 14 deletions crates/wasmi/src/engine/regmach/bytecode/construct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
)*
}
Expand Down Expand Up @@ -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::BranchI32EqImm`] 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::BranchI32NeImm`] 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::BranchI64EqImm`] 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::BranchI64NeImm`] 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`.
Expand Down
173 changes: 64 additions & 109 deletions crates/wasmi/src/engine/regmach/bytecode/mod.rs

Large diffs are not rendered by default.

25 changes: 19 additions & 6 deletions crates/wasmi/src/engine/regmach/bytecode/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,25 @@ impl From<i16> for BranchOffset16 {
}
}

impl TryFrom<BranchOffset> for BranchOffset16 {
type Error = TranslationError;

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,
));
};
Ok(Self(offset16))
}
}

impl From<BranchOffset16> for BranchOffset {
fn from(offset: BranchOffset16) -> Self {
Self::from(i32::from(offset.to_i16()))
}
}

impl BranchOffset16 {
/// Creates a 16-bit [`BranchOffset16`] from a 32-bit [`BranchOffset`] if possible.
pub fn new(offset: BranchOffset) -> Option<Self> {
Expand Down Expand Up @@ -527,12 +546,6 @@ impl BranchOffset16 {
}
}

impl From<BranchOffset16> for BranchOffset {
fn from(offset: BranchOffset16) -> Self {
Self::from(i32::from(offset.to_i16()))
}
}

/// A generic fused comparison and conditional branch [`Instruction`].
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
pub struct BranchBinOpInstr {
Expand Down
12 changes: 0 additions & 12 deletions crates/wasmi/src/engine/regmach/executor/instrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
Loading

0 comments on commit 834c3fb

Please sign in to comment.