reduce instruction enum size

explodingcamera · explodingcamera · commit 83a768d77ac5 · 2024-02-27T01:50:54.000+01:00
Signed-off-by: Henry Gressmann &lt;mail@henrygressmann.de&gt;
diff --git 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<ConstInstruction> {
diff --git 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::<Result<Vec<Instruction>, 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
@@ -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
@@ -52,7 +52,7 @@ macro_rules! mem_load {
         })?;
 
         const LEN: usize = core::mem::size_of::<$load_type>();
-        let val = mem_ref.load_as::<LEN, $load_type>(addr, $arg.align as usize)?;
+        let val = mem_ref.load_as::<LEN, $load_type>(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
@@ -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,25 +217,25 @@ 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;
             }
         }
 
         Loop(args, end_offset) => {
             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::<Result<Vec<_>>>()?;
 
-            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::<i64>()?;
diff --git 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
@@ -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<const SIZE: usize, T: MemLoadable<SIZE>>(&self, addr: usize, _align: usize) -> Result<T> {
+    pub(crate) fn load_as<const SIZE: usize, T: MemLoadable<SIZE>>(&self, addr: usize) -> Result<T> {
         let Some(end) = addr.checked_add(SIZE) else {
             return Err(self.trap_oob(addr, SIZE));
         };
@@ -223,16 +223,16 @@ 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);
     }
 
     #[test]
     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
@@ -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
@@ -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

Original file line number	Diff line number	Diff line change
`@@ -226,7 +226,7 @@ pub(crate) fn convert_valtype(valtype: &wasmparser::ValType) -> ValType {`
`226`	`226`	`}`
`227`	`227`
`228`	`228`	`pub(crate) fn convert_memarg(memarg: wasmparser::MemArg) -> MemoryArg {`
`229`		`- MemoryArg { offset: memarg.offset, align: memarg.align, align_max: memarg.max_align, mem_addr: memarg.memory }`
	`229`	`+ MemoryArg { offset: memarg.offset, mem_addr: memarg.memory }`
`230`	`230`	`}`
`231`	`231`
`232`	`232`	`pub(crate) fn process_const_operators(ops: OperatorsReader<'_>) -> Result<ConstInstruction> {`
Original file line number	Diff line number	Diff line change
`@@ -26,21 +26,21 @@ pub struct MemoryRefMut<'a> {`
`26`	`26`	`impl<'a> MemoryRefLoad for MemoryRef<'a> {`
`27`	`27`	`/// Load a slice of memory`
`28`	`28`	`fn load(&self, offset: usize, len: usize) -> Result<&[u8]> {`
`29`		`- self.instance.load(offset, 0, len)`
	`29`	`+ self.instance.load(offset, len)`
`30`	`30`	`}`
`31`	`31`	`}`
`32`	`32`
`33`	`33`	`impl<'a> MemoryRefLoad for MemoryRefMut<'a> {`
`34`	`34`	`/// Load a slice of memory`
`35`	`35`	`fn load(&self, offset: usize, len: usize) -> Result<&[u8]> {`
`36`		`- self.instance.load(offset, 0, len)`
	`36`	`+ self.instance.load(offset, len)`
`37`	`37`	`}`
`38`	`38`	`}`
`39`	`39`
`40`	`40`	`impl MemoryRef<'_> {`
`41`	`41`	`/// Load a slice of memory`
`42`	`42`	`pub fn load(&self, offset: usize, len: usize) -> Result<&[u8]> {`
`43`		`- self.instance.load(offset, 0, len)`
	`43`	`+ self.instance.load(offset, len)`
`44`	`44`	`}`
`45`	`45`
`46`	`46`	`/// Load a slice of memory as a vector`
`@@ -52,7 +52,7 @@ impl MemoryRef<'_> {`
`52`	`52`	`impl MemoryRefMut<'_> {`
`53`	`53`	`/// Load a slice of memory`
`54`	`54`	`pub fn load(&self, offset: usize, len: usize) -> Result<&[u8]> {`
`55`		`- self.instance.load(offset, 0, len)`
	`55`	`+ self.instance.load(offset, len)`
`56`	`56`	`}`
`57`	`57`
`58`	`58`	`/// Load a slice of memory as a vector`
`@@ -82,7 +82,7 @@ impl MemoryRefMut<'_> {`
`82`	`82`
`83`	`83`	`/// Store a slice of memory`
`84`	`84`	`pub fn store(&mut self, offset: usize, len: usize, data: &[u8]) -> Result<()> {`
`85`		`- self.instance.store(offset, 0, data, len)`
	`85`	`+ self.instance.store(offset, len, data)`
`86`	`86`	`}`
`87`	`87`	`}`
`88`	`88`
Original file line number	Diff line number	Diff line change
`@@ -52,7 +52,7 @@ macro_rules! mem_load {`
`52`	`52`	`})?;`
`53`	`53`
`54`	`54`	`const LEN: usize = core::mem::size_of::<$load_type>();`
`55`		`- let val = mem_ref.load_as::<LEN, $load_type>(addr, $arg.align as usize)?;`
	`55`	`+ let val = mem_ref.load_as::<LEN, $load_type>(addr)?;`
`56`	`56`	`$stack.values.push((val as $target_type).into());`
`57`	`57`	`}};`
`58`	`58`	`}`
`@@ -76,7 +76,7 @@ macro_rules! mem_store {`
`76`	`76`	`let val = val as $store_type;`
`77`	`77`	`let val = val.to_le_bytes();`
`78`	`78`
`79`		`- mem.borrow_mut().store(($arg.offset + addr) as usize, $arg.align as usize, &val, val.len())?;`
	`79`	`+ mem.borrow_mut().store(($arg.offset + addr) as usize, val.len(), &val)?;`
`80`	`80`	`}};`
`81`	`81`	`}`
`82`	`82`