Use Stack in Executor (#1071)

* use Stack in Executor

This replaces the ValueStack and CallStack references with a single Stack reference.

* swap values and calls fields in Stack

This has severe performance implications on the Executor. Unfortunately in Rust 1.78 this regresses performance overall and in Rust 1.79 this improves performance all in all.

* apply clippy suggestion

* Stack: swap field reset order

* refactor Stack::merge_call_frames

* use EngineResources reference in Executor

* fix doc link

* apply rustfmt

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

@@ -1,6 +1,9 @@
 pub use self::call::{dispatch_host_func, ResumableHostError};
 use self::return_::ReturnOutcome;
-use super::cache::{CachedGlobal, CachedMemory};
+use super::{
+    cache::{CachedGlobal, CachedMemory},
+    Stack,
+};
 use crate::{
     core::{TrapCode, UntypedVal},
     engine::{
@@ -21,8 +24,7 @@ use crate::{
         },
         code_map::InstructionPtr,
         executor::stack::{CallFrame, CallStack, FrameRegisters, ValueStack},
-        func_types::FuncTypeRegistry,
-        CodeMap,
+        EngineResources,
     },
     memory::DataSegment,
     module::DEFAULT_MEMORY_INDEX,
@@ -73,12 +75,10 @@ macro_rules! forward_return {
 #[inline(never)]
 pub fn execute_instrs<'engine, T>(
     store: &mut Store<T>,
-    value_stack: &'engine mut ValueStack,
-    call_stack: &'engine mut CallStack,
-    code_map: &'engine CodeMap,
-    func_types: &'engine FuncTypeRegistry,
+    stack: &'engine mut Stack,
+    res: &'engine EngineResources,
 ) -> Result<(), Error> {
-    Executor::new(value_stack, call_stack, code_map, func_types).execute(store)
+    Executor::new(stack, res).execute(store)
 }
 
 /// An execution context for executing a Wasmi function frame.
@@ -92,59 +92,34 @@ struct Executor<'engine> {
     memory: CachedMemory,
     /// The cached global variable at index 0.
     global: CachedGlobal,
-    /// The value stack.
-    ///
-    /// # Note
-    ///
-    /// This reference is mainly used to synchronize back state
-    /// after manipulations to the value stack via `sp`.
-    value_stack: &'engine mut ValueStack,
-    /// The call stack.
-    ///
-    /// # Note
-    ///
-    /// This is used to store the stack of nested function calls.
-    call_stack: &'engine mut CallStack,
-    /// The Wasm function code map.
-    ///
-    /// # Note
-    ///
-    /// This is used to lookup Wasm function information.
-    code_map: &'engine CodeMap,
-    /// The Wasm function type registry.
-    ///
-    /// # Note
+    /// The value and call stacks.
+    stack: &'engine mut Stack,
+    /// The static resources of an [`Engine`].
     ///
-    /// This is used to lookup Wasm function information.
-    func_types: &'engine FuncTypeRegistry,
+    /// [`Engine`]: crate::Engine
+    res: &'engine EngineResources,
 }
 
 impl<'engine> Executor<'engine> {
     /// Creates a new [`Executor`] for executing a Wasmi function frame.
     #[inline(always)]
-    pub fn new(
-        value_stack: &'engine mut ValueStack,
-        call_stack: &'engine mut CallStack,
-        code_map: &'engine CodeMap,
-        func_types: &'engine FuncTypeRegistry,
-    ) -> Self {
-        let frame = call_stack
+    pub fn new(stack: &'engine mut Stack, res: &'engine EngineResources) -> Self {
+        let frame = stack
+            .calls
             .peek()
             .expect("must have call frame on the call stack");
         // Safety: We are using the frame's own base offset as input because it is
         //         guaranteed by the Wasm validation and translation phase to be
         //         valid for all register indices used by the associated function body.
-        let sp = unsafe { value_stack.stack_ptr_at(frame.base_offset()) };
+        let sp = unsafe { stack.values.stack_ptr_at(frame.base_offset()) };
         let ip = frame.instr_ptr();
         Self {
             sp,
             ip,
             memory: CachedMemory::default(),
             global: CachedGlobal::default(),
-            value_stack,
-            call_stack,
-            code_map,
-            func_types,
+            stack,
+            res,
         }
     }
 
@@ -160,7 +135,7 @@ impl<'engine> Executor<'engine> {
     #[inline(always)]
     fn execute<T>(mut self, store: &mut Store<T>) -> Result<(), Error> {
         use Instruction as Instr;
-        let instance = Self::instance(self.call_stack);
+        let instance = Self::instance(&self.stack.calls);
         self.memory.update(&mut store.inner, instance);
         self.global.update(&mut store.inner, instance);
         loop {
@@ -914,7 +889,7 @@ macro_rules! get_entity {
             )]
             #[inline]
             fn $name(&self, store: &StoreInner, index: $index_ty) -> $id_ty {
-                let instance = Self::instance(self.call_stack);
+                let instance = Self::instance(&self.stack.calls);
                 let index = ::core::primitive::u32::from(index);
                 store
                     .resolve_instance(instance)
@@ -1037,7 +1012,7 @@ impl<'engine> Executor<'engine> {
     ///
     /// The initialization of the [`Executor`] allows for efficient execution.
     fn init_call_frame(&mut self, frame: &CallFrame) {
-        Self::init_call_frame_impl(self.value_stack, &mut self.sp, &mut self.ip, frame)
+        Self::init_call_frame_impl(&mut self.stack.values, &mut self.sp, &mut self.ip, frame)
     }
 
     /// Initializes the [`Executor`] state for the [`CallFrame`].

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

@@ -4,7 +4,7 @@ use crate::{
     engine::{
         bytecode::{FuncIdx, Instruction, Register, RegisterSpan, SignatureIdx, TableIdx},
         code_map::InstructionPtr,
-        executor::stack::{CallFrame, FrameParams, Stack, ValueStack},
+        executor::stack::{CallFrame, FrameParams, ValueStack},
         func_types::FuncTypeRegistry,
         CompiledFunc,
         CompiledFuncEntity,
@@ -172,7 +172,8 @@ impl<'engine> Executor<'engine> {
         // other parts of the code more fragile with respect to instruction ordering.
         self.ip.add(offset);
         let caller = self
-            .call_stack
+            .stack
+            .calls
             .peek_mut()
             .expect("caller call frame must be on the stack");
         caller.update_instr_ptr(self.ip);
@@ -218,10 +219,11 @@ impl<'engine> Executor<'engine> {
         // We have to reinstantiate the `self.sp` [`FrameRegisters`] since we just called
         // [`ValueStack::alloc_call_frame`] which might invalidate all live [`FrameRegisters`].
         let caller = self
-            .call_stack
+            .stack
+            .calls
             .peek()
             .expect("need to have a caller on the call stack");
-        let (mut uninit_params, offsets) = self.value_stack.alloc_call_frame(func, |this| {
+        let (mut uninit_params, offsets) = self.stack.values.alloc_call_frame(func, |this| {
             // Safety: We use the base offset of a live call frame on the call stack.
             self.sp = unsafe { this.stack_ptr_at(caller.base_offset()) };
         })?;
@@ -298,7 +300,7 @@ impl<'engine> Executor<'engine> {
         func: CompiledFunc,
         mut instance: Option<Instance>,
     ) -> Result<(), Error> {
-        let func = self.code_map.get(Some(store.fuel_mut()), func)?;
+        let func = self.res.code_map.get(Some(store.fuel_mut()), func)?;
         let mut called = self.dispatch_compiled_func::<C>(results, func)?;
         match <C as CallContext>::KIND {
             CallKind::Nested => {
@@ -316,16 +318,14 @@ impl<'engine> Executor<'engine> {
                 // on the value stack which is what the function expects. After this operation we ensure
                 // that `self.sp` is adjusted via a call to `init_call_frame` since it may have been
                 // invalidated by this method.
-                let caller_instance = unsafe {
-                    Stack::merge_call_frames(self.call_stack, self.value_stack, &mut called)
-                };
+                let caller_instance = unsafe { self.stack.merge_call_frames(&mut called) };
                 if let Some(caller_instance) = caller_instance {
                     instance.get_or_insert(caller_instance);
                 }
             }
         }
         self.init_call_frame(&called);
-        self.call_stack.push(called, instance)?;
+        self.stack.calls.push(called, instance)?;
         Ok(())
     }
 
@@ -368,7 +368,8 @@ impl<'engine> Executor<'engine> {
     ///
     /// [`CallStack`]: crate::engine::executor::stack::CallStack
     fn caller_results(&self) -> RegisterSpan {
-        self.call_stack
+        self.stack
+            .calls
             .peek()
             .expect("must have caller on the stack")
             .results()
@@ -496,20 +497,18 @@ impl<'engine> Executor<'engine> {
         func: &Func,
         host_func: HostFuncEntity,
     ) -> Result<(), Error> {
-        let (len_params, len_results) = self.func_types.len_params_results(host_func.ty_dedup());
+        let (len_params, len_results) =
+            self.res.func_types.len_params_results(host_func.ty_dedup());
         let max_inout = len_params.max(len_results);
-        let instance = *self
-            .call_stack
-            .instance()
-            .expect("need to have an instance on the call stack");
+        let instance = *Self::instance(&self.stack.calls);
         // We have to reinstantiate the `self.sp` [`FrameRegisters`] since we just called
         // [`ValueStack::reserve`] which might invalidate all live [`FrameRegisters`].
         let caller = match <C as CallContext>::KIND {
-            CallKind::Nested => self.call_stack.peek().copied(),
-            CallKind::Tail => self.call_stack.pop().map(|(frame, _instance)| frame),
+            CallKind::Nested => self.stack.calls.peek().copied(),
+            CallKind::Tail => self.stack.calls.pop().map(|(frame, _instance)| frame),
         }
         .expect("need to have a caller on the call stack");
-        let buffer = self.value_stack.extend_by(max_inout, |this| {
+        let buffer = self.stack.values.extend_by(max_inout, |this| {
             // Safety: we use the base offset of a live call frame on the call stack.
             self.sp = unsafe { this.stack_ptr_at(caller.base_offset()) };
         })?;
@@ -521,14 +520,14 @@ impl<'engine> Executor<'engine> {
             self.update_instr_ptr_at(1);
         }
         self.dispatch_host_func::<T>(store, host_func, &instance)
-            .map_err(|error| match self.call_stack.is_empty() {
+            .map_err(|error| match self.stack.calls.is_empty() {
                 true => error,
                 false => ResumableHostError::new(error, *func, results).into(),
             })?;
         self.memory.update(&mut store.inner, &instance);
         self.global.update(&mut store.inner, &instance);
         let results = results.iter(len_results);
-        let returned = self.value_stack.drop_return(max_inout);
+        let returned = self.stack.values.drop_return(max_inout);
         for (result, value) in results.zip(returned) {
             // # Safety (1)
             //
@@ -553,8 +552,8 @@ impl<'engine> Executor<'engine> {
     ) -> Result<(usize, usize), Error> {
         dispatch_host_func(
             store,
-            self.func_types,
-            self.value_stack,
+            &self.res.func_types,
+            &mut self.stack.values,
             host_func,
             Some(instance),
         )
@@ -636,7 +635,7 @@ impl<'engine> Executor<'engine> {
         let actual_signature = store.inner.resolve_func(func).ty_dedup();
         let expected_signature = store
             .inner
-            .resolve_instance(Self::instance(self.call_stack))
+            .resolve_instance(Self::instance(&self.stack.calls))
             .get_signature(u32::from(func_type))
             .unwrap_or_else(|| {
                 panic!("missing signature for call_indirect at index: {func_type:?}")

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

@@ -97,7 +97,7 @@ impl<'engine> Executor<'engine> {
                 // The `memory.grow` operation might have invalidated the cached
                 // linear memory so we need to reset it in order for the cache to
                 // reload in case it is used again.
-                let instance = Self::instance(self.call_stack);
+                let instance = Self::instance(&self.stack.calls);
                 self.memory.update(store, instance);
                 return_value
             }

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

@@ -26,18 +26,24 @@ impl<'engine> Executor<'engine> {
     #[inline(always)]
     fn return_impl(&mut self, store: &mut StoreInner) -> ReturnOutcome {
         let (returned, popped_instance) = self
-            .call_stack
+            .stack
+            .calls
             .pop()
             .expect("the executing call frame is always on the stack");
-        self.value_stack.truncate(returned.frame_offset());
-        let new_instance = popped_instance.and_then(|_| self.call_stack.instance());
+        self.stack.values.truncate(returned.frame_offset());
+        let new_instance = popped_instance.and_then(|_| self.stack.calls.instance());
         if let Some(new_instance) = new_instance {
             self.global.update(store, new_instance);
             self.memory.update(store, new_instance);
         }
-        match self.call_stack.peek() {
+        match self.stack.calls.peek() {
             Some(caller) => {
-                Self::init_call_frame_impl(self.value_stack, &mut self.sp, &mut self.ip, caller);
+                Self::init_call_frame_impl(
+                    &mut self.stack.values,
+                    &mut self.sp,
+                    &mut self.ip,
+                    caller,
+                );
                 ReturnOutcome::Wasm
             }
             None => ReturnOutcome::Host,
@@ -56,7 +62,8 @@ impl<'engine> Executor<'engine> {
     #[inline(always)]
     fn return_caller_results(&mut self) -> (FrameRegisters, RegisterSpan) {
         let (callee, caller) = self
-            .call_stack
+            .stack
+            .calls
             .peek_2()
             .expect("the callee must exist on the call stack");
         match caller {
@@ -68,7 +75,7 @@ impl<'engine> Executor<'engine> {
                 //
                 // Safety: The caller call frame is still live on the value stack
                 //         and therefore it is safe to acquire its value stack pointer.
-                let caller_sp = unsafe { self.value_stack.stack_ptr_at(caller.base_offset()) };
+                let caller_sp = unsafe { self.stack.values.stack_ptr_at(caller.base_offset()) };
                 let results = callee.results();
                 (caller_sp, results)
             }
@@ -77,7 +84,7 @@ impl<'engine> Executor<'engine> {
                 //
                 // In this case we transfer the single return `value` to the root
                 // register span of the entire value stack which is simply its zero index.
-                let dst_sp = self.value_stack.root_stack_ptr();
+                let dst_sp = self.stack.values.root_stack_ptr();
                 let results = RegisterSpan::new(Register::from_i16(0));
                 (dst_sp, results)
             }

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

@@ -416,7 +416,7 @@ impl<'engine> Executor<'engine> {
         let table_index = self.fetch_table_index(1);
         let element_index = self.fetch_element_segment_index(2);
         let (instance, table, element, fuel) = store.resolve_table_init_params(
-            Self::instance(self.call_stack),
+            Self::instance(&self.stack.calls),
             &self.get_table(store, table_index),
             &self.get_element_segment(store, element_index),
         );

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

@@ -293,11 +293,7 @@ impl<'engine> EngineExecutor<'engine> {
     /// When encountering a Wasm or host trap during execution.
     #[inline(always)]
     fn execute_func<T>(&mut self, store: &mut Store<T>) -> Result<(), Error> {
-        let value_stack = &mut self.stack.values;
-        let call_stack = &mut self.stack.calls;
-        let code_map = &self.res.code_map;
-        let func_types = &self.res.func_types;
-        execute_instrs(store, value_stack, call_stack, code_map, func_types)
+        execute_instrs(store, self.stack, self.res)
     }
 
     /// Convenience forwarder to [`dispatch_host_func`].