Skip to content

Commit

Permalink
replace usage of RegSpanIter with BoundedRegSpan
Browse files Browse the repository at this point in the history
  • Loading branch information
Robbepop committed Sep 17, 2024
1 parent 8032092 commit 10c0789
Show file tree
Hide file tree
Showing 25 changed files with 200 additions and 179 deletions.
21 changes: 8 additions & 13 deletions crates/wasmi/src/engine/bytecode/construct.rs
Original file line number Diff line number Diff line change
@@ -1,19 +1,20 @@
use super::{
utils::{BranchOffset16, Sign},
AnyConst32,
BoundedRegSpan,
BranchOffset,
Const16,
Const32,
Data,
Elem,
EngineFunc,
FixedRegSpan,
Func,
FuncType,
Global,
Instruction,
Reg,
RegSpan,
RegSpanIter,
Table,
};
use crate::core::TrapCode;
Expand Down Expand Up @@ -89,7 +90,7 @@ impl Instruction {
}

/// Creates a new [`Instruction::ReturnSpan`] from the given `values`.
pub fn return_span(values: RegSpanIter) -> Self {
pub fn return_span(values: BoundedRegSpan) -> Self {
Self::ReturnSpan { values }
}

Expand Down Expand Up @@ -152,7 +153,7 @@ impl Instruction {
}

/// Creates a new [`Instruction::ReturnNezMany`] for the given `condition` and `values`.
pub fn return_nez_span(condition: Reg, values: RegSpanIter) -> Self {
pub fn return_nez_span(condition: Reg, values: BoundedRegSpan) -> Self {
Self::ReturnNezSpan { condition, values }
}

Expand Down Expand Up @@ -262,7 +263,7 @@ impl Instruction {
/// Creates a new [`Instruction::Copy2`].
pub fn copy2(results: RegSpan, value0: impl Into<Reg>, value1: impl Into<Reg>) -> Self {
Self::Copy2 {
results,
results: <FixedRegSpan<2>>::new(results).unwrap(),
values: [value0.into(), value1.into()],
}
}
Expand Down Expand Up @@ -293,10 +294,7 @@ impl Instruction {

/// Creates a new [`Instruction::CopySpan`] copying multiple consecutive values.
pub fn copy_span(results: RegSpan, values: RegSpan, len: u16) -> Self {
debug_assert!(RegSpanIter::has_overlapping_copies(
results.iter(len),
values.iter(len)
));
debug_assert!(RegSpan::has_overlapping_copies(results, values, len));

Check warning on line 297 in crates/wasmi/src/engine/bytecode/construct.rs

View check run for this annotation

Codecov / codecov/patch

crates/wasmi/src/engine/bytecode/construct.rs#L297

Added line #L297 was not covered by tests
Self::CopySpan {
results,
values,
Expand All @@ -306,10 +304,7 @@ impl Instruction {

/// Creates a new [`Instruction::CopySpanNonOverlapping`] copying multiple consecutive values.
pub fn copy_span_non_overlapping(results: RegSpan, values: RegSpan, len: u16) -> Self {
debug_assert!(!RegSpanIter::has_overlapping_copies(
results.iter(len),
values.iter(len)
));
debug_assert!(!RegSpan::has_overlapping_copies(results, values, len));
Self::CopySpanNonOverlapping {
results,
values,
Expand Down Expand Up @@ -1025,7 +1020,7 @@ impl Instruction {
}

/// Creates a new [`Instruction::RegisterSpan`].
pub fn register_span(span: RegSpanIter) -> Self {
pub fn register_span(span: BoundedRegSpan) -> Self {
Self::RegisterSpan { span }
}

Expand Down
16 changes: 8 additions & 8 deletions crates/wasmi/src/engine/bytecode/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ pub mod index {
pub use self::{
error::Error as IrError,
immediate::{AnyConst32, Const16, Const32},
span::{BoundedRegSpan, FixedRegSpan, RegSpan, RegSpanIter},
span::{BoundedRegSpan, FixedRegSpan, RegSpan},
utils::{
BlockFuel,
BranchOffset,
Expand Down Expand Up @@ -142,10 +142,10 @@ pub enum Instruction {
///
/// # Note
///
/// Returns values as stored in the [`RegSpanIter`].
/// Returns values as stored in the [`BoundedRegSpan`].
ReturnSpan {
/// The underlying [`RegSpanIter`] value.
values: RegSpanIter,
/// The underlying [`BoundedRegSpan`] value.
values: BoundedRegSpan,
},
/// A Wasm `return` instruction.
///
Expand Down Expand Up @@ -246,7 +246,7 @@ pub enum Instruction {
/// The register holding the condition to evaluate against zero.
condition: Reg,
/// The returned values.
values: RegSpanIter,
values: BoundedRegSpan,
},
/// A conditional `return` instruction.
///
Expand Down Expand Up @@ -1118,7 +1118,7 @@ pub enum Instruction {
/// This is a Wasmi utility instruction used to translate Wasm control flow.
Copy2 {
/// The registers holding the result of the instruction.
results: RegSpan,
results: FixedRegSpan<2>,
/// The registers holding the values to copy.
values: [Reg; 2],
},
Expand Down Expand Up @@ -5446,8 +5446,8 @@ pub enum Instruction {
/// The 32-bit immediate value.
imm: AnyConst32,
},
/// A [`RegSpanIter`] instruction parameter.
RegisterSpan { span: RegSpanIter },
/// A [`BoundedRegSpan`] instruction parameter.
RegisterSpan { span: BoundedRegSpan },
/// A [`Reg`] instruction parameter.
///
/// # Note
Expand Down
12 changes: 12 additions & 0 deletions crates/wasmi/src/engine/bytecode/span.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,18 @@ impl RegSpan {
pub fn head_mut(&mut self) -> &mut Reg {
&mut self.0

Check warning on line 43 in crates/wasmi/src/engine/bytecode/span.rs

View check run for this annotation

Codecov / codecov/patch

crates/wasmi/src/engine/bytecode/span.rs#L43

Added line #L43 was not covered by tests
}

/// Returns `true` if `copy_span results <- values` has overlapping copies.
///
/// # Examples
///
/// - `[ ]`: empty never overlaps
/// - `[ 1 <- 0 ]`: single element never overlaps
/// - `[ 0 <- 1, 1 <- 2, 2 <- 3 ]`: no overlap
/// - `[ 1 <- 0, 2 <- 1 ]`: overlaps!
pub fn has_overlapping_copies(results: Self, values: Self, len: u16) -> bool {
RegSpanIter::has_overlapping_copies(results.iter(len), values.iter(len))
}
}

/// A [`RegSpan`] with a statically known number of [`Reg`].
Expand Down
2 changes: 1 addition & 1 deletion crates/wasmi/src/engine/bytecode/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ fn has_overlapping_copy_spans_works() {
}

fn has_overlapping_copy_spans(results: RegSpan, values: RegSpan, len: u16) -> bool {
RegSpanIter::has_overlapping_copies(results.iter(len), values.iter(len))
RegSpan::has_overlapping_copies(results, values, len)
}

// len == 0
Expand Down
25 changes: 14 additions & 11 deletions crates/wasmi/src/engine/bytecode/visit_regs.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use super::{BoundedRegSpan, FixedRegSpan, Instruction, Reg, RegSpan, RegSpanIter};
use super::{BoundedRegSpan, FixedRegSpan, Instruction, Reg, RegSpan};

impl Instruction {
/// Visit [`Reg`]s of `self` via the `visitor`.
Expand Down Expand Up @@ -58,16 +58,6 @@ impl<const N: u16> HostVisitor for &'_ mut FixedRegSpan<N> {
}
}

impl HostVisitor for &'_ mut RegSpanIter {
fn host_visitor<V: VisitRegs>(self, _visitor: &mut V) {
// let len = self.len_as_u16();
// let mut span = self.span();
// visitor.visit_input_regs(&mut span, Some(len));
// *self = span.iter(len);
todo!()
}
}

/// Type-wrapper to signal that the wrapped [`Reg`], [`RegSpan`] (etc.) is a result.
pub struct Res<T>(T);

Expand All @@ -83,6 +73,19 @@ impl HostVisitor for Res<&'_ mut RegSpan> {
}
}

impl HostVisitor for Res<&'_ mut BoundedRegSpan> {
fn host_visitor<V: VisitRegs>(self, visitor: &mut V) {
let len = self.0.len();
visitor.visit_result_regs(self.0.span_mut(), Some(len));

Check warning on line 79 in crates/wasmi/src/engine/bytecode/visit_regs.rs

View check run for this annotation

Codecov / codecov/patch

crates/wasmi/src/engine/bytecode/visit_regs.rs#L77-L79

Added lines #L77 - L79 were not covered by tests
}
}

impl<const N: u16> HostVisitor for Res<&'_ mut FixedRegSpan<N>> {
fn host_visitor<V: VisitRegs>(self, visitor: &mut V) {
visitor.visit_result_regs(self.0.span_mut(), Some(N));
}
}

macro_rules! host_visitor {
( $visitor:expr => $($r:expr),* $(,)? ) => {{
$( HostVisitor::host_visitor($r, $visitor) );*
Expand Down
2 changes: 1 addition & 1 deletion crates/wasmi/src/engine/executor/instrs/branch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ impl<'engine> Executor<'engine> {
let Instruction::RegisterSpan { span: values } = *self.ip.get() else {
unreachable!()
};
let len = values.len_as_u16();
let len = values.len();

Check warning on line 124 in crates/wasmi/src/engine/executor/instrs/branch.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L124 was not covered by tests
let values = values.span();
self.ip.add(offset);
match *self.ip.get() {
Expand Down
8 changes: 4 additions & 4 deletions crates/wasmi/src/engine/executor/instrs/copy.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use super::{Executor, InstructionPtr};
use crate::{
core::UntypedVal,
engine::bytecode::{AnyConst32, Const32, Instruction, Reg, RegSpan},
engine::bytecode::{AnyConst32, Const32, FixedRegSpan, Instruction, Reg, RegSpan},
};
use core::slice;
use smallvec::SmallVec;
Expand All @@ -23,15 +23,15 @@ impl<'engine> Executor<'engine> {

/// Executes an [`Instruction::Copy2`].
#[inline(always)]
pub fn execute_copy_2(&mut self, results: RegSpan, values: [Reg; 2]) {
pub fn execute_copy_2(&mut self, results: FixedRegSpan<2>, values: [Reg; 2]) {

Check warning on line 26 in crates/wasmi/src/engine/executor/instrs/copy.rs

View check run for this annotation

Codecov / codecov/patch

crates/wasmi/src/engine/executor/instrs/copy.rs#L26

Added line #L26 was not covered by tests
self.execute_copy_2_impl(results, values);
self.next_instr()
}

/// Internal implementation of [`Instruction::Copy2`] execution.
#[inline(always)]
fn execute_copy_2_impl(&mut self, results: RegSpan, values: [Reg; 2]) {
let result0 = results.head();
fn execute_copy_2_impl(&mut self, results: FixedRegSpan<2>, values: [Reg; 2]) {

Check warning on line 33 in crates/wasmi/src/engine/executor/instrs/copy.rs

View check run for this annotation

Codecov / codecov/patch

crates/wasmi/src/engine/executor/instrs/copy.rs#L33

Added line #L33 was not covered by tests
let result0 = results.span().head();
let result1 = result0.next();
// We need `tmp` in case `results[0] == values[1]` to avoid overwriting `values[1]` before reading it.
let tmp = self.get_register(values[1]);
Expand Down
8 changes: 4 additions & 4 deletions crates/wasmi/src/engine/executor/instrs/return_.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use super::{Executor, InstructionPtr};
use crate::{
core::UntypedVal,
engine::{
bytecode::{AnyConst32, Const32, Instruction, Reg, RegSpan, RegSpanIter},
bytecode::{AnyConst32, BoundedRegSpan, Const32, Instruction, Reg, RegSpan},
executor::stack::FrameRegisters,
},
store::StoreInner,
Expand Down Expand Up @@ -189,10 +189,10 @@ impl<'engine> Executor<'engine> {
pub fn execute_return_span(
&mut self,
store: &mut StoreInner,
values: RegSpanIter,
values: BoundedRegSpan,
) -> ReturnOutcome {
let (mut caller_sp, results) = self.return_caller_results();
let results = results.iter(values.len_as_u16());
let results = results.iter(values.len());
for (result, value) in results.zip(values) {
// Safety: The `callee.results()` always refer to a span of valid
// registers of the `caller` that does not overlap with the
Expand Down Expand Up @@ -342,7 +342,7 @@ impl<'engine> Executor<'engine> {
&mut self,
store: &mut StoreInner,
condition: Reg,
values: RegSpanIter,
values: BoundedRegSpan,
) -> ReturnOutcome {
self.execute_return_nez_impl(store, condition, values, Self::execute_return_span)
}
Expand Down
19 changes: 8 additions & 11 deletions crates/wasmi/src/engine/translator/control_frame.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use super::LabelRef;
use super::ValueStack;
use crate::{
engine::{
bytecode::{RegSpan, RegSpanIter},
bytecode::{BoundedRegSpan, RegSpan},
BlockType,
Instr,
TranslationError,
Expand Down Expand Up @@ -106,9 +106,8 @@ impl BlockControlFrame {
}

/// Returns an iterator over the registers holding the branching parameters of the [`BlockControlFrame`].
pub fn branch_params(&self, engine: &Engine) -> RegSpanIter {
self.branch_params
.iter(self.block_type().len_results(engine))
pub fn branch_params(&self, engine: &Engine) -> BoundedRegSpan {
BoundedRegSpan::new(self.branch_params, self.block_type().len_results(engine))
}

/// Returns the label for the branch destination of the [`BlockControlFrame`].
Expand Down Expand Up @@ -211,9 +210,8 @@ impl LoopControlFrame {
}

/// Returns an iterator over the registers holding the branching parameters of the [`LoopControlFrame`].
pub fn branch_params(&self, engine: &Engine) -> RegSpanIter {
self.branch_params
.iter(self.block_type().len_params(engine))
pub fn branch_params(&self, engine: &Engine) -> BoundedRegSpan {
BoundedRegSpan::new(self.branch_params, self.block_type().len_params(engine))
}

/// Returns the label for the branch destination of the [`LoopControlFrame`].
Expand Down Expand Up @@ -381,9 +379,8 @@ impl IfControlFrame {
}

/// Returns an iterator over the registers holding the branching parameters of the [`IfControlFrame`].
pub fn branch_params(&self, engine: &Engine) -> RegSpanIter {
self.branch_params
.iter(self.block_type().len_results(engine))
pub fn branch_params(&self, engine: &Engine) -> BoundedRegSpan {
BoundedRegSpan::new(self.branch_params, self.block_type().len_results(engine))
}

/// Returns the label for the branch destination of the [`IfControlFrame`].
Expand Down Expand Up @@ -602,7 +599,7 @@ impl ControlFrame {
}

/// Returns an iterator over the registers holding the branch parameters of the [`ControlFrame`].
pub fn branch_params(&self, engine: &Engine) -> RegSpanIter {
pub fn branch_params(&self, engine: &Engine) -> BoundedRegSpan {
match self {
Self::Block(frame) => frame.branch_params(engine),
Self::Loop(frame) => frame.branch_params(engine),
Expand Down
Loading

0 comments on commit 10c0789

Please sign in to comment.