From 4cf45ad6fbc477cbfdbfb2ad09e3cbf1f196c926 Mon Sep 17 00:00:00 2001 From: Robin Freyler Date: Fri, 29 Nov 2024 13:55:45 +0100 Subject: [PATCH] `wasmi_ir`: remove `InstrSequence` type (#1323) * wasmi_ir: remove InstrSequence * remove wasmi_ir::Instr type --- crates/ir/src/index.rs | 24 -- crates/ir/src/lib.rs | 3 - crates/ir/src/primitive.rs | 8 +- crates/ir/src/sequence.rs | 229 ------------------ .../src/engine/translator/instr_encoder.rs | 13 - crates/wasmi/src/engine/translator/labels.rs | 7 +- 6 files changed, 9 insertions(+), 275 deletions(-) delete mode 100644 crates/ir/src/sequence.rs diff --git a/crates/ir/src/index.rs b/crates/ir/src/index.rs index 98e97eaf17..c1c034636e 100644 --- a/crates/ir/src/index.rs +++ b/crates/ir/src/index.rs @@ -5,11 +5,6 @@ use crate::Error; macro_rules! for_each_index { ($mac:ident) => { $mac! { - /// Used to query the [`Instruction`] of an [`InstrSequence`]. - /// - /// [`Instruction`]: crate::Instruction - /// [`InstrSequence`]: crate::InstrSequence - Instr(pub(crate) u32); /// A Wasmi register. Reg(pub(crate) i16); /// A Wasm function index. @@ -113,22 +108,3 @@ impl Reg { self.0.is_negative() } } - -impl Instr { - /// Creates an [`Instr`] from the given `usize` value. - /// - /// # Panics - /// - /// If the `value` exceeds limitations for [`Instr`]. - pub fn from_usize(index: usize) -> Self { - let index = index.try_into().unwrap_or_else(|error| { - panic!("invalid index {index} for instruction reference: {error}") - }); - Self(index) - } - - /// Returns the index underlying to `self` as `usize`. - pub fn into_usize(self) -> usize { - self.0 as usize - } -} diff --git a/crates/ir/src/lib.rs b/crates/ir/src/lib.rs index 23679f1c83..d1b2185d30 100644 --- a/crates/ir/src/lib.rs +++ b/crates/ir/src/lib.rs @@ -10,7 +10,6 @@ mod for_each_op; mod immeditate; pub mod index; mod primitive; -mod sequence; mod span; mod visit_regs; @@ -23,7 +22,6 @@ use wasmi_core as core; pub use self::{ error::Error, immeditate::{AnyConst16, AnyConst32, Const16, Const32}, - index::Instr, index::Reg, primitive::{ BlockFuel, @@ -36,7 +34,6 @@ pub use self::{ Sign, }, r#enum::Instruction, - sequence::{InstrIter, InstrIterMut, InstrSequence}, span::{BoundedRegSpan, FixedRegSpan, RegSpan, RegSpanIter}, visit_regs::VisitRegs, }; diff --git a/crates/ir/src/primitive.rs b/crates/ir/src/primitive.rs index 08e8ddefa5..0c5bb24252 100644 --- a/crates/ir/src/primitive.rs +++ b/crates/ir/src/primitive.rs @@ -1,4 +1,4 @@ -use crate::{core::UntypedVal, Const16, Error, Instr}; +use crate::{core::UntypedVal, Const16, Error}; use core::marker::PhantomData; /// The sign of a value. @@ -152,9 +152,9 @@ impl BranchOffset { /// # Errors /// /// If the resulting [`BranchOffset`] is out of bounds. - pub fn from_src_to_dst(src: Instr, dst: Instr) -> Result { - let src = i64::from(u32::from(src)); - let dst = i64::from(u32::from(dst)); + pub fn from_src_to_dst(src: u32, dst: u32) -> Result { + let src = i64::from(src); + let dst = i64::from(dst); let Some(offset) = dst.checked_sub(src) else { // Note: This never needs to be called on backwards branches since they are immediated resolved. unreachable!( diff --git a/crates/ir/src/sequence.rs b/crates/ir/src/sequence.rs deleted file mode 100644 index 258d28d367..0000000000 --- a/crates/ir/src/sequence.rs +++ /dev/null @@ -1,229 +0,0 @@ -use crate::{core::TrapCode, index::*, *}; -use ::core::{ - num::{NonZeroI32, NonZeroI64, NonZeroU32, NonZeroU64}, - slice, -}; -use alloc::{boxed::Box, vec::Vec}; - -/// A sequence of [`Instruction`]s. -#[derive(Debug, Default, Clone, PartialEq, Eq)] -pub struct InstrSequence { - /// The [`Instruction`] that make up all built instructions in sequence. - instrs: Vec, -} - -impl From for Vec { - fn from(sequence: InstrSequence) -> Self { - sequence.into_vec() - } -} - -impl From for Box<[Instruction]> { - fn from(sequence: InstrSequence) -> Self { - sequence.into_boxed_slice() - } -} - -impl InstrSequence { - /// Returns `self` as vector of [`Instruction`]s. - pub fn into_vec(self) -> Vec { - self.instrs - } - - /// Returns `self` as boxed slice of [`Instruction`]s. - pub fn into_boxed_slice(self) -> Box<[Instruction]> { - self.instrs.into_boxed_slice() - } - - /// Clears all [`Instruction`]s from `self` emptying `self` in the process. - /// - /// # Note - /// - /// This invalidates all [`Instr`] references to `self`. - pub fn clear(&mut self) { - self.instrs.clear() - } - - /// Returns the underlying [`Instruction`]s as shared slice. - pub fn as_slice(&self) -> &[Instruction] { - &self.instrs[..] - } - - /// Returns the underlying [`Instruction`]s as mutable slice. - pub fn as_slice_mut(&mut self) -> &mut [Instruction] { - &mut self.instrs[..] - } - - /// Returns the number of [`Instruction`] in `self`. - #[inline] - pub fn len(&self) -> usize { - self.instrs.len() - } - - /// Returns `true` if `self` is empty. - #[inline] - pub fn is_empty(&self) -> bool { - self.len() == 0 - } - - /// Returns the [`Instruction`] that is associated to `instr`. - #[inline] - pub fn get(&self, instr: Instr) -> Option { - self.instrs.get(instr.into_usize()).copied() - } - - /// Returns a mutable reference to the [`Instruction`] that is associated to `instr`. - #[inline] - pub fn get_mut(&mut self, instr: Instr) -> Option<&mut Instruction> { - self.instrs.get_mut(instr.into_usize()) - } - - /// Returns an iterator yielding the [`Instruction`] of the [`InstrSequence`]. - pub fn iter(&self) -> InstrIter { - InstrIter::new(self) - } - - /// Returns an iterator yielding mutable [`Instruction`] of the [`InstrSequence`]. - pub fn iter_mut(&mut self) -> InstrIterMut { - InstrIterMut::new(self) - } - - /// Pops the last [`Instruction`] in `self` if any. - /// - /// Returns `None` if `self` is empty. - pub fn pop(&mut self) -> Option { - self.instrs.pop() - } -} - -impl<'a> IntoIterator for &'a InstrSequence { - type Item = &'a Instruction; - type IntoIter = InstrIter<'a>; - - #[inline] - fn into_iter(self) -> Self::IntoIter { - self.iter() - } -} - -impl<'a> IntoIterator for &'a mut InstrSequence { - type Item = &'a mut Instruction; - type IntoIter = InstrIterMut<'a>; - - #[inline] - fn into_iter(self) -> Self::IntoIter { - self.iter_mut() - } -} - -macro_rules! define_builder { - ( - $( - $( #[doc = $doc:literal] )* - #[snake_name($snake_name:ident)] - $name:ident - $( - { - // $( @result )? - // $( @results )? - $( - $( #[$field_docs:meta] )* $(@)? - $field_name:ident: $field_ty:ty - ),* - $(,)? - } - )? - ),* $(,)? - ) => { - impl InstrSequence { - $( - #[doc = concat!("Pushes an [`Instruction::", stringify!($name), "`].")] - /// - /// Returns the [`Instr`] to query the pushed [`Instruction`]. - pub fn $snake_name( - &mut self, - $( $( $field_name: impl Into<$field_ty> ),* )? - ) -> Instr { - let pos = Instr::from_usize(self.instrs.len()); - self.instrs.push(Instruction::$name { - $( $( $field_name: $field_name.into() ),* )? - }); - pos - } - )* - } - }; -} -for_each_op!(define_builder); - -/// Iterator yielding the [`Instruction`]s of an [`InstrSequence`]. -#[derive(Debug)] -pub struct InstrIter<'a> { - instrs: slice::Iter<'a, Instruction>, -} - -impl<'a> InstrIter<'a> { - /// Creates a new [`InstrIter`] for the [`InstrSequence`]. - fn new(builder: &'a InstrSequence) -> Self { - Self { - instrs: builder.instrs.iter(), - } - } -} - -impl<'a> Iterator for InstrIter<'a> { - type Item = &'a Instruction; - - #[inline] - fn next(&mut self) -> Option { - self.instrs.next() - } - - fn size_hint(&self) -> (usize, Option) { - self.instrs.size_hint() - } -} - -impl DoubleEndedIterator for InstrIter<'_> { - fn next_back(&mut self) -> Option { - self.instrs.next_back() - } -} - -impl ExactSizeIterator for InstrIter<'_> {} - -/// Iterator yielding the [`Instruction`]s of an [`InstrSequence`] mutably. -#[derive(Debug)] -pub struct InstrIterMut<'a> { - instrs: slice::IterMut<'a, Instruction>, -} - -impl<'a> InstrIterMut<'a> { - /// Creates a new [`InstrIter`] for the [`InstrSequence`]. - fn new(builder: &'a mut InstrSequence) -> Self { - Self { - instrs: builder.instrs.iter_mut(), - } - } -} - -impl<'a> Iterator for InstrIterMut<'a> { - type Item = &'a mut Instruction; - - #[inline] - fn next(&mut self) -> Option { - self.instrs.next() - } - - fn size_hint(&self) -> (usize, Option) { - self.instrs.size_hint() - } -} - -impl DoubleEndedIterator for InstrIterMut<'_> { - fn next_back(&mut self) -> Option { - self.instrs.next_back() - } -} - -impl ExactSizeIterator for InstrIterMut<'_> {} diff --git a/crates/wasmi/src/engine/translator/instr_encoder.rs b/crates/wasmi/src/engine/translator/instr_encoder.rs index b1f69e4d66..621eeda08c 100644 --- a/crates/wasmi/src/engine/translator/instr_encoder.rs +++ b/crates/wasmi/src/engine/translator/instr_encoder.rs @@ -19,7 +19,6 @@ use crate::{ FuelCosts, }, ir::{ - self, BoundedRegSpan, BranchOffset, BranchOffset16, @@ -41,18 +40,6 @@ use core::mem; #[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)] pub struct Instr(u32); -impl From for Instr { - fn from(instr: ir::Instr) -> Self { - Self(u32::from(instr)) - } -} - -impl From for ir::Instr { - fn from(instr: Instr) -> Self { - Self::from(instr.0) - } -} - impl Instr { /// Creates an [`Instr`] from the given `usize` value. /// diff --git a/crates/wasmi/src/engine/translator/labels.rs b/crates/wasmi/src/engine/translator/labels.rs index b3079c58c2..eed64a5f7a 100644 --- a/crates/wasmi/src/engine/translator/labels.rs +++ b/crates/wasmi/src/engine/translator/labels.rs @@ -148,7 +148,9 @@ impl LabelRegistry { user: Instr, ) -> Result { let offset = match *self.get_label(label) { - Label::Pinned(target) => BranchOffset::from_src_to_dst(user.into(), target.into())?, + Label::Pinned(target) => { + BranchOffset::from_src_to_dst(user.into_u32(), target.into_u32())? + } Label::Unpinned => { self.users.push(LabelUser::new(label, user)); BranchOffset::uninit() @@ -205,7 +207,8 @@ impl Iterator for ResolvedUserIter<'_> { .registry .resolve_label(next.label) .unwrap_or_else(|err| panic!("failed to resolve user: {err}")); - let offset = BranchOffset::from_src_to_dst(src.into(), dst.into()).map_err(Into::into); + let offset = + BranchOffset::from_src_to_dst(src.into_u32(), dst.into_u32()).map_err(Into::into); Some((src, offset)) } }