lann opened issue #5480:
Currently the
wasmtime::component::Val::Float32
and::Float64
variants wrapu32
andu64
values:I suspect this is just an oversight as typed functions do use
f32
/f64
. I'm happy to make this update if it makes sense.
dicej commented on issue #5480:
For the record, this floats-as-bits representation was copied from the equivalent API for modules: https://github.com/bytecodealliance/wasmtime/blob/b47e644c3dcd857221ef7163e780a0399a183966/crates/wasmtime/src/values.rs#L22-L32, which was added in this commit: https://github.com/bytecodealliance/wasmtime/commit/f88e92a57ce9654917193c88a55cb3de863653ee. I imagine that if we're going to change this API, we should also change the module API as well.
jameysharp commented on issue #5480:
We do this in Cranelift too. I don't know if it's for the same reason that Wasmtime's API works this way, but Cranelift's reason was discussed in #2251. In short, Rust doesn't make any guarantees about the bit-representation of NaN values being preserved in floating-point types. So if you care about that, you need to keep the raw bits in an integer type until the time when you want to do actual floating-point operations on them.
lann commented on issue #5480:
@jameysharp Thanks for that context. The component model requires canonicalization of NaNs so it seems like it makes sense to diverge from the copied API here.
dicej edited a comment on issue #5480:
For the record, this floats-as-bits representation was copied from the equivalent API for modules: https://github.com/bytecodealliance/wasmtime/blob/b47e644c3dcd857221ef7163e780a0399a183966/crates/wasmtime/src/values.rs#L22-L32, which was added in this commit: https://github.com/bytecodealliance/wasmtime/commit/f88e92a57ce9654917193c88a55cb3de863653ee.
I imagine that if we're going to change this API, we should also change the module API as well.Edit: As Lann pointed out, since the component model requires NaNs to be canonicalized, it may make sense to use a different representation for component values vs. core Wasm values.
alexcrichton commented on issue #5480:
I agree that while this matches the core wasm API it can be different for hte ocmponent model due to all NaN's being canonicalized. @lann would you be up for PR-ing this change?
alexcrichton labeled issue #5480:
Currently the
wasmtime::component::Val::Float32
and::Float64
variants wrapu32
andu64
values:I suspect this is just an oversight as typed functions do use
f32
/f64
. I'm happy to make this update if it makes sense.
lann commented on issue #5480:
Sure, I can take it.
alexcrichton closed issue #5480:
Currently the
wasmtime::component::Val::Float32
and::Float64
variants wrapu32
andu64
values:I suspect this is just an oversight as typed functions do use
f32
/f64
. I'm happy to make this update if it makes sense.
Last updated: Dec 23 2024 at 12:05 UTC