From f88147e2645323f21829efb698d4a1664f322221 Mon Sep 17 00:00:00 2001 From: Robin Freyler Date: Thu, 20 Oct 2022 15:32:28 +0200 Subject: [PATCH 1/2] simplify and optimize wasmi codegen --- .../engine/func_builder/locals_registry.rs | 203 +----------------- crates/wasmi/src/engine/func_builder/mod.rs | 164 +++++--------- .../src/engine/func_builder/value_stack.rs | 130 +++++------ 3 files changed, 114 insertions(+), 383 deletions(-) diff --git a/crates/wasmi/src/engine/func_builder/locals_registry.rs b/crates/wasmi/src/engine/func_builder/locals_registry.rs index 15a86be5da..0549ff7fb1 100644 --- a/crates/wasmi/src/engine/func_builder/locals_registry.rs +++ b/crates/wasmi/src/engine/func_builder/locals_registry.rs @@ -1,7 +1,3 @@ -use alloc::vec::Vec; -use core::cmp::Ordering; -use wasmi_core::ValueType; - /// A registry where local variables of a function are registered and resolved. /// /// # Note @@ -23,57 +19,11 @@ use wasmi_core::ValueType; /// exploitation impact. #[derive(Debug, Default)] pub struct LocalsRegistry { - /// An efficient store for the registered local variable groups. - groups: Vec, - /// Max local index. - max_index: u32, -} - -/// A group of local values as encoded in the Wasm binary. -#[derive(Debug, Copy, Clone)] -pub struct LocalGroup { - /// The (included) minimum local index of the local variable group. - min_index: u32, - /// The (excluded) maximum local index of the local variable group. - max_index: u32, - /// The shared [`ValueType`] of the local variables in the group. - value_type: ValueType, -} - -impl LocalGroup { - /// Creates a new [`LocalGroup`] with the given `amount` of local of shared [`ValueType`]. - pub fn new(value_type: ValueType, min_index: u32, max_index: u32) -> Self { - assert!(min_index < max_index); - Self { - min_index, - max_index, - value_type, - } - } - - /// Returns the shared [`ValueType`] of the local group. - pub fn value_type(&self) -> ValueType { - self.value_type - } - - /// Returns the minimum viable local index for the local group. - pub fn min_index(&self) -> u32 { - self.min_index - } - - /// Returns the maximum viable local index for the local group. - pub fn max_index(&self) -> u32 { - self.max_index - } + /// The amount of registered local variables. + len_registered: u32, } impl LocalsRegistry { - /// Resets the [`LocalsRegistry`] to allow for reuse. - pub fn reset(&mut self) { - self.groups.clear(); - self.max_index = 0; - } - /// Returns the number of registered local variables. /// /// # Note @@ -82,7 +32,7 @@ impl LocalsRegistry { /// this function actually returns the amount of function parameters /// and explicitly defined local variables. pub fn len_registered(&self) -> u32 { - self.max_index + self.len_registered } /// Registers the `amount` of locals with their shared [`ValueType`]. @@ -90,154 +40,15 @@ impl LocalsRegistry { /// # Panics /// /// If too many local variables have been registered. - pub fn register_locals(&mut self, value_type: ValueType, amount: u32) { + pub fn register_locals(&mut self, amount: u32) { if amount == 0 { return; } - let min_index = self.max_index; - let max_index = self.max_index.checked_add(amount).unwrap_or_else(|| { + self.len_registered = self.len_registered.checked_add(amount).unwrap_or_else(|| { panic!( - "encountered local variable index overflow \ - upon registering {} locals of type {:?}", - amount, value_type + "tried to register too many local variables for function: got {}, additional {}", + self.len_registered, amount ) }); - self.groups - .push(LocalGroup::new(value_type, min_index, max_index)); - self.max_index = max_index; - } - - /// Resolves the local variable at the given index. - pub fn resolve_local(&mut self, local_index: u32) -> Option { - if local_index >= self.max_index { - // Bail out early if the local index is invalid. - return None; - } - // Search for the local variable type in the groups - // array using efficient binary search, insert it into the - // `used` cache and return it to the caller. - match self.groups.binary_search_by(|group| { - if local_index < group.min_index() { - return Ordering::Greater; - } - if local_index >= group.max_index() { - return Ordering::Less; - } - Ordering::Equal - }) { - Ok(found_index) => { - let value_type = self.groups[found_index].value_type(); - Some(value_type) - } - Err(_) => unreachable!( - "unexpectedly could not find valid local group index \ - using `local_index` = {}", - local_index - ), - } - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - #[should_panic] - fn register_too_many() { - let mut registry = LocalsRegistry::default(); - registry.register_locals(ValueType::I32, u32::MAX); - registry.register_locals(ValueType::I32, 1); - } - - #[test] - fn empty_works() { - let mut registry = LocalsRegistry::default(); - for local_index in 0..10 { - assert!(registry.resolve_local(local_index).is_none()); - } - assert_eq!(registry.len_registered(), 0); - } - - #[test] - fn single_works() { - let mut registry = LocalsRegistry::default(); - registry.register_locals(ValueType::I32, 1); - registry.register_locals(ValueType::I64, 1); - registry.register_locals(ValueType::F32, 1); - registry.register_locals(ValueType::F64, 1); - // Duplicate value types with another set of local groups. - registry.register_locals(ValueType::I32, 1); - registry.register_locals(ValueType::I64, 1); - registry.register_locals(ValueType::F32, 1); - registry.register_locals(ValueType::F64, 1); - fn assert_valid_accesses(registry: &mut LocalsRegistry, offset: u32) { - assert_eq!(registry.resolve_local(offset + 0), Some(ValueType::I32)); - assert_eq!(registry.resolve_local(offset + 1), Some(ValueType::I64)); - assert_eq!(registry.resolve_local(offset + 2), Some(ValueType::F32)); - assert_eq!(registry.resolve_local(offset + 3), Some(ValueType::F64)); - } - // Assert the value types of the first group. - assert_valid_accesses(&mut registry, 0); - // Assert that also the second bunch of local groups work properly. - assert_valid_accesses(&mut registry, 4); - // Repeat the process to also check if the cache works. - assert_valid_accesses(&mut registry, 0); - assert_valid_accesses(&mut registry, 4); - // Assert that an index out of bounds yields `None`. - assert!(registry.resolve_local(registry.len_registered()).is_none()); - } - - #[test] - fn multiple_works() { - let mut registry = LocalsRegistry::default(); - let amount = 10; - registry.register_locals(ValueType::I32, amount); - registry.register_locals(ValueType::I64, amount); - registry.register_locals(ValueType::F32, amount); - registry.register_locals(ValueType::F64, amount); - // Duplicate value types with another set of local groups. - registry.register_locals(ValueType::I32, amount); - registry.register_locals(ValueType::I64, amount); - registry.register_locals(ValueType::F32, amount); - registry.register_locals(ValueType::F64, amount); - fn assert_local_group( - registry: &mut LocalsRegistry, - offset: u32, - amount: u32, - value_type: ValueType, - ) { - for local_index in 0..amount { - assert_eq!( - registry.resolve_local(offset + local_index), - Some(value_type) - ); - } - } - fn assert_valid_accesses(registry: &mut LocalsRegistry, offset: u32, amount: u32) { - let value_types = [ - ValueType::I32, - ValueType::I64, - ValueType::F32, - ValueType::F64, - ]; - for i in 0..4 { - assert_local_group( - registry, - offset + i * amount, - amount, - value_types[i as usize], - ); - } - } - // Assert the value types of the first group. - assert_valid_accesses(&mut registry, 0, amount); - // Assert that also the second bunch of local groups work properly. - assert_valid_accesses(&mut registry, 4 * amount, amount); - // Repeat the process to also check if the cache works. - assert_valid_accesses(&mut registry, 0, amount); - assert_valid_accesses(&mut registry, 4 * amount, amount); - // Assert that an index out of bounds yields `None`. - assert!(registry.resolve_local(registry.len_registered()).is_none()); } } diff --git a/crates/wasmi/src/engine/func_builder/mod.rs b/crates/wasmi/src/engine/func_builder/mod.rs index 1fae9fe707..fdb1f0d17b 100644 --- a/crates/wasmi/src/engine/func_builder/mod.rs +++ b/crates/wasmi/src/engine/func_builder/mod.rs @@ -19,7 +19,7 @@ use self::{ control_stack::ControlFlowStack, labels::LabelRef, locals_registry::LocalsRegistry, - value_stack::ValueStack, + value_stack::ValueStackHeight, }; pub use self::{ error::TranslationError, @@ -69,6 +69,10 @@ pub struct FuncBuilder<'parser> { reachable: bool, /// The Wasm function validator. validator: FuncValidator, + /// The height of the emulated value stack. + stack_height: ValueStackHeight, + /// Stores and resolves local variable types. + locals: LocalsRegistry, /// The reusable data structures of the [`FuncBuilder`]. alloc: FunctionBuilderAllocations, } @@ -78,16 +82,12 @@ pub struct FuncBuilder<'parser> { pub struct FunctionBuilderAllocations { /// The control flow frame stack that represents the Wasm control flow. control_frames: ControlFlowStack, - /// The emulated value stack. - value_stack: ValueStack, /// The instruction builder. /// /// # Note /// /// Allows to incrementally construct the instruction of a function. inst_builder: InstructionsBuilder, - /// Stores and resolves local variable types. - locals: LocalsRegistry, /// Buffer for translating `br_table`. br_table_branches: Vec, } @@ -101,9 +101,7 @@ impl FunctionBuilderAllocations { /// by another [`FuncBuilder`]. fn reset(&mut self) { self.control_frames.reset(); - self.value_stack.reset(); self.inst_builder.reset(); - self.locals.reset(); self.br_table_branches.clear(); } } @@ -117,14 +115,17 @@ impl<'parser> FuncBuilder<'parser> { validator: FuncValidator, mut allocations: FunctionBuilderAllocations, ) -> Self { + let mut locals = LocalsRegistry::default(); Self::register_func_body_block(func, res, &mut allocations); - Self::register_func_params(func, res, &mut allocations); + Self::register_func_params(func, res, &mut locals); Self { engine: engine.clone(), func, res, reachable: true, validator, + stack_height: ValueStackHeight::default(), + locals, alloc: allocations, } } @@ -169,15 +170,15 @@ impl<'parser> FuncBuilder<'parser> { fn register_func_params( func: FuncIdx, res: ModuleResources<'parser>, - allocations: &mut FunctionBuilderAllocations, + locals: &mut LocalsRegistry, ) -> usize { let dedup_func_type = res.get_type_of_func(func); let func_type = res .engine() .resolve_func_type(dedup_func_type, Clone::clone); let params = func_type.params(); - for param_type in params { - allocations.locals.register_locals(*param_type, 1); + for _param_type in params { + locals.register_locals(1); } params.len() } @@ -190,15 +191,13 @@ impl<'parser> FuncBuilder<'parser> { value_type: wasmparser::ValType, ) -> Result<(), TranslationError> { self.validator.define_locals(offset, amount, value_type)?; - let value_type = crate::module::value_type_from_wasmparser(value_type) - .ok_or_else(|| TranslationError::unsupported_value_type(value_type))?; - self.alloc.locals.register_locals(value_type, amount); + self.locals.register_locals(amount); Ok(()) } /// Returns the number of local variables of the function under construction. fn len_locals(&self) -> usize { - let len_params_locals = self.alloc.locals.len_registered() as usize; + let len_params_locals = self.locals.len_registered() as usize; let len_params = self.func_type().params().len(); debug_assert!(len_params_locals >= len_params); len_params_locals - len_params @@ -213,7 +212,7 @@ impl<'parser> FuncBuilder<'parser> { let func_body = self.alloc.inst_builder.finish( &self.engine, self.len_locals(), - self.alloc.value_stack.max_stack_height() as usize, + self.stack_height.max_stack_height() as usize, ); let allocations = ReusableAllocations { translation: self.alloc, @@ -258,7 +257,7 @@ impl<'parser> FuncBuilder<'parser> { ControlFrameKind::Loop => frame.block_type().len_params(&self.engine), }; // Find out how many values we need to drop. - let current_height = self.alloc.value_stack.len(); + let current_height = self.stack_height.height(); let origin_height = frame.stack_height().expect("frame is reachable"); assert!( origin_height <= current_height, @@ -296,7 +295,7 @@ impl<'parser> FuncBuilder<'parser> { .checked_sub(1) .expect("control flow frame stack must not be empty") as u32; let drop_keep = self.compute_drop_keep(max_depth)?; - let len_params_locals = self.alloc.locals.len_registered() as usize; + let len_params_locals = self.locals.len_registered() as usize; DropKeep::new( // Drop all local variables and parameters upon exit. drop_keep.drop() + len_params_locals, @@ -308,8 +307,8 @@ impl<'parser> FuncBuilder<'parser> { /// Returns the relative depth on the stack of the local variable. fn relative_local_depth(&self, local_idx: u32) -> usize { debug_assert!(self.is_reachable()); - let stack_height = self.alloc.value_stack.len() as usize; - let len_params_locals = self.alloc.locals.len_registered() as usize; + let stack_height = self.stack_height.height() as usize; + let len_params_locals = self.locals.len_registered() as usize; stack_height .checked_add(len_params_locals) .and_then(|x| x.checked_sub(local_idx as usize)) @@ -394,7 +393,7 @@ impl<'parser> FuncBuilder<'parser> { /// since we have already validated the input Wasm prior. fn frame_stack_height(&self, block_type: BlockType) -> u32 { let len_params = block_type.len_params(&self.engine); - let stack_height = self.alloc.value_stack.len(); + let stack_height = self.stack_height.height(); stack_height.checked_sub(len_params).unwrap_or_else(|| { panic!( "encountered emulated value stack underflow with \ @@ -462,8 +461,7 @@ impl<'parser> FuncBuilder<'parser> { ) -> Result<(), TranslationError> { let block_type = BlockType::try_from_wasmparser(block_type, self.res)?; if self.is_reachable() { - let condition = self.alloc.value_stack.pop1(); - debug_assert_eq!(condition, ValueType::I32); + self.stack_height.pop1(); let stack_height = self.frame_stack_height(block_type); let else_label = self.alloc.inst_builder.new_label(); let end_label = self.alloc.inst_builder.new_label(); @@ -523,9 +521,9 @@ impl<'parser> FuncBuilder<'parser> { // We need to reset the value stack to exactly how it has been // when entering the `if` in the first place so that the `else` // block has the same parameters on top of the stack. - self.alloc.value_stack.shrink_to(if_frame.stack_height()); - if_frame.block_type().foreach_param(&self.engine, |param| { - self.alloc.value_stack.push(param); + self.stack_height.shrink_to(if_frame.stack_height()); + if_frame.block_type().foreach_param(&self.engine, |_param| { + self.stack_height.push(); }); self.alloc.control_frames.push_frame(if_frame); // We can reset reachability now since the parent `if` block was reachable. @@ -564,12 +562,12 @@ impl<'parser> FuncBuilder<'parser> { self.reachable = frame_reachable; } if let Some(frame_stack_height) = frame_stack_height { - self.alloc.value_stack.shrink_to(frame_stack_height); + self.stack_height.shrink_to(frame_stack_height); } let frame = self.alloc.control_frames.pop_frame(); frame .block_type() - .foreach_result(&self.engine, |result| self.alloc.value_stack.push(result)); + .foreach_result(&self.engine, |_result| self.stack_height.push()); Ok(()) } @@ -597,8 +595,7 @@ impl<'parser> FuncBuilder<'parser> { /// Translates a Wasm `br_if` control flow operator. pub fn translate_br_if(&mut self, relative_depth: u32) -> Result<(), TranslationError> { self.translate_if_reachable(|builder| { - let condition = builder.alloc.value_stack.pop1(); - debug_assert_eq!(condition, ValueType::I32); + builder.stack_height.pop1(); match builder.acquire_target(relative_depth)? { AcquiredTarget::Branch(end_label, drop_keep) => { let params = builder.branch_params(end_label, drop_keep); @@ -658,9 +655,7 @@ impl<'parser> FuncBuilder<'parser> { }) .map(RelativeDepth::from_u32); - let case = builder.alloc.value_stack.pop1(); - debug_assert_eq!(case, ValueType::I32); - + builder.stack_height.pop1(); builder.alloc.br_table_branches.clear(); for (n, depth) in targets.into_iter().enumerate() { let relative_depth = compute_instr(builder, n, depth)?; @@ -698,13 +693,8 @@ impl<'parser> FuncBuilder<'parser> { /// Adjusts the emulated [`ValueStack`] given the [`FuncType`] of the call. fn adjust_value_stack_for_call(&mut self, func_type: &FuncType) { let (params, results) = func_type.params_results(); - for param in params.iter().rev() { - let popped = self.alloc.value_stack.pop1(); - debug_assert_eq!(popped, *param); - } - for result in results { - self.alloc.value_stack.push(*result); - } + self.stack_height.pop_n(params.len() as u32); + self.stack_height.push_n(results.len() as u32); } /// Translates a Wasm `call` instruction. @@ -735,8 +725,7 @@ impl<'parser> FuncBuilder<'parser> { let func_type_idx = FuncTypeIdx(index); let table_idx = TableIdx(table_index); assert_eq!(table_idx.into_u32(), DEFAULT_TABLE_INDEX); - let func_type_offset = builder.alloc.value_stack.pop1(); - debug_assert_eq!(func_type_offset, ValueType::I32); + builder.stack_height.pop1(); let func_type = builder.func_type_at(func_type_idx); builder.adjust_value_stack_for_call(&func_type); let func_type_idx = func_type_idx.into_u32().into(); @@ -751,7 +740,7 @@ impl<'parser> FuncBuilder<'parser> { /// Translates a Wasm `drop` instruction. pub fn translate_drop(&mut self) -> Result<(), TranslationError> { self.translate_if_reachable(|builder| { - builder.alloc.value_stack.pop1(); + builder.stack_height.pop1(); builder.alloc.inst_builder.push_inst(Instruction::Drop); Ok(()) }) @@ -760,10 +749,8 @@ impl<'parser> FuncBuilder<'parser> { /// Translates a Wasm `select` instruction. pub fn translate_select(&mut self) -> Result<(), TranslationError> { self.translate_if_reachable(|builder| { - let (v0, v1, selector) = builder.alloc.value_stack.pop3(); - debug_assert_eq!(selector, ValueType::I32); - debug_assert_eq!(v0, v1); - builder.alloc.value_stack.push(v0); + builder.stack_height.pop3(); + builder.stack_height.push(); builder.alloc.inst_builder.push_inst(Instruction::Select); Ok(()) }) @@ -777,12 +764,7 @@ impl<'parser> FuncBuilder<'parser> { .alloc .inst_builder .push_inst(Instruction::local_get(local_depth)); - let value_type = builder - .alloc - .locals - .resolve_local(local_idx) - .unwrap_or_else(|| panic!("failed to resolve local {}", local_idx)); - builder.alloc.value_stack.push(value_type); + builder.stack_height.push(); Ok(()) }) } @@ -790,18 +772,12 @@ impl<'parser> FuncBuilder<'parser> { /// Translate a Wasm `local.set` instruction. pub fn translate_local_set(&mut self, local_idx: u32) -> Result<(), TranslationError> { self.translate_if_reachable(|builder| { - let actual = builder.alloc.value_stack.pop1(); + builder.stack_height.pop1(); let local_depth = builder.relative_local_depth(local_idx); builder .alloc .inst_builder .push_inst(Instruction::local_set(local_depth)); - let expected = builder - .alloc - .locals - .resolve_local(local_idx) - .unwrap_or_else(|| panic!("failed to resolve local {}", local_idx)); - debug_assert_eq!(actual, expected); Ok(()) }) } @@ -814,13 +790,6 @@ impl<'parser> FuncBuilder<'parser> { .alloc .inst_builder .push_inst(Instruction::local_tee(local_depth)); - let expected = builder - .alloc - .locals - .resolve_local(local_idx) - .unwrap_or_else(|| panic!("failed to resolve local {}", local_idx)); - let actual = builder.alloc.value_stack.top(); - debug_assert_eq!(actual, expected); Ok(()) }) } @@ -829,8 +798,7 @@ impl<'parser> FuncBuilder<'parser> { pub fn translate_global_get(&mut self, global_idx: u32) -> Result<(), TranslationError> { self.translate_if_reachable(|builder| { let global_idx = GlobalIdx(global_idx); - let global_type = builder.res.get_type_of_global(global_idx); - builder.alloc.value_stack.push(global_type.value_type()); + builder.stack_height.push(); let global_idx = global_idx.into_u32().into(); builder .alloc @@ -846,9 +814,7 @@ impl<'parser> FuncBuilder<'parser> { let global_idx = GlobalIdx(global_idx); let global_type = builder.res.get_type_of_global(global_idx); debug_assert_eq!(global_type.mutability(), Mutability::Mutable); - let expected = global_type.value_type(); - let actual = builder.alloc.value_stack.pop1(); - debug_assert_eq!(actual, expected); + builder.stack_height.pop1(); let global_idx = global_idx.into_u32().into(); builder .alloc @@ -888,15 +854,14 @@ impl<'parser> FuncBuilder<'parser> { fn translate_load( &mut self, memarg: wasmparser::MemArg, - loaded_type: ValueType, + _loaded_type: ValueType, make_inst: fn(Offset) -> Instruction, ) -> Result<(), TranslationError> { self.translate_if_reachable(|builder| { let (memory_idx, offset) = Self::decompose_memarg(memarg); debug_assert_eq!(memory_idx.into_u32(), DEFAULT_MEMORY_INDEX); - let pointer = builder.alloc.value_stack.pop1(); - debug_assert_eq!(pointer, ValueType::I32); - builder.alloc.value_stack.push(loaded_type); + builder.stack_height.pop1(); + builder.stack_height.push(); let offset = Offset::from(offset); builder.alloc.inst_builder.push_inst(make_inst(offset)); Ok(()) @@ -1033,15 +998,13 @@ impl<'parser> FuncBuilder<'parser> { fn translate_store( &mut self, memarg: wasmparser::MemArg, - stored_value: ValueType, + _stored_value: ValueType, make_inst: fn(Offset) -> Instruction, ) -> Result<(), TranslationError> { self.translate_if_reachable(|builder| { let (memory_idx, offset) = Self::decompose_memarg(memarg); debug_assert_eq!(memory_idx.into_u32(), DEFAULT_MEMORY_INDEX); - let (pointer, stored) = builder.alloc.value_stack.pop2(); - debug_assert_eq!(pointer, ValueType::I32); - assert_eq!(stored_value, stored); + builder.stack_height.pop2(); let offset = Offset::from(offset); builder.alloc.inst_builder.push_inst(make_inst(offset)); Ok(()) @@ -1129,7 +1092,7 @@ impl<'parser> FuncBuilder<'parser> { self.translate_if_reachable(|builder| { let memory_idx = MemoryIdx(memory_idx); debug_assert_eq!(memory_idx.into_u32(), DEFAULT_MEMORY_INDEX); - builder.alloc.value_stack.push(ValueType::I32); + builder.stack_height.push(); builder .alloc .inst_builder @@ -1147,7 +1110,6 @@ impl<'parser> FuncBuilder<'parser> { self.translate_if_reachable(|builder| { let memory_idx = MemoryIdx(memory_idx); debug_assert_eq!(memory_idx.into_u32(), DEFAULT_MEMORY_INDEX); - debug_assert_eq!(builder.alloc.value_stack.top(), ValueType::I32); builder .alloc .inst_builder @@ -1172,7 +1134,7 @@ impl<'parser> FuncBuilder<'parser> { { self.translate_if_reachable(|builder| { let value = value.into(); - builder.alloc.value_stack.push(value.value_type()); + builder.stack_height.push(); builder .alloc .inst_builder @@ -1217,13 +1179,12 @@ impl<'parser> FuncBuilder<'parser> { /// - `i64.eqz` fn translate_unary_cmp( &mut self, - input_type: ValueType, + _input_type: ValueType, inst: Instruction, ) -> Result<(), TranslationError> { self.translate_if_reachable(|builder| { - let condition = builder.alloc.value_stack.pop1(); - debug_assert_eq!(condition, input_type); - builder.alloc.value_stack.push(ValueType::I32); + builder.stack_height.pop1(); + builder.stack_height.push(); builder.alloc.inst_builder.push_inst(inst); Ok(()) }) @@ -1248,14 +1209,12 @@ impl<'parser> FuncBuilder<'parser> { /// - `{i32, u32, i64, u64, f32, f64}.ge` fn translate_binary_cmp( &mut self, - input_type: ValueType, + _input_type: ValueType, inst: Instruction, ) -> Result<(), TranslationError> { self.translate_if_reachable(|builder| { - let (v0, v1) = builder.alloc.value_stack.pop2(); - debug_assert_eq!(v0, v1); - debug_assert_eq!(v0, input_type); - builder.alloc.value_stack.push(ValueType::I32); + builder.stack_height.pop2(); + builder.stack_height.push(); builder.alloc.inst_builder.push_inst(inst); Ok(()) }) @@ -1444,12 +1403,10 @@ impl<'parser> FuncBuilder<'parser> { /// - `{f32, f64}.sqrt` pub fn translate_unary_operation( &mut self, - value_type: ValueType, + _value_type: ValueType, inst: Instruction, ) -> Result<(), TranslationError> { self.translate_if_reachable(|builder| { - let actual_type = builder.alloc.value_stack.top(); - debug_assert_eq!(actual_type, value_type); builder.alloc.inst_builder.push_inst(inst); Ok(()) }) @@ -1493,14 +1450,12 @@ impl<'parser> FuncBuilder<'parser> { /// - `{f32, f64}.copysign` pub fn translate_binary_operation( &mut self, - value_type: ValueType, + _value_type: ValueType, inst: Instruction, ) -> Result<(), TranslationError> { self.translate_if_reachable(|builder| { - let (v0, v1) = builder.alloc.value_stack.pop2(); - debug_assert_eq!(v0, v1); - debug_assert_eq!(v0, value_type); - builder.alloc.value_stack.push(value_type); + builder.stack_height.pop2(); + builder.stack_height.push(); builder.alloc.inst_builder.push_inst(inst); Ok(()) }) @@ -1829,14 +1784,13 @@ impl<'parser> FuncBuilder<'parser> { /// - `f64.reinterpret_i64` pub fn translate_conversion( &mut self, - input_type: ValueType, - output_type: ValueType, + _input_type: ValueType, + _output_type: ValueType, inst: Instruction, ) -> Result<(), TranslationError> { self.translate_if_reachable(|builder| { - let input = builder.alloc.value_stack.pop1(); - debug_assert_eq!(input, input_type); - builder.alloc.value_stack.push(output_type); + builder.stack_height.pop1(); + builder.stack_height.push(); builder.alloc.inst_builder.push_inst(inst); Ok(()) }) diff --git a/crates/wasmi/src/engine/func_builder/value_stack.rs b/crates/wasmi/src/engine/func_builder/value_stack.rs index ac66577f2e..04224a6863 100644 --- a/crates/wasmi/src/engine/func_builder/value_stack.rs +++ b/crates/wasmi/src/engine/func_builder/value_stack.rs @@ -1,121 +1,87 @@ -use alloc::vec::Vec; use core::cmp; -use wasmi_core::ValueType; -/// The value stack that is emulated during Wasm to `wasmi` bytecode translation. -#[derive(Debug, Default)] -pub struct ValueStack { - /// The values of the emulated value stack. - values: Vec, +/// The current height of the emulated Wasm value stack. +#[derive(Debug, Default, Copy, Clone)] +pub struct ValueStackHeight { + /// The current height of the emulated value stack of the translated function. + /// + /// # Note + /// + /// This does not include input parameters and local variables. + height: u32, /// The maximum height of the emulated value stack of the translated function. /// /// # Note /// /// This does not include input parameters and local variables. - max_stack_height: u32, + max_height: u32, } -impl ValueStack { - /// Resets the [`ValueStack`] to allow for reuse. - pub fn reset(&mut self) { - self.values.clear(); - self.max_stack_height = 0; +impl ValueStackHeight { + /// Returns the current length of the emulated value stack. + /// + /// # Note + /// + /// This does not include input parameters and local variables. + pub fn height(&self) -> u32 { + self.height } /// Returns the maximum value stack height. + /// + /// # Note + /// + /// This does not include input parameters and local variables. pub fn max_stack_height(&self) -> u32 { - self.max_stack_height + self.max_height } - /// Updates the pinned maximum stack height. + /// Updates the pinned maximum value stack height. fn update_max_height(&mut self) { - let max = self.max_stack_height; - let len = self.len(); - self.max_stack_height = cmp::max(len, max); + self.max_height = cmp::max(self.height, self.max_height); } - /// Pushes the [`ValueType`] to the emulated [`ValueStack`]. - /// - /// # Note - /// - /// In this [`ValueStack`] we push [`ValueType`] instead of [`Value`] - /// to the stack since we are just emulating the Wasm [`ValueStack`] during - /// translation from Wasm bytecode to `wasmi` bytecode. - /// - /// [`Value`]: [`wasmi_core::Value`] - pub fn push(&mut self, value_type: ValueType) { - self.values.push(value_type); + /// Pushes an `amount` of values to the emulated value stack. + pub fn push_n(&mut self, amount: u32) { + self.height += amount; self.update_max_height(); } - /// Returns the top most [`ValueType`] form the emulated [`ValueStack`]. - pub fn top(&self) -> ValueType { - *self - .values - .last() - .expect("tried to peek last value from empty emulated value stack") + /// Pushes a value to the emulated value stack. + pub fn push(&mut self) { + self.push_n(1) } - /// Pops the top most [`ValueType`] from the emulated [`ValueStack`]. - /// - /// # Panics - /// - /// If the emulated [`ValueStack`] is empty. - pub fn pop1(&mut self) -> ValueType { - self.values - .pop() - .expect("tried to pop value from an empty emulated value stack") + /// Pops an `amount` of elements from the emulated value stack. + pub fn pop_n(&mut self, amount: u32) { + debug_assert!(amount <= self.height); + self.height -= amount; } - /// Pops the 2 top most [`ValueType`] from the emulated [`ValueStack`]. - /// - /// # Panics - /// - /// If the emulated [`ValueStack`] is empty. - pub fn pop2(&mut self) -> (ValueType, ValueType) { - let rhs = self.pop1(); - let lhs = self.pop1(); - (lhs, rhs) + /// Pops 1 element from the emulated value stack. + pub fn pop1(&mut self) { + self.pop_n(1) } - /// Pops the 3 top most [`ValueType`] from the emulated [`ValueStack`]. - /// - /// # Panics - /// - /// If the emulated [`ValueStack`] is empty. - pub fn pop3(&mut self) -> (ValueType, ValueType, ValueType) { - let v2 = self.pop1(); - let v1 = self.pop1(); - let v0 = self.pop1(); - (v0, v1, v2) + /// Pops 2 elements from the emulated value stack. + pub fn pop2(&mut self) { + self.pop_n(2) } - /// Returns the current length of the emulated [`ValueStack`]. - pub fn len(&self) -> u32 { - self.values.len() as u32 + /// Pops 3 elements from the emulated value stack. + pub fn pop3(&mut self) { + self.pop_n(3) } - /// Shrinks the [`ValueStack`] to the given height. + /// Shrinks the emulated value stack to the given height. /// /// # Panics /// - /// If the [`ValueStack`] height already is below the height since this + /// If the value stack height already is below the height since this /// usually indicates a bug in the translation of the Wasm to `wasmi` /// bytecode procedures. pub fn shrink_to(&mut self, new_height: u32) { - let current_height = self.len(); - assert!( - new_height <= current_height, - "tried to shrink the value stack of height {} to height {}", - current_height, - new_height - ); - let new_height = usize::try_from(new_height).unwrap_or_else(|error| { - panic!( - "could not convert stack height from `u32` to `usize`: {}", - error - ) - }); - self.values.resize(new_height, ValueType::I32); + assert!(new_height <= self.height); + self.height = new_height; } } From 48ae16b77406c15e2cc00d27dab8e9cd6859013d Mon Sep 17 00:00:00 2001 From: Robin Freyler Date: Thu, 20 Oct 2022 15:41:33 +0200 Subject: [PATCH 2/2] fix internal doc links --- crates/wasmi/src/engine/func_builder/locals_registry.rs | 2 +- crates/wasmi/src/engine/func_builder/mod.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/wasmi/src/engine/func_builder/locals_registry.rs b/crates/wasmi/src/engine/func_builder/locals_registry.rs index 0549ff7fb1..c1c482c4af 100644 --- a/crates/wasmi/src/engine/func_builder/locals_registry.rs +++ b/crates/wasmi/src/engine/func_builder/locals_registry.rs @@ -35,7 +35,7 @@ impl LocalsRegistry { self.len_registered } - /// Registers the `amount` of locals with their shared [`ValueType`]. + /// Registers an `amount` of local variables. /// /// # Panics /// diff --git a/crates/wasmi/src/engine/func_builder/mod.rs b/crates/wasmi/src/engine/func_builder/mod.rs index fdb1f0d17b..633c6237de 100644 --- a/crates/wasmi/src/engine/func_builder/mod.rs +++ b/crates/wasmi/src/engine/func_builder/mod.rs @@ -690,7 +690,7 @@ impl<'parser> FuncBuilder<'parser> { }) } - /// Adjusts the emulated [`ValueStack`] given the [`FuncType`] of the call. + /// Adjusts the emulated value stack given the [`FuncType`] of the call. fn adjust_value_stack_for_call(&mut self, func_type: &FuncType) { let (params, results) = func_type.params_results(); self.stack_height.pop_n(params.len() as u32);