Clean up and refactor RegSpan type and usage (#1173)

* remove unused methods

* rename RegSpanIter::iter -> iter_sized

* rename method RegSpan::iter_u16 -> iter

* replace uses of RegSpan::iter_sized with iter

* return u16 from BlockType::len_{params,results}

crates/wasmi/src/engine/block_type.rs

@@ -3,6 +3,7 @@ use crate::{
     engine::DedupFuncType,
     module::{utils::WasmiValueType, FuncTypeIdx, ModuleHeader},
     Engine,
+    FuncType,
 };
 
 /// The type of a Wasm control flow block.
@@ -63,22 +64,22 @@ impl BlockType {
     }
 
     /// Returns the number of parameters of the [`BlockType`].
-    pub fn len_params(&self, engine: &Engine) -> usize {
+    pub fn len_params(&self, engine: &Engine) -> u16 {
         match &self.inner {
             BlockTypeInner::Empty | BlockTypeInner::Returns => 0,
             BlockTypeInner::FuncType(func_type) => {
-                engine.resolve_func_type(func_type, |func_type| func_type.params().len())
+                engine.resolve_func_type(func_type, FuncType::len_params)
             }
         }
     }
 
     /// Returns the number of results of the [`BlockType`].
-    pub fn len_results(&self, engine: &Engine) -> usize {
+    pub fn len_results(&self, engine: &Engine) -> u16 {
         match &self.inner {
             BlockTypeInner::Empty => 0,
             BlockTypeInner::Returns => 1,
             BlockTypeInner::FuncType(func_type) => {
-                engine.resolve_func_type(func_type, |func_type| func_type.results().len())
+                engine.resolve_func_type(func_type, FuncType::len_results)
             }
         }
     }

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

@@ -294,8 +294,8 @@ 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_u16(len),
-            values.iter_u16(len)
+            results.iter(len),
+            values.iter(len)
         ));
         Self::CopySpan {
             results,
@@ -307,8 +307,8 @@ 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_u16(len),
-            values.iter_u16(len)
+            results.iter(len),
+            values.iter(len)
         ));
         Self::CopySpanNonOverlapping {
             results,

crates/wasmi/src/engine/bytecode/tests.rs

@@ -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_u16(len), values.iter_u16(len))
+        RegSpanIter::has_overlapping_copies(results.iter(len), values.iter(len))
     }
 
     // len == 0

crates/wasmi/src/engine/bytecode/utils.rs

@@ -74,12 +74,12 @@ impl RegSpan {
     }
 
     /// Returns a [`RegSpanIter`] yielding `len` [`Reg`].
-    pub fn iter(self, len: usize) -> RegSpanIter {
+    pub fn iter_sized(self, len: usize) -> RegSpanIter {
         RegSpanIter::new(self.0, len)
     }
 
     /// Returns a [`RegSpanIter`] yielding `len` [`Reg`].
-    pub fn iter_u16(self, len: u16) -> RegSpanIter {
+    pub fn iter(self, len: u16) -> RegSpanIter {
         RegSpanIter::new_u16(self.0, len)
     }
 
@@ -154,32 +154,6 @@ impl RegSpanIter {
         self.len() == 0
     }
 
-    /// Returns the [`Reg`] with the minimum index of the [`RegSpanIter`].
-    fn min_register(&self) -> Reg {
-        self.span().head()
-    }
-
-    /// Returns the [`Reg`] with the maximum index of the [`RegSpanIter`].
-    ///
-    /// # Note
-    ///
-    /// - Returns [`Self::min_register`] in case the [`RegSpanIter`] is empty.
-    fn max_register(&self) -> Reg {
-        self.clone()
-            .next_back()
-            .unwrap_or_else(|| self.min_register())
-    }
-
-    /// Returns `true` if the [`Reg`] is contains in the [`RegSpanIter`].
-    pub fn contains(&self, register: Reg) -> bool {
-        if self.is_empty() {
-            return false;
-        }
-        let min = self.min_register();
-        let max = self.max_register();
-        min <= register && register <= max
-    }
-
     /// Returns `true` if `copy_span results <- values` has overlapping copies.
     ///
     /// # Examples

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

@@ -509,9 +509,9 @@ impl<'engine> Executor<'engine> {
         func: &Func,
         host_func: HostFuncEntity,
     ) -> Result<(), Error> {
-        let len_params = usize::from(host_func.len_params());
-        let len_results = usize::from(host_func.len_results());
-        let max_inout = len_params.max(len_results);
+        let len_params = host_func.len_params();
+        let len_results = host_func.len_results();
+        let max_inout = usize::from(len_params.max(len_results));
         let instance = *self.stack.calls.instance_expect();
         // We have to reinstantiate the `self.sp` [`FrameRegisters`] since we just called
         // [`ValueStack::reserve`] which might invalidate all live [`FrameRegisters`].

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

@@ -74,8 +74,8 @@ impl<'engine> Executor<'engine> {
     /// Internal implementation of [`Instruction::CopySpan`] execution.
     #[inline(always)]
     pub fn execute_copy_span_impl(&mut self, results: RegSpan, values: RegSpan, len: u16) {
-        let results = results.iter_u16(len);
-        let values = values.iter_u16(len);
+        let results = results.iter(len);
+        let values = values.iter(len);
         let mut tmp = <SmallVec<[UntypedVal; 8]>>::default();
         tmp.extend(values.into_iter().map(|value| self.get_register(value)));
         for (result, value) in results.into_iter().zip(tmp) {
@@ -109,8 +109,8 @@ impl<'engine> Executor<'engine> {
         values: RegSpan,
         len: u16,
     ) {
-        let results = results.iter_u16(len);
-        let values = values.iter_u16(len);
+        let results = results.iter(len);
+        let values = values.iter(len);
         for (result, value) in results.into_iter().zip(values.into_iter()) {
             let value = self.get_register(value);
             self.set_register(result, value);
@@ -151,7 +151,7 @@ impl<'engine> Executor<'engine> {
             ),
         };
         tmp.extend(values.iter().map(|value| self.get_register(*value)));
-        for (result, value) in results.iter(tmp.len()).zip(tmp) {
+        for (result, value) in results.iter_sized(tmp.len()).zip(tmp) {
             self.set_register(result, value);
         }
         ip

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

@@ -143,7 +143,7 @@ impl<'engine> Executor<'engine> {
     ) -> ReturnOutcome {
         let (mut caller_sp, results) = self.return_caller_results();
         debug_assert!(u16::try_from(N).is_ok());
-        for (result, value) in results.iter_u16(N as u16).zip(values) {
+        for (result, value) in results.iter(N as u16).zip(values) {
             let value = self.get_register(value);
             // Safety: The `callee.results()` always refer to a span of valid
             //         registers of the `caller` that does not overlap with the
@@ -192,7 +192,7 @@ impl<'engine> Executor<'engine> {
         values: RegSpanIter,
     ) -> ReturnOutcome {
         let (mut caller_sp, results) = self.return_caller_results();
-        let results = results.iter(values.len());
+        let results = results.iter(values.len_as_u16());
         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

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

@@ -277,7 +277,7 @@ impl<'engine> EngineExecutor<'engine> {
         let mut caller_sp = unsafe { self.stack.values.stack_ptr_at(caller.base_offset()) };
         let call_params = params.call_params();
         let len_params = call_params.len();
-        for (result, param) in caller_results.iter(len_params).zip(call_params) {
+        for (result, param) in caller_results.iter_sized(len_params).zip(call_params) {
             unsafe { caller_sp.set(result, param) };
         }
         self.execute_func(store)?;

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

@@ -27,7 +27,7 @@ impl BlockHeight {
     /// Creates a new [`BlockHeight`] for the given [`ValueStack`] `height` and [`BlockType`].
     pub fn new(engine: &Engine, height: usize, block_type: BlockType) -> Result<Self, Error> {
         fn new_impl(engine: &Engine, height: usize, block_type: BlockType) -> Option<BlockHeight> {
-            let len_params = u16::try_from(block_type.len_params(engine)).ok()?;
+            let len_params = block_type.len_params(engine);
             let height = u16::try_from(height).ok()?;
             let block_height = height.checked_sub(len_params)?;
             Some(BlockHeight(block_height))

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

@@ -526,7 +526,7 @@ impl InstrEncoder {
     /// - `[ 0 <- 1, 1 <- 2, 2 <- 3 ]``: no overlap
     /// - `[ 1 <- 0, 2 <- 1 ]`: overlaps!
     pub fn has_overlapping_copy_spans(results: RegSpan, values: RegSpan, len: usize) -> bool {
-        RegSpanIter::has_overlapping_copies(results.iter(len), values.iter(len))
+        RegSpanIter::has_overlapping_copies(results.iter_sized(len), values.iter_sized(len))
     }
 
     /// Returns `true` if the `copy results <- values` instruction has overlaps.

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

@@ -825,7 +825,7 @@ impl FuncTranslator {
         });
         for copy_group in copy_groups {
             let len = copy_group.len();
-            let results = RegSpan::new(copy_group[0].preserved).iter(len);
+            let results = RegSpan::new(copy_group[0].preserved).iter_sized(len);
             let providers = &mut self.alloc.buffer.providers;
             providers.clear();
             providers.extend(
@@ -1205,14 +1205,19 @@ impl FuncTranslator {
     /// If this procedure would allocate more registers than are available.
     fn alloc_branch_params(
         &mut self,
-        len_block_params: usize,
-        len_branch_params: usize,
+        len_block_params: u16,
+        len_branch_params: u16,
     ) -> Result<RegSpan, Error> {
         let params = &mut self.alloc.buffer.providers;
         // Pop the block parameters off the stack.
-        self.alloc.stack.pop_n(len_block_params, params);
+        self.alloc
+            .stack
+            .pop_n(usize::from(len_block_params), params);
         // Peek the branch parameter registers which are going to be returned.
-        let branch_params = self.alloc.stack.peek_dynamic_n(len_branch_params)?;
+        let branch_params = self
+            .alloc
+            .stack
+            .peek_dynamic_n(usize::from(len_branch_params))?;
         // Push the block parameters onto the stack again as if nothing happened.
         self.alloc.stack.push_n(params)?;
         params.clear();

crates/wasmi/src/engine/translator/stack/mod.rs

@@ -382,7 +382,7 @@ impl ValueStack {
     /// If this procedure would allocate more registers than are available.
     pub fn push_dynamic_n(&mut self, n: usize) -> Result<RegSpan, Error> {
         let registers = self.reg_alloc.push_dynamic_n(n)?;
-        for register in registers.iter(n) {
+        for register in registers.iter_sized(n) {
             self.providers.push_dynamic(register);
         }
         Ok(registers)

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

@@ -153,10 +153,14 @@ impl<'a> VisitOperator<'a> for FuncTranslator {
         }
         // Copy `loop` parameters over to where it expects its branch parameters.
         let len_block_params = block_type.len_params(self.engine());
-        self.alloc
+        self.alloc.stack.pop_n(
+            usize::from(len_block_params),
+            &mut self.alloc.buffer.providers,
+        );
+        let branch_params = self
+            .alloc
             .stack
-            .pop_n(len_block_params, &mut self.alloc.buffer.providers);
-        let branch_params = self.alloc.stack.push_dynamic_n(len_block_params)?;
+            .push_dynamic_n(usize::from(len_block_params))?;
         // self.preserve_locals()?; // TODO: find a case where local preservation before loops is necessary
         let fuel_info = self.fuel_info();
         self.alloc.instr_encoder.encode_copies(
@@ -235,9 +239,10 @@ impl<'a> VisitOperator<'a> for FuncTranslator {
             TypedProvider::Register(condition) => {
                 // Push the `if` parameters on the `else` provider stack for
                 // later use in case we eventually visit the `else` branch.
-                self.alloc
-                    .stack
-                    .peek_n(len_block_params, &mut self.alloc.buffer.providers);
+                self.alloc.stack.peek_n(
+                    usize::from(len_block_params),
+                    &mut self.alloc.buffer.providers,
+                );
                 self.alloc
                     .control_stack
                     .push_else_providers(self.alloc.buffer.providers.iter().copied())?;

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

@@ -578,6 +578,6 @@ impl VisitInputRegisters for RegSpanIter {
         let len = self.len_as_u16();
         let mut span = self.span();
         f(span.head_mut());
-        *self = span.iter_u16(len);
+        *self = span.iter(len);
     }
 }