wiggle: add initial support for shared memory#5225
Merged
Conversation
This change is the first in a series of changes to support shared memory in Wiggle. Since Wiggle was written under the assumption of single-threaded guest-side access, this change introduces a `shared` field to guest memories in order to flag when this assumption will not be the case. This change always sets `shared` to `false`; once a few more pieces are in place, `shared` will be set dynamically when a shared memory is detected, e.g., in a change like bytecodealliance#5054. Using the `shared` field, we can now decide to load Wiggle values differently under the new assumptions. This change makes the guest `T::read` and `T::write` calls into `Relaxed` atomic loads and stores in order to maintain WebAssembly's expected memory consistency guarantees. We choose Rust's `Relaxed` here to match the `Unordered` memory consistency described in the [memory model] section of the ECMA spec. [memory model]: https://tc39.es/ecma262/multipage/memory-model.html#sec-memory-model Since 128-bit scalar types do not have `Atomic*` equivalents, we remove their `T::read` and `T::write` implementations here. They are unused by any WASI implementations in the project.
alexcrichton
reviewed
Nov 8, 2022
Member
alexcrichton
left a comment
alexcrichton
left a comment
There was a problem hiding this comment.
One interesting option here is to actually always use atomic loads since Relaxed shouldn't be all that different from non-atomic in this context perf-wise. It might even be a tiny bit faster with the lack of a branch perhaps?
Subscribe to Label Actioncc @kubkon DetailsThis issue or pull request has been labeled: "wasi"Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
Values must be little-endian for WebAssembly's sake; this adds conversions to and from the host's native endianness. Additionally, this splits out the macro into `integer_primitives!` and `float_primitives!` versions.
Member
Author
|
cc: @pchickey |
alexcrichton
approved these changes
Nov 8, 2022
15 tasks
abrown
added a commit
to abrown/wasmtime
that referenced
this pull request
Nov 14, 2022
`wiggle` looks for an exported `Memory` named `"memory"` to use for its guest slices. This change allows it to use a `SharedMemory` if this is the kind of memory used for the export. It is `unsafe` to use shared memory in Wiggle because of broken Rust guarantees: previously, Wiggle could hand out slices to WebAssembly linear memory that could be concurrently modified by some other thread. With the introduction of Wiggle's new `UnsafeGuestSlice` (bytecodealliance#5225, bytecodealliance#5229, bytecodealliance#5264), Wiggle should now correctly communicate its guarantees through its API.
abrown
added a commit
to abrown/wasmtime
that referenced
this pull request
Nov 15, 2022
`wiggle` looks for an exported `Memory` named `"memory"` to use for its guest slices. This change allows it to use a `SharedMemory` if this is the kind of memory used for the export. It is `unsafe` to use shared memory in Wiggle because of broken Rust guarantees: previously, Wiggle could hand out slices to WebAssembly linear memory that could be concurrently modified by some other thread. With the introduction of Wiggle's new `UnsafeGuestSlice` (bytecodealliance#5225, bytecodealliance#5229, bytecodealliance#5264), Wiggle should now correctly communicate its guarantees through its API.
alexcrichton
pushed a commit
that referenced
this pull request
Nov 15, 2022
`wiggle` looks for an exported `Memory` named `"memory"` to use for its guest slices. This change allows it to use a `SharedMemory` if this is the kind of memory used for the export. It is `unsafe` to use shared memory in Wiggle because of broken Rust guarantees: previously, Wiggle could hand out slices to WebAssembly linear memory that could be concurrently modified by some other thread. With the introduction of Wiggle's new `UnsafeGuestSlice` (#5225, #5229, #5264), Wiggle should now correctly communicate its guarantees through its API.
abrown
added
the
wasm-proposal:threads
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This change is the first in a series of changes to support shared memory in Wiggle. Since Wiggle was written under the assumption of single-threaded guest-side access, this change introduces a
sharedfield to guest memories in order to flag when this assumption will not be the case. This change always setssharedtofalse; once a few more pieces are in place,sharedwill be set dynamically when a shared memory is detected, e.g., in a change like #5054.Using the
sharedfield, we can now decide to load Wiggle values differently under the new assumptions. This change makes the guestT::readandT::writecalls intoRelaxedatomic loads and stores in order to maintain WebAssembly's expected memory consistency guarantees. We choose Rust'sRelaxedhere to match theUnorderedmemory consistency described in the memory model section of the ECMA spec.Since 128-bit scalar types do not have
Atomic*equivalents, we remove theirT::readandT::writeimplementations here. They are unused by any WASI implementations in the project.