From ca8c5c98360f40503dcc1cf9c235b8c1505a7729 Mon Sep 17 00:00:00 2001 From: yjh Date: Mon, 31 Oct 2022 16:39:43 +0800 Subject: [PATCH 1/3] refactor: add `eval_top3` to keep same style with `eval_top`/`eval_top2` (#542) chore: add `eval_top3` to keep same style with `eval_top`/`eval_top2` --- crates/wasmi/src/engine/executor.rs | 9 ++++++--- crates/wasmi/src/engine/stack/values/vref.rs | 17 +++++------------ 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/crates/wasmi/src/engine/executor.rs b/crates/wasmi/src/engine/executor.rs index 1db4544369..51874108c8 100644 --- a/crates/wasmi/src/engine/executor.rs +++ b/crates/wasmi/src/engine/executor.rs @@ -648,10 +648,13 @@ impl<'ctx, 'engine, 'func, HostData> Executor<'ctx, 'engine, 'func, HostData> { } fn visit_select(&mut self) { - self.value_stack.pop2_eval(|e1, e2, e3| { + self.value_stack.eval_top3(|e1, e2, e3| { let condition = >::from(e3); - let result = if condition { *e1 } else { e2 }; - *e1 = result; + if condition { + e1 + } else { + e2 + } }); self.next_instr() } diff --git a/crates/wasmi/src/engine/stack/values/vref.rs b/crates/wasmi/src/engine/stack/values/vref.rs index 50614609c7..1c6970a20e 100644 --- a/crates/wasmi/src/engine/stack/values/vref.rs +++ b/crates/wasmi/src/engine/stack/values/vref.rs @@ -211,22 +211,15 @@ impl<'a> ValueStackRef<'a> { ) } - /// Evaluates `f` on the top three stack entries. - /// - /// In summary this procedure does the following: - /// - /// - Pop entry `e3`. - /// - Pop entry `e2`. - /// - Peek entry `&mut e1_ptr`. - /// - Evaluate `f(e1_ptr, e2, e3)`. + /// Evaluates the given closure `f` for the 3 top most stack values. #[inline] - pub fn pop2_eval(&mut self, f: F) + pub fn eval_top3(&mut self, f: F) where - F: FnOnce(&mut UntypedValue, UntypedValue, UntypedValue), + F: FnOnce(UntypedValue, UntypedValue, UntypedValue) -> UntypedValue, { let (e2, e3) = self.pop2(); - let e1 = self.last_mut(); - f(e1, e2, e3) + let e1 = self.last(); + *self.last_mut() = f(e1, e2, e3) } /// Evaluates the given closure `f` for the top most stack value. From 0d087e2df16bf5cbaf32495c590c1294ceecadf4 Mon Sep 17 00:00:00 2001 From: yjh Date: Mon, 31 Oct 2022 16:44:12 +0800 Subject: [PATCH 2/3] refactor: impl ExtendInto and WrapInto for Self (#541) * feat: impl ExtendInto for Self * feat: impl WrapInto for Self --- crates/core/src/value.rs | 12 ++++++++++++ crates/wasmi/src/engine/executor.rs | 27 ++++----------------------- 2 files changed, 16 insertions(+), 23 deletions(-) diff --git a/crates/core/src/value.rs b/crates/core/src/value.rs index 40f4b8a5b0..200f4ec59a 100644 --- a/crates/core/src/value.rs +++ b/crates/core/src/value.rs @@ -508,6 +508,12 @@ impl_wrap_into!(u64, f32, F32); // largest or smallest finite value representable by f32. This is a bug and will be fixed. impl_wrap_into!(f64, f32); +// Casting to self +impl_wrap_into!(i32, i32); +impl_wrap_into!(i64, i64); +impl_wrap_into!(F32, F32); +impl_wrap_into!(F64, F64); + impl WrapInto for F64 { #[inline] fn wrap_into(self) -> F32 { @@ -626,6 +632,12 @@ impl_extend_into!(i64, f64, F64); impl_extend_into!(u64, f64, F64); impl_extend_into!(f32, f64, F64); +// Casting to self +impl_extend_into!(i32, i32); +impl_extend_into!(i64, i64); +impl_extend_into!(F32, F32); +impl_extend_into!(F64, F64); + impl ExtendInto for F32 { #[inline] fn extend_into(self) -> F64 { diff --git a/crates/wasmi/src/engine/executor.rs b/crates/wasmi/src/engine/executor.rs index 51874108c8..9e5a077844 100644 --- a/crates/wasmi/src/engine/executor.rs +++ b/crates/wasmi/src/engine/executor.rs @@ -333,21 +333,10 @@ impl<'ctx, 'engine, 'func, HostData> Executor<'ctx, 'engine, 'func, HostData> { /// - `f64.load` fn execute_load(&mut self, offset: Offset) -> Result<(), TrapCode> where + T: ExtendInto + LittleEndianConvert, UntypedValue: From, - T: LittleEndianConvert, { - self.value_stack.try_eval_top(|address| { - let raw_address = u32::from(address); - let address = Self::effective_address(offset, raw_address)?; - let mut bytes = <::Bytes as Default>::default(); - self.cache - .default_memory_bytes(self.ctx.as_context_mut()) - .read(address, bytes.as_mut())?; - let value = ::from_le_bytes(bytes); - Ok(value.into()) - })?; - self.next_instr(); - Ok(()) + self.execute_load_extend::(offset) } /// Loads a value of type `U` from the default memory at the given address offset and extends it into `T`. @@ -397,17 +386,9 @@ impl<'ctx, 'engine, 'func, HostData> Executor<'ctx, 'engine, 'func, HostData> { /// - `f64.store` fn execute_store(&mut self, offset: Offset) -> Result<(), TrapCode> where - T: LittleEndianConvert + From, + T: WrapInto + LittleEndianConvert + From, { - let (address, value) = self.value_stack.pop2(); - let value = T::from(value); - let address = Self::effective_address(offset, u32::from(address))?; - let bytes = ::into_le_bytes(value); - self.cache - .default_memory_bytes(self.ctx.as_context_mut()) - .write(address, bytes.as_ref())?; - self.next_instr(); - Ok(()) + self.execute_store_wrap::(offset) } /// Stores a value of type `T` wrapped to type `U` into the default memory at the given address offset. From f5e1c46ed4bea411b4796c11ab5c2b8ad92c4739 Mon Sep 17 00:00:00 2001 From: yjh Date: Mon, 31 Oct 2022 17:37:23 +0800 Subject: [PATCH 3/3] refactor: improve call wasm (#508) * chore: improve call wasm * chore: improve call wasm * chore: improve call wasm * clippy * use `==` to check recursion_limit Co-authored-by: Robin Freyler --- crates/wasmi/src/engine/mod.rs | 2 +- crates/wasmi/src/engine/stack/frames.rs | 14 +++----------- crates/wasmi/src/engine/stack/mod.rs | 5 +++-- 3 files changed, 7 insertions(+), 14 deletions(-) diff --git a/crates/wasmi/src/engine/mod.rs b/crates/wasmi/src/engine/mod.rs index f0518aae5a..b1191294b3 100644 --- a/crates/wasmi/src/engine/mod.rs +++ b/crates/wasmi/src/engine/mod.rs @@ -350,7 +350,7 @@ impl EngineInner { CallOutcome::NestedCall(called_func) => { match called_func.as_internal(ctx.as_context()) { FuncEntityInternal::Wasm(wasm_func) => { - self.stack.call_wasm(frame, wasm_func, &self.code_map)?; + *frame = self.stack.call_wasm(frame, wasm_func, &self.code_map)?; } FuncEntityInternal::Host(host_func) => { cache.reset_default_memory_bytes(); diff --git a/crates/wasmi/src/engine/stack/frames.rs b/crates/wasmi/src/engine/stack/frames.rs index 59a948e333..9732234b01 100644 --- a/crates/wasmi/src/engine/stack/frames.rs +++ b/crates/wasmi/src/engine/stack/frames.rs @@ -3,7 +3,6 @@ use super::{err_stack_overflow, DEFAULT_MAX_RECURSION_DEPTH}; use crate::{core::TrapCode, engine::code_map::InstructionPtr, Instance}; use alloc::vec::Vec; -use core::mem::replace; /// A function frame of a function on the call stack. #[derive(Debug, Copy, Clone)] @@ -72,20 +71,13 @@ impl CallStack { FuncFrame::new(ip, instance) } - /// Pushes a Wasm function onto the [`CallStack`]. - pub(crate) fn push( - &mut self, - caller: &mut FuncFrame, - ip: InstructionPtr, - instance: Instance, - ) -> Result { + /// Pushes a Wasm caller function onto the [`CallStack`]. + pub(crate) fn push(&mut self, caller: FuncFrame) -> Result<(), TrapCode> { if self.len() == self.recursion_limit { return Err(err_stack_overflow()); } - let frame = FuncFrame::new(ip, instance); - let caller = replace(caller, frame); self.frames.push(caller); - Ok(frame) + Ok(()) } /// Pops the last [`FuncFrame`] from the [`CallStack`] if any. diff --git a/crates/wasmi/src/engine/stack/mod.rs b/crates/wasmi/src/engine/stack/mod.rs index 362b6dd921..919c0e7dec 100644 --- a/crates/wasmi/src/engine/stack/mod.rs +++ b/crates/wasmi/src/engine/stack/mod.rs @@ -137,13 +137,14 @@ impl Stack { /// Prepares the [`Stack`] for the given Wasm function call. pub(crate) fn call_wasm<'engine>( &mut self, - caller: &mut FuncFrame, + caller: &FuncFrame, wasm_func: &WasmFuncEntity, code_map: &'engine CodeMap, ) -> Result { let ip = self.call_wasm_impl(wasm_func, code_map)?; + self.frames.push(*caller)?; let instance = wasm_func.instance(); - let frame = self.frames.push(caller, ip, instance)?; + let frame = FuncFrame::new(ip, instance); Ok(frame) }