From 83a768d77ac57f5d8e8884173765e0efc80831f4 Mon Sep 17 00:00:00 2001 From: Henry Gressmann Date: Tue, 27 Feb 2024 01:50:54 +0100 Subject: reduce instruction enum size Signed-off-by: Henry Gressmann --- crates/parser/src/conversion.rs | 2 +- crates/parser/src/visit.rs | 29 +++++++------- crates/tinywasm/src/reference.rs | 10 ++--- crates/tinywasm/src/runtime/interpreter/macros.rs | 4 +- crates/tinywasm/src/runtime/interpreter/mod.rs | 29 ++++++++------ crates/tinywasm/src/runtime/value.rs | 1 - crates/tinywasm/src/store/memory.rs | 12 +++--- crates/tinywasm/src/store/mod.rs | 46 +++++++++++------------ crates/types/src/instructions.rs | 12 +++--- 9 files changed, 76 insertions(+), 69 deletions(-) (limited to 'crates') diff --git a/crates/parser/src/conversion.rs b/crates/parser/src/conversion.rs index 53cceb6..c13d08f 100644 --- a/crates/parser/src/conversion.rs +++ b/crates/parser/src/conversion.rs @@ -226,7 +226,7 @@ pub(crate) fn convert_valtype(valtype: &wasmparser::ValType) -> ValType { } pub(crate) fn convert_memarg(memarg: wasmparser::MemArg) -> MemoryArg { - MemoryArg { offset: memarg.offset, align: memarg.align, align_max: memarg.max_align, mem_addr: memarg.memory } + MemoryArg { offset: memarg.offset, mem_addr: memarg.memory } } pub(crate) fn process_const_operators(ops: OperatorsReader<'_>) -> Result { diff --git a/crates/parser/src/visit.rs b/crates/parser/src/visit.rs index f994e7e..3a10a93 100644 --- a/crates/parser/src/visit.rs +++ b/crates/parser/src/visit.rs @@ -338,15 +338,16 @@ impl<'a> wasmparser::VisitOperator<'a> for FunctionBuilder { } fn visit_local_set(&mut self, idx: u32) -> Self::Output { - if self.instructions.len() < 1 { - return self.visit(Instruction::I64Rotl); + if let Some(instruction) = self.instructions.last_mut() { + match instruction { + // Needs more testing, seems to make performance worse + // Instruction::LocalGet(a) => *instruction = Instruction::LocalGetSet(*a, idx), + _ => return self.visit(Instruction::LocalSet(idx)), + }; + // Ok(()) + } else { + self.visit(Instruction::LocalSet(idx)) } - - // LocalGetSet - match self.instructions[self.instructions.len() - 1..] { - // Instruction::LocalGet(a) => *instruction = Instruction::LocalGetSet(*a, idx), - _ => return self.visit(Instruction::LocalSet(idx)), - }; } fn visit_local_tee(&mut self, idx: u32) -> Self::Output { @@ -413,7 +414,7 @@ impl<'a> wasmparser::VisitOperator<'a> for FunctionBuilder { match self.instructions[label_pointer] { Instruction::Else(ref mut else_instr_end_offset) => { - *else_instr_end_offset = current_instr_ptr - label_pointer; + *else_instr_end_offset = (current_instr_ptr - label_pointer as usize) as u32; #[cold] fn error() -> crate::ParseError { @@ -430,13 +431,13 @@ impl<'a> wasmparser::VisitOperator<'a> for FunctionBuilder { return Err(error()); }; - *else_offset = Some(label_pointer - if_label_pointer); - *end_offset = current_instr_ptr - if_label_pointer; + *else_offset = Some((label_pointer - if_label_pointer) as u32); + *end_offset = (current_instr_ptr - if_label_pointer) as u32; } Instruction::Block(_, ref mut end_offset) | Instruction::Loop(_, ref mut end_offset) | Instruction::If(_, _, ref mut end_offset) => { - *end_offset = current_instr_ptr - label_pointer; + *end_offset = (current_instr_ptr - label_pointer) as u32; } _ => { return Err(crate::ParseError::UnsupportedOperator( @@ -456,7 +457,9 @@ impl<'a> wasmparser::VisitOperator<'a> for FunctionBuilder { .collect::, wasmparser::BinaryReaderError>>() .expect("BrTable targets are invalid, this should have been caught by the validator"); - self.instructions.extend(IntoIterator::into_iter([Instruction::BrTable(def, instrs.len())]).chain(instrs)); + self.instructions + .extend(IntoIterator::into_iter([Instruction::BrTable(def, instrs.len() as u32)]).chain(instrs)); + Ok(()) } diff --git a/crates/tinywasm/src/reference.rs b/crates/tinywasm/src/reference.rs index 4c6d703..6713a42 100644 --- a/crates/tinywasm/src/reference.rs +++ b/crates/tinywasm/src/reference.rs @@ -26,21 +26,21 @@ pub struct MemoryRefMut<'a> { impl<'a> MemoryRefLoad for MemoryRef<'a> { /// Load a slice of memory fn load(&self, offset: usize, len: usize) -> Result<&[u8]> { - self.instance.load(offset, 0, len) + self.instance.load(offset, len) } } impl<'a> MemoryRefLoad for MemoryRefMut<'a> { /// Load a slice of memory fn load(&self, offset: usize, len: usize) -> Result<&[u8]> { - self.instance.load(offset, 0, len) + self.instance.load(offset, len) } } impl MemoryRef<'_> { /// Load a slice of memory pub fn load(&self, offset: usize, len: usize) -> Result<&[u8]> { - self.instance.load(offset, 0, len) + self.instance.load(offset, len) } /// Load a slice of memory as a vector @@ -52,7 +52,7 @@ impl MemoryRef<'_> { impl MemoryRefMut<'_> { /// Load a slice of memory pub fn load(&self, offset: usize, len: usize) -> Result<&[u8]> { - self.instance.load(offset, 0, len) + self.instance.load(offset, len) } /// Load a slice of memory as a vector @@ -82,7 +82,7 @@ impl MemoryRefMut<'_> { /// Store a slice of memory pub fn store(&mut self, offset: usize, len: usize, data: &[u8]) -> Result<()> { - self.instance.store(offset, 0, data, len) + self.instance.store(offset, len, data) } } diff --git a/crates/tinywasm/src/runtime/interpreter/macros.rs b/crates/tinywasm/src/runtime/interpreter/macros.rs index 1da8758..a13531b 100644 --- a/crates/tinywasm/src/runtime/interpreter/macros.rs +++ b/crates/tinywasm/src/runtime/interpreter/macros.rs @@ -52,7 +52,7 @@ macro_rules! mem_load { })?; const LEN: usize = core::mem::size_of::<$load_type>(); - let val = mem_ref.load_as::(addr, $arg.align as usize)?; + let val = mem_ref.load_as::(addr)?; $stack.values.push((val as $target_type).into()); }}; } @@ -76,7 +76,7 @@ macro_rules! mem_store { let val = val as $store_type; let val = val.to_le_bytes(); - mem.borrow_mut().store(($arg.offset + addr) as usize, $arg.align as usize, &val, val.len())?; + mem.borrow_mut().store(($arg.offset + addr) as usize, val.len(), &val)?; }}; } diff --git a/crates/tinywasm/src/runtime/interpreter/mod.rs b/crates/tinywasm/src/runtime/interpreter/mod.rs index 91d1859..45e10b7 100644 --- a/crates/tinywasm/src/runtime/interpreter/mod.rs +++ b/crates/tinywasm/src/runtime/interpreter/mod.rs @@ -202,7 +202,7 @@ fn exec_one(cf: &mut CallFrame, stack: &mut Stack, store: &mut Store, module: &M cf.enter_block( BlockFrame::new( cf.instr_ptr, - cf.instr_ptr + end_offset, + cf.instr_ptr + end_offset as usize, stack.values.len(), BlockType::If, &args, @@ -217,17 +217,17 @@ fn exec_one(cf: &mut CallFrame, stack: &mut Stack, store: &mut Store, module: &M // falsy value is on the top of the stack if let Some(else_offset) = else_offset { let label = BlockFrame::new( - cf.instr_ptr + else_offset, - cf.instr_ptr + end_offset, + cf.instr_ptr + else_offset as usize, + cf.instr_ptr + end_offset as usize, stack.values.len(), BlockType::Else, &args, module, ); - cf.instr_ptr += else_offset; + cf.instr_ptr += else_offset as usize; cf.enter_block(label, &mut stack.values, &mut stack.blocks); } else { - cf.instr_ptr += end_offset; + cf.instr_ptr += end_offset as usize; } } @@ -235,7 +235,7 @@ fn exec_one(cf: &mut CallFrame, stack: &mut Stack, store: &mut Store, module: &M cf.enter_block( BlockFrame::new( cf.instr_ptr, - cf.instr_ptr + end_offset, + cf.instr_ptr + end_offset as usize, stack.values.len(), BlockType::Loop, &args, @@ -250,7 +250,7 @@ fn exec_one(cf: &mut CallFrame, stack: &mut Stack, store: &mut Store, module: &M cf.enter_block( BlockFrame::new( cf.instr_ptr, - cf.instr_ptr + end_offset, + cf.instr_ptr + end_offset as usize, stack.values.len(), // - params, BlockType::Block, &args, @@ -262,7 +262,7 @@ fn exec_one(cf: &mut CallFrame, stack: &mut Stack, store: &mut Store, module: &M } BrTable(default, len) => { - let instr = cf.instructions()[cf.instr_ptr + 1..cf.instr_ptr + 1 + len] + let instr = cf.instructions()[cf.instr_ptr + 1..cf.instr_ptr + 1 + len as usize] .iter() .map(|i| match i { BrLabel(l) => Ok(*l), @@ -273,7 +273,7 @@ fn exec_one(cf: &mut CallFrame, stack: &mut Stack, store: &mut Store, module: &M }) .collect::>>()?; - if unlikely(instr.len() != len) { + if unlikely(instr.len() != len as usize) { panic!( "Expected {} BrLabel instructions, got {}, this should have been validated by the parser", len, @@ -319,7 +319,7 @@ fn exec_one(cf: &mut CallFrame, stack: &mut Stack, store: &mut Store, module: &M let res_count = block.results; stack.values.truncate_keep(block.stack_ptr, res_count); - cf.instr_ptr += end_offset; + cf.instr_ptr += end_offset as usize; } EndBlockFrame => { @@ -409,7 +409,7 @@ fn exec_one(cf: &mut CallFrame, stack: &mut Stack, store: &mut Store, module: &M // copy between two memories let mem2 = store.get_mem(module.resolve_mem_addr(to) as usize)?; let mut mem2 = mem2.borrow_mut(); - mem2.copy_from_slice(dst as usize, mem.load(src as usize, 0, size as usize)?)?; + mem2.copy_from_slice(dst as usize, mem.load(src as usize, size as usize)?)?; } } @@ -447,7 +447,7 @@ fn exec_one(cf: &mut CallFrame, stack: &mut Stack, store: &mut Store, module: &M let data = &data[offset..(offset + size)]; // mem.store checks bounds - mem.store(dst, 0, data, size)?; + mem.store(dst, size, data)?; } DataDrop(data_index) => { @@ -704,6 +704,11 @@ fn exec_one(cf: &mut CallFrame, stack: &mut Stack, store: &mut Store, module: &M stack.values.push(cf.get_local(b as usize)); } + LocalGetSet(a, b) => { + let a = cf.get_local(a as usize); + cf.set_local(b as usize, a); + } + // I64Xor + I64Const + I64RotL I64XorConstRotl(rotate_by) => { let val = stack.values.pop_t::()?; diff --git a/crates/tinywasm/src/runtime/value.rs b/crates/tinywasm/src/runtime/value.rs index 56fdf60..4835eab 100644 --- a/crates/tinywasm/src/runtime/value.rs +++ b/crates/tinywasm/src/runtime/value.rs @@ -7,7 +7,6 @@ use tinywasm_types::{ValType, WasmValue}; /// /// See [`WasmValue`] for the public representation. #[derive(Clone, Copy, Default, PartialEq, Eq)] -#[repr(transparent)] // pub struct RawWasmValue([u8; 16]); pub struct RawWasmValue([u8; 8]); diff --git a/crates/tinywasm/src/store/memory.rs b/crates/tinywasm/src/store/memory.rs index cbaed8d..1c8acce 100644 --- a/crates/tinywasm/src/store/memory.rs +++ b/crates/tinywasm/src/store/memory.rs @@ -37,7 +37,7 @@ impl MemoryInstance { Error::Trap(crate::Trap::MemoryOutOfBounds { offset: addr, len, max: self.data.len() }) } - pub(crate) fn store(&mut self, addr: usize, _align: usize, data: &[u8], len: usize) -> Result<()> { + pub(crate) fn store(&mut self, addr: usize, len: usize, data: &[u8]) -> Result<()> { let Some(end) = addr.checked_add(len) else { return Err(self.trap_oob(addr, data.len())); }; @@ -67,7 +67,7 @@ impl MemoryInstance { self.kind.page_count_max.unwrap_or(MAX_PAGES as u64) as usize } - pub(crate) fn load(&self, addr: usize, _align: usize, len: usize) -> Result<&[u8]> { + pub(crate) fn load(&self, addr: usize, len: usize) -> Result<&[u8]> { let Some(end) = addr.checked_add(len) else { return Err(self.trap_oob(addr, len)); }; @@ -80,7 +80,7 @@ impl MemoryInstance { } // this is a workaround since we can't use generic const expressions yet (https://github.com/rust-lang/rust/issues/76560) - pub(crate) fn load_as>(&self, addr: usize, _align: usize) -> Result { + pub(crate) fn load_as>(&self, addr: usize) -> Result { let Some(end) = addr.checked_add(SIZE) else { return Err(self.trap_oob(addr, SIZE)); }; @@ -223,8 +223,8 @@ mod memory_instance_tests { fn test_memory_store_and_load() { let mut memory = create_test_memory(); let data_to_store = [1, 2, 3, 4]; - assert!(memory.store(0, 0, &data_to_store, data_to_store.len()).is_ok()); - let loaded_data = memory.load(0, 0, data_to_store.len()).unwrap(); + assert!(memory.store(0, data_to_store.len(), &data_to_store).is_ok()); + let loaded_data = memory.load(0, data_to_store.len()).unwrap(); assert_eq!(loaded_data, &data_to_store); } @@ -232,7 +232,7 @@ mod memory_instance_tests { fn test_memory_store_out_of_bounds() { let mut memory = create_test_memory(); let data_to_store = [1, 2, 3, 4]; - assert!(memory.store(memory.data.len(), 0, &data_to_store, data_to_store.len()).is_err()); + assert!(memory.store(memory.data.len(), data_to_store.len(), &data_to_store).is_err()); } #[test] diff --git a/crates/tinywasm/src/store/mod.rs b/crates/tinywasm/src/store/mod.rs index ce02dd7..a3d99fe 100644 --- a/crates/tinywasm/src/store/mod.rs +++ b/crates/tinywasm/src/store/mod.rs @@ -349,36 +349,36 @@ impl Store { let data_count = self.data.datas.len(); let mut data_addrs = Vec::with_capacity(data_count); for (i, data) in datas.into_iter().enumerate() { - let data_val = - match data.kind { - tinywasm_types::DataKind::Active { mem: mem_addr, offset } => { - // a. Assert: memidx == 0 - if mem_addr != 0 { - return Err(Error::UnsupportedFeature("data segments for non-zero memories".to_string())); - } + let data_val = match data.kind { + tinywasm_types::DataKind::Active { mem: mem_addr, offset } => { + // a. Assert: memidx == 0 + if mem_addr != 0 { + return Err(Error::UnsupportedFeature("data segments for non-zero memories".to_string())); + } - let mem_addr = mem_addrs.get(mem_addr as usize).copied().ok_or_else(|| { - Error::Other(format!("memory {} not found for data segment {}", mem_addr, i)) - })?; + let mem_addr = mem_addrs + .get(mem_addr as usize) + .copied() + .ok_or_else(|| Error::Other(format!("memory {} not found for data segment {}", mem_addr, i)))?; - let offset = self.eval_i32_const(&offset)?; + let offset = self.eval_i32_const(&offset)?; - let mem = self.data.memories.get_mut(mem_addr as usize).ok_or_else(|| { + let mem = + self.data.memories.get_mut(mem_addr as usize).ok_or_else(|| { Error::Other(format!("memory {} not found for data segment {}", mem_addr, i)) })?; - // See comment for active element sections in the function above why we need to do this here - if let Err(Error::Trap(trap)) = - mem.borrow_mut().store(offset as usize, 0, &data.data, data.data.len()) - { - return Ok((data_addrs.into_boxed_slice(), Some(trap))); - } - - // drop the data - None + // See comment for active element sections in the function above why we need to do this here + if let Err(Error::Trap(trap)) = mem.borrow_mut().store(offset as usize, data.data.len(), &data.data) + { + return Ok((data_addrs.into_boxed_slice(), Some(trap))); } - tinywasm_types::DataKind::Passive => Some(data.data.to_vec()), - }; + + // drop the data + None + } + tinywasm_types::DataKind::Passive => Some(data.data.to_vec()), + }; self.data.datas.push(DataInstance::new(data_val, idx)); data_addrs.push((i + data_count) as Addr); diff --git a/crates/types/src/instructions.rs b/crates/types/src/instructions.rs index dd941b3..a19b4d2 100644 --- a/crates/types/src/instructions.rs +++ b/crates/types/src/instructions.rs @@ -15,14 +15,14 @@ pub enum BlockArgs { pub struct MemoryArg { pub offset: u64, pub mem_addr: MemAddr, - pub align: u8, - pub align_max: u8, + // pub align: u8, + // pub align_max: u8, } type BrTableDefault = u32; -type BrTableLen = usize; -type EndOffset = usize; -type ElseOffset = usize; +type BrTableLen = u32; +type EndOffset = u32; +type ElseOffset = u32; #[derive(Debug, Clone, Copy, PartialEq)] #[cfg_attr(feature = "archive", derive(rkyv::Archive, rkyv::Serialize, rkyv::Deserialize), archive(check_bytes))] @@ -62,7 +62,7 @@ pub enum Instruction { // LocalGet + I32Const + I32Store // Also common, helps us skip the stack entirely - I32LocalGetConstStore(LocalAddr, i32, MemoryArg), // I32Store + LocalGet + I32Const + // I32LocalGetConstStore(LocalAddr, i32, MemoryArg), // I32Store + LocalGet + I32Const // I64Xor + I64Const + I64RotL // Commonly used by a few crypto libraries -- cgit v1.3.1