Wasmtime GitHub notifications bot (Dec 20 2022 at 15:46): Wasmtime GitHub notifications bot (Dec 20 2022 at 15:46):lann opened issue #5480:
Currently the
wasmtime::component::Val::Float32and::Float64variants wrapu32andu64values: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.
Wasmtime GitHub notifications bot (Dec 20 2022 at 15:50):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.
Wasmtime GitHub notifications bot (Dec 20 2022 at 16:53):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.
Wasmtime GitHub notifications bot (Dec 20 2022 at 17:06):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.
Wasmtime GitHub notifications bot (Dec 20 2022 at 17:22):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.
Wasmtime GitHub notifications bot (Jan 03 2023 at 15:34):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?
Wasmtime GitHub notifications bot (Jan 03 2023 at 15:34):alexcrichton labeled issue #5480:
Currently the
wasmtime::component::Val::Float32and::Float64variants wrapu32andu64values: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.
Wasmtime GitHub notifications bot (Jan 03 2023 at 15:54):lann commented on issue #5480:
Sure, I can take it.
Wasmtime GitHub notifications bot (Jan 03 2023 at 20:23):alexcrichton closed issue #5480:
Currently the
wasmtime::component::Val::Float32and::Float64variants wrapu32andu64values: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: Oct 23 2024 at 20:03 UTC