Stream: git-wasmtime

Topic: wasmtime / PR #4035 Store the `ValRaw` type in little-end...

Wasmtime GitHub notifications bot (Apr 14 2022 at 15:35):

alexcrichton opened PR #4035 from val-raw-little-endian to main:

This commit changes the internal representation of the ValRaw type to
an unconditionally little-endian format instead of its current
native-endian format. The documentation and various accessors here have
been updated as well as the associated trampolines that read ValRaw
to always work with little-endian values, converting to the host
endianness as necessary.

The motivation for this change originally comes from the implementation
of the component model that I'm working on. One aspect of the component
model's canonical ABI is how variants are passed to functions as
immediate arguments. For example for a component model function:

foo: function(x: expected<i32, f64>)

This translates to a core wasm function:

(module
  (func (export "foo") (param i32 i64)
    ;; ...
  )
)

The first i32 parameter to the core wasm function is the discriminant
of whether the result is an "ok" or an "err". The second i64, however,
is the "join" operation on the i32 and f64 payloads. Essentially
these two types are unioned into one type to get passed into the function.

Currently in the implementation of the component model my plan is to
construct a *mut [ValRaw] to pass through to WebAssembly, always
invoking component exports through host trampolines. This means that the
implementation for Result<T, E> needs to do the correct "join"
operation here when encoding a particular case into the corresponding
ValRaw.

I personally found this particularly tricky to do structurally. The
solution that I settled on with fitzgen was that if ValRaw was always
stored in a little endian format then we could employ a trick where when
encoding a variant we first set all the ValRaw slots to zero, then the
associated case we have is encoding. Afterwards the ValRaw values are
already encoded into the correct format as if they'd been "join"ed.

For example if we were to encode Ok(1i32) then this would produce
ValRaw { i32: 1 }, which memory-wise is equivalent to ValRaw { i64: 1 }
if the other bytes in the ValRaw are guaranteed to be zero. Similarly
storing ValRaw { f64 } is equivalent to the storage required for
ValRaw { i64 } here in the join operation.

Note, though, that this equivalence relies on everything being
little-endian. Otherwise the in-memory representations of ValRaw { i32: 1 }
and ValRaw { i64: 1 } are different.

That motivation is what leads to this change. It's expected that this is
a low-to-zero cost change in the sense that little-endian platforms will
see no change and big-endian platforms are already required to
efficiently byte-swap loads/stores as WebAssembly requires that.
Additionally the ValRaw type is an esoteric niche use case primarily
used for accelerating the C API right now, so it's expected that not
many users will have to update for this change.

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

• [ ] This has been discussed in issue #..., or if not, please tell us why
  here.
• [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.
• [ ] This PR contains test cases, if meaningful.
• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so. The list of
  suggested reviewers on the right can help you.

Please ensure all communication adheres to the code of conduct.
-->

Wasmtime GitHub notifications bot (Apr 14 2022 at 16:38):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Apr 14 2022 at 16:52):

alexcrichton updated PR #4035 from val-raw-little-endian to main.

Wasmtime GitHub notifications bot (Apr 14 2022 at 17:15):

bjorn3 submitted PR review.

Wasmtime GitHub notifications bot (Apr 14 2022 at 17:15):

bjorn3 created PR review comment:

Maybe store char[4], char[8], ... instead? Otherwise people will likely forget to do the endianness transform. Especially as all common architectures are little endian too.

Wasmtime GitHub notifications bot (Apr 14 2022 at 17:15):

bjorn3 submitted PR review.

Wasmtime GitHub notifications bot (Apr 14 2022 at 17:15):

bjorn3 created PR review comment:

*ValRaw

Wasmtime GitHub notifications bot (Apr 14 2022 at 18:08):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Apr 14 2022 at 18:08):

alexcrichton created PR review comment:

I would prefer to leave this in to avoid the need for funky casts and such in C. This is not intended to be super heavily used either and other language bindings will have to deal with this anyway.

Wasmtime GitHub notifications bot (Apr 14 2022 at 18:09):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Apr 14 2022 at 18:09):

alexcrichton created PR review comment:

The comment here is actually intended to point to Val in that ValRaw is the unsafe way of working with what is otherwise a wasm Val value.

Wasmtime GitHub notifications bot (Apr 14 2022 at 18:09):

alexcrichton merged PR #4035.

Wasmtime GitHub notifications bot (Apr 15 2022 at 08:31):

bjorn3 submitted PR review.

Wasmtime GitHub notifications bot (Apr 15 2022 at 08:31):

bjorn3 created PR review comment:

You need funky casts either way to support big endian systems.

Last updated: Dec 23 2024 at 12:05 UTC