Skip to content

Commit 3e2e964

Browse files
explodingcameraexplodingcamera
committed
chore: simplify InternalValue trait, improve address calculations
Signed-off-by: Henry <mail@henrygressmann.de>
1 parent de980a6 commit 3e2e964

10 files changed

Lines changed: 67 additions & 116 deletions

File tree

‎crates/tinywasm/src/interpreter/executor.rs‎

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1282,7 +1282,7 @@ impl<'store, const BUDGETED: bool> Executor<'store, BUDGETED> {
12821282
) -> Result<(), Trap> {
12831283
let value = T::local_get(&self.store.value_stack, &self.cf, u16::from(value_local));
12841284
let mem_addr = self.mem_addr(memarg.mem_addr());
1285-
let mem = self.store.state.get_mem(mem_addr);
1285+
let mem = self.store.state.get_mem_mut(mem_addr);
12861286
let addr = if mem.is_64bit() {
12871287
let base = u64::local_get(&self.store.value_stack, &self.cf, u16::from(addr_local));
12881288
let base = cold_err!(usize::try_from(base).map_err(|_| Trap::MemoryOutOfBounds {
@@ -1295,7 +1295,6 @@ impl<'store, const BUDGETED: bool> Executor<'store, BUDGETED> {
12951295
let base = u32::local_get(&self.store.value_stack, &self.cf, u16::from(addr_local));
12961296
mem.effective_addr::<N>(base as usize, memarg.offset())?
12971297
};
1298-
let mem = self.store.state.get_mem_mut(mem_addr);
12991298
value.store_at(&mut mem.inner, addr)
13001299
}
13011300

@@ -1307,7 +1306,7 @@ impl<'store, const BUDGETED: bool> Executor<'store, BUDGETED> {
13071306
increment: impl FnOnce(T) -> T,
13081307
) -> Result<(), Trap> {
13091308
let mem_addr = self.mem_addr(memarg.mem_addr());
1310-
let mem = self.store.state.get_mem(mem_addr);
1309+
let mem = self.store.state.get_mem_mut(mem_addr);
13111310
let addr = if mem.is_64bit() {
13121311
let base = i64::local_get(&self.store.value_stack, &self.cf, u16::from(addr_local)) as u64;
13131312
let base = cold_err!(usize::try_from(base).map_err(|_| Trap::MemoryOutOfBounds {
@@ -1321,7 +1320,6 @@ impl<'store, const BUDGETED: bool> Executor<'store, BUDGETED> {
13211320
mem.effective_addr::<N>(base as usize, memarg.offset())?
13221321
};
13231322

1324-
let mem = self.store.state.get_mem_mut(mem_addr);
13251323
let value = cold_err!(T::load_at(&mem.inner, addr))?;
13261324
increment(value).store_at(&mut mem.inner, addr)
13271325
}
@@ -1339,10 +1337,9 @@ impl<'store, const BUDGETED: bool> Executor<'store, BUDGETED> {
13391337
let acc = T::stack_pop(&mut self.store.value_stack);
13401338
let fma = acc + lhs * rhs;
13411339
let mem_addr = self.mem_addr(m.mem_addr());
1342-
let mem = self.store.state.get_mem(mem_addr);
1340+
let mem = self.store.state.get_mem_mut(mem_addr);
13431341
let base = self.store.value_stack.pop_memory_operand(mem.kind.arch())?;
13441342
let addr = mem.effective_addr::<N>(base, m.offset())?;
1345-
let mem = self.store.state.get_mem_mut(mem_addr);
13461343
fma.store_at(&mut mem.inner, addr)?;
13471344
Ok(())
13481345
}
@@ -1651,14 +1648,14 @@ impl<'store, const BUDGETED: bool> Executor<'store, BUDGETED> {
16511648

16521649
fn exec_memory_grow(&mut self, addr: u32) -> Result<(), Trap> {
16531650
let mem_addr = self.mem_addr(addr);
1654-
let is_64bit = self.store.state.get_mem(mem_addr).is_64bit();
1651+
let limiter = self.store.engine.config().resource_limiter.as_deref();
1652+
let mem = self.store.state.get_mem_mut(mem_addr);
1653+
let is_64bit = mem.is_64bit();
16551654
let pages_delta = match is_64bit {
16561655
true => <i64>::stack_pop(&mut self.store.value_stack),
16571656
false => i64::from(<i32>::stack_pop(&mut self.store.value_stack)),
16581657
};
1659-
let limiter = self.store.engine.config().resource_limiter.as_deref();
16601658

1661-
let mem = self.store.state.get_mem_mut(mem_addr);
16621659
let size = mem.grow(pages_delta, limiter)?.unwrap_or(-1);
16631660
match is_64bit {
16641661
true => self.store.value_stack.push::<i64>(size)?,
@@ -1811,10 +1808,9 @@ impl<'store, const BUDGETED: bool> Executor<'store, BUDGETED> {
18111808
let val = U::from_mem_bytes(val_bytes);
18121809
let m = arg.memory_arg_idx.get(&self.func.data);
18131810
let mem_addr = self.mem_addr(m.mem_addr());
1814-
let mem = self.store.state.get_mem(mem_addr);
1811+
let mem = self.store.state.get_mem_mut(mem_addr);
18151812
let base = self.store.value_stack.pop_memory_operand(mem.kind.arch())?;
18161813
let addr = mem.effective_addr::<N>(base, m.offset())?;
1817-
let mem = self.store.state.get_mem_mut(mem_addr);
18181814
cold_err!(val.store_at(&mut mem.inner, addr))?;
18191815
Ok(())
18201816
}
@@ -1829,10 +1825,9 @@ impl<'store, const BUDGETED: bool> Executor<'store, BUDGETED> {
18291825

18301826
let m = index.get(&self.func.data);
18311827
let mem_addr = self.mem_addr(m.mem_addr());
1832-
let mem = self.store.state.get_mem(mem_addr);
1828+
let mem = self.store.state.get_mem_mut(mem_addr);
18331829
let base = self.store.value_stack.pop_memory_operand(mem.kind.arch())?;
18341830
let addr = mem.effective_addr::<N>(base, m.offset())?;
1835-
let mem = self.store.state.get_mem_mut(mem_addr);
18361831
cold_err!(val.store_at(&mut mem.inner, addr))?;
18371832
Ok(())
18381833
}

‎crates/tinywasm/src/interpreter/simd/mod.rs‎

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -15,24 +15,6 @@ use core::arch::wasm64 as wasm;
1515
/// A 128-bit SIMD value
1616
pub struct Value128(pub(crate) [u8; 16]);
1717

18-
impl From<[u8; 16]> for Value128 {
19-
fn from(bytes: [u8; 16]) -> Self {
20-
Self(bytes)
21-
}
22-
}
23-
24-
impl Value128 {
25-
#[inline(always)]
26-
pub const fn from_le_bytes(bytes: [u8; 16]) -> Self {
27-
Self(bytes)
28-
}
29-
30-
#[inline(always)]
31-
pub const fn to_le_bytes(self) -> [u8; 16] {
32-
self.0
33-
}
34-
}
35-
3618
impl From<Value128> for i128 {
3719
fn from(val: Value128) -> Self {
3820
i128::from_le_bytes(val.0)

‎crates/tinywasm/src/interpreter/stack/value_stack.rs‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -153,9 +153,9 @@ impl<T: Copy + Default> Stack<T> {
153153
return;
154154
}
155155

156-
let keep = (len - n).min(end_keep);
157-
self.data.copy_within(len - keep..len, n);
158-
self.data.truncate(n + keep);
156+
let keep = len.wrapping_sub(n).min(end_keep);
157+
self.data.copy_within(len.wrapping_sub(keep)..len, n);
158+
self.data.truncate(n.wrapping_add(keep));
159159
}
160160

161161
#[inline(always)]
@@ -231,12 +231,12 @@ impl<T: Copy + Default> Stack<T> {
231231
}
232232

233233
if !condition {
234-
let dst = len - needed;
235-
let src = len - count;
234+
let dst = len.wrapping_sub(needed);
235+
let src = len.wrapping_sub(count);
236236
self.data.copy_within(src..len, dst);
237237
}
238238

239-
self.data.truncate(len - count);
239+
self.data.truncate(len.wrapping_sub(count));
240240
}
241241
}
242242

@@ -400,7 +400,7 @@ impl ValueStack {
400400
WasmValue::F32(v) => self.stack_32.push(v.to_bits())?,
401401
WasmValue::F64(v) => self.stack_64.push(v.to_bits())?,
402402
WasmValue::Ref(v) => self.stack_32.push(ValueRef::from(v).raw())?,
403-
WasmValue::V128(v) => self.stack_128.push(v.into())?,
403+
WasmValue::V128(v) => self.stack_128.push(Value128(v))?,
404404
}
405405
}
406406
Ok(())

‎crates/tinywasm/src/interpreter/values.rs‎

Lines changed: 30 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -6,19 +6,13 @@ use tinywasm_types::{GlobalAddr, LocalAddr, RefValue, WasmValue};
66
pub(crate) type Value32 = u32;
77
pub(crate) type Value64 = u64;
88

9-
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
9+
#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)]
1010
/// Packed internal representation of a WebAssembly reference.
1111
///
1212
/// Unlike the public [`RefValue`], this stores no explicit reference category.
1313
/// Converting it back therefore requires the value's canonical reference type.
1414
pub(crate) struct ValueRef(u32);
1515

16-
impl Default for ValueRef {
17-
fn default() -> Self {
18-
Self::NULL
19-
}
20-
}
21-
2216
impl ValueRef {
2317
const HOST_ANY_TAG: u32 = 1 << 30;
2418
pub(crate) const NULL: Self = Self(0);
@@ -100,31 +94,19 @@ pub(crate) enum TinyWasmValue {
10094
ValueRef(ValueRef),
10195
}
10296

103-
impl From<&WasmValue> for TinyWasmValue {
104-
fn from(value: &WasmValue) -> Self {
97+
impl From<WasmValue> for TinyWasmValue {
98+
fn from(value: WasmValue) -> Self {
10599
match value {
106-
WasmValue::I32(v) => Self::Value32(*v as u32),
107-
WasmValue::I64(v) => Self::Value64(*v as u64),
100+
WasmValue::I32(v) => Self::Value32(v as u32),
101+
WasmValue::I64(v) => Self::Value64(v as u64),
108102
WasmValue::F32(v) => Self::Value32(v.to_bits()),
109103
WasmValue::F64(v) => Self::Value64(v.to_bits()),
110-
WasmValue::Ref(value) => Self::ValueRef((*value).into()),
111-
WasmValue::V128(v) => Self::Value128((*v).into()),
104+
WasmValue::Ref(value) => Self::ValueRef(value.into()),
105+
WasmValue::V128(v) => Self::Value128(Value128(v)),
112106
}
113107
}
114108
}
115109

116-
impl From<WasmValue> for TinyWasmValue {
117-
fn from(value: WasmValue) -> Self {
118-
Self::from(&value)
119-
}
120-
}
121-
122-
impl From<[u8; 16]> for TinyWasmValue {
123-
fn from(value: [u8; 16]) -> Self {
124-
Self::Value128(Value128::from(value))
125-
}
126-
}
127-
128110
mod sealed {
129111
#[expect(unreachable_pub)]
130112
pub trait Sealed {}
@@ -148,23 +130,14 @@ pub(crate) trait InternalValue: sealed::Sealed + Copy + Default {
148130
macro_rules! impl_internalvalue {
149131
(
150132
$(
151-
$variant:ident, $stack:ident, $stack_base:ident, $global_get:ident, $global_set:ident, $outer:ty,
152-
|$to_value_v:ident| $to_value:expr,
133+
$stack:ident, $stack_base:ident, $global_get:ident, $global_set:ident, $outer:ty,
153134
|$to_stack_v:ident| $to_stack:expr,
154135
|$from_stack_v:ident| $from_stack:expr
155136
)*
156137
) => {
157138
$(
158139
impl sealed::Sealed for $outer {}
159140

160-
impl From<$outer> for TinyWasmValue {
161-
#[inline(always)]
162-
fn from(value: $outer) -> Self {
163-
let $to_value_v = value;
164-
TinyWasmValue::$variant($to_value)
165-
}
166-
}
167-
168141
impl InternalValue for $outer {
169142
#[inline(always)]
170143
fn stack_push(stack: &mut ValueStack, value: Self) -> Result<(), crate::Trap> {
@@ -175,34 +148,41 @@ macro_rules! impl_internalvalue {
175148

176149
#[inline(always)]
177150
fn local_get(stack: &ValueStack, frame: &CallFrame, index: LocalAddr) -> Self {
178-
let $from_stack_v = *stack.$stack.get(frame.locals_base.$stack_base as usize + index as usize);
151+
let $from_stack_v =
152+
*stack.$stack.get(frame.locals_base.$stack_base.wrapping_add(u32::from(index)) as usize);
179153
$from_stack
180154
}
181155

182156
#[inline(always)]
183157
fn local_push(stack: &mut ValueStack, frame: &CallFrame, index: LocalAddr) -> Result<(), crate::Trap> {
184-
stack.$stack.push_copy(frame.locals_base.$stack_base as usize + index as usize)
158+
stack.$stack.push_copy(frame.locals_base.$stack_base.wrapping_add(u32::from(index)) as usize)
185159
}
186160

187161
#[inline(always)]
188162
fn local_set(stack: &mut ValueStack, frame: &CallFrame, index: LocalAddr, value: Self) {
189163
let $to_stack_v = value;
190-
let abs_index = frame.locals_base.$stack_base as usize + index as usize;
164+
let abs_index = frame.locals_base.$stack_base.wrapping_add(u32::from(index)) as usize;
191165
stack.$stack.set(abs_index, $to_stack);
192166
}
193167

194168
#[inline(always)]
195-
fn local_update(stack: &mut ValueStack, frame: &CallFrame, index: LocalAddr, f: impl FnOnce(Self) -> Self) {
196-
let abs_index = frame.locals_base.$stack_base as usize + index as usize;
169+
fn local_update(
170+
stack: &mut ValueStack,
171+
frame: &CallFrame,
172+
index: LocalAddr,
173+
f: impl FnOnce(Self) -> Self,
174+
) {
175+
let abs_index = frame.locals_base.$stack_base.wrapping_add(u32::from(index)) as usize;
197176
let $from_stack_v = *stack.$stack.get(abs_index);
198177
let $to_stack_v = f($from_stack);
199178
stack.$stack.set(abs_index, $to_stack);
200179
}
201180

202181
#[inline(always)]
203182
fn local_copy(stack: &mut ValueStack, frame: &CallFrame, from: LocalAddr, to: LocalAddr) {
204-
let base = frame.locals_base.$stack_base as usize;
205-
stack.$stack.copy(base + from as usize, base + to as usize);
183+
let from = frame.locals_base.$stack_base.wrapping_add(u32::from(from)) as usize;
184+
let to = frame.locals_base.$stack_base.wrapping_add(u32::from(to)) as usize;
185+
stack.$stack.copy(from, to);
206186
}
207187

208188
#[inline(always)]
@@ -245,12 +225,12 @@ macro_rules! impl_internalvalue {
245225
}
246226

247227
impl_internalvalue! {
248-
Value32, stack_32, s32, get_32, set_32, u32, |v| v, |v| v, |v| v
249-
Value64, stack_64, s64, get_64, set_64, u64, |v| v, |v| v, |v| v
250-
Value32, stack_32, s32, get_32, set_32, i32, |v| v as u32, |v| v as u32, |v| v as i32
251-
Value64, stack_64, s64, get_64, set_64, i64, |v| v as u64, |v| v as u64, |v| v as i64
252-
Value32, stack_32, s32, get_32, set_32, f32, |v| f32::to_bits(v), |v| f32::to_bits(v), |v| f32::from_bits(v)
253-
Value64, stack_64, s64, get_64, set_64, f64, |v| f64::to_bits(v), |v| f64::to_bits(v), |v| f64::from_bits(v)
254-
ValueRef, stack_32, s32, get_32, set_32, ValueRef, |v| v, |v| v.raw(), |v| ValueRef(v)
255-
Value128, stack_128, s128, get_128, set_128, Value128, |v| v, |v| v, |v| v
228+
stack_32, s32, get_32, set_32, u32, |v| v, |v| v
229+
stack_64, s64, get_64, set_64, u64, |v| v, |v| v
230+
stack_32, s32, get_32, set_32, i32, |v| v as u32, |v| v as i32
231+
stack_64, s64, get_64, set_64, i64, |v| v as u64, |v| v as i64
232+
stack_32, s32, get_32, set_32, f32, |v| f32::to_bits(v), |v| f32::from_bits(v)
233+
stack_64, s64, get_64, set_64, f64, |v| f64::to_bits(v), |v| f64::from_bits(v)
234+
stack_32, s32, get_32, set_32, ValueRef, |v| v.raw(), |v| ValueRef(v)
235+
stack_128, s128, get_128, set_128, Value128, |v| v, |v| v
256236
}

‎crates/tinywasm/src/store/const_expr.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use alloc::{format, vec::Vec};
22
use tinywasm_types::*;
33

44
use super::{State, default_value};
5-
use crate::interpreter::{TinyWasmValue, ValueRef};
5+
use crate::interpreter::{TinyWasmValue, Value128, ValueRef};
66
use crate::{Error, Result, Trap};
77

88
fn resolve<T: Copy>(items: &[T], index: u32, kind: &str) -> Result<T> {
@@ -57,7 +57,7 @@ pub(super) fn eval_const(
5757
I64Const(value) => return Ok(TinyWasmValue::Value64(*value as u64)),
5858
F32Const(value) => return Ok(TinyWasmValue::Value32(value.to_bits())),
5959
F64Const(value) => return Ok(TinyWasmValue::Value64(value.to_bits())),
60-
V128Const(value) => return Ok(TinyWasmValue::Value128((*value).into())),
60+
V128Const(value) => return Ok(TinyWasmValue::Value128(Value128(*value))),
6161
GlobalGet32(index) => {
6262
return Ok(TinyWasmValue::Value32(state.globals.get_32(resolve(global_addrs, *index, "global")?)));
6363
}
@@ -87,7 +87,7 @@ pub(super) fn eval_const(
8787
I64Const(value) => stack.push(TinyWasmValue::Value64(*value as u64)),
8888
F32Const(value) => stack.push(TinyWasmValue::Value32(value.to_bits())),
8989
F64Const(value) => stack.push(TinyWasmValue::Value64(value.to_bits())),
90-
V128Const(value) => stack.push(TinyWasmValue::Value128((*value).into())),
90+
V128Const(value) => stack.push(TinyWasmValue::Value128(Value128(*value))),
9191
GlobalGet32(index) => {
9292
stack.push(TinyWasmValue::Value32(state.globals.get_32(resolve(global_addrs, *index, "global")?)));
9393
}

‎crates/tinywasm/src/store/gc/mod.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ pub(crate) fn default_value(storage: StorageType) -> TinyWasmValue {
1818
TinyWasmValue::Value32(0)
1919
}
2020
StorageType::Value(WasmType::I64 | WasmType::F64) => TinyWasmValue::Value64(0),
21-
StorageType::Value(WasmType::V128) => TinyWasmValue::Value128(Value128::from([0; 16])),
21+
StorageType::Value(WasmType::V128) => TinyWasmValue::Value128(Value128([0; 16])),
2222
StorageType::Value(WasmType::Ref(_)) => TinyWasmValue::ValueRef(ValueRef::NULL),
2323
}
2424
}
@@ -85,7 +85,7 @@ pub(crate) fn decode_data(
8585
2 => TinyWasmValue::Value32(u32::from(u16::from_le_bytes(bytes.try_into().unwrap()))),
8686
4 => TinyWasmValue::Value32(u32::from_le_bytes(bytes.try_into().unwrap())),
8787
8 => TinyWasmValue::Value64(u64::from_le_bytes(bytes.try_into().unwrap())),
88-
16 => TinyWasmValue::Value128(Value128::from(<[u8; 16]>::try_from(bytes).unwrap())),
88+
16 => TinyWasmValue::Value128(Value128(<[u8; 16]>::try_from(bytes).unwrap())),
8989
_ => unreachable!(),
9090
}));
9191
Ok(values)

‎crates/tinywasm/src/store/memory/instance.rs‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,8 @@ impl core::fmt::Debug for MemoryInstance {
2222

2323
impl MemoryInstance {
2424
#[inline]
25-
fn host_size(kind: MemoryType, pages: u64) -> Result<usize> {
26-
let size = pages
27-
.checked_mul(kind.page_size())
28-
.ok_or(Error::UnsupportedFeature("memory size exceeds the host address space"))?;
29-
usize::try_from(size).map_err(|_| Error::UnsupportedFeature("memory size exceeds the host address space"))
25+
fn host_size(kind: MemoryType, pages: u64) -> Option<usize> {
26+
pages.checked_mul(kind.page_size()).and_then(|size| usize::try_from(size).ok())
3027
}
3128

3229
#[inline]
@@ -58,7 +55,10 @@ impl MemoryInstance {
5855
}
5956

6057
pub(crate) fn new(kind: MemoryType, limiter: Option<&dyn ResourceLimiter>) -> Result<Self> {
61-
let initial_len = Self::host_size(kind, kind.page_count_initial())?;
58+
let initial_len = cold_err!(
59+
Self::host_size(kind, kind.page_count_initial())
60+
.ok_or(Error::UnsupportedFeature("memory size exceeds the host address space"))
61+
)?;
6262

6363
crate::log::debug!(
6464
"initializing memory with {} pages of {} bytes",
@@ -117,7 +117,7 @@ impl MemoryInstance {
117117
});
118118
}
119119

120-
let Ok(new_size) = Self::host_size(self.kind, new_pages as u64) else {
120+
let Some(new_size) = Self::host_size(self.kind, new_pages as u64) else {
121121
return cold!(Ok(None));
122122
};
123123
if new_size == self.inner.len() {
@@ -130,8 +130,8 @@ impl MemoryInstance {
130130
return cold!(Ok(None));
131131
}
132132

133-
if cold_err!(self.inner.grow_to(new_size)).is_err() {
134-
return Ok(None);
133+
if self.inner.grow_to(new_size).is_err() {
134+
return cold!(Ok(None));
135135
}
136136
self.page_count = new_pages;
137137
Ok(i64::try_from(current_pages).ok())

0 commit comments

Comments
 (0)