Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for v128 to the typed function API #7010

Merged
merged 5 commits into from
Sep 14, 2023

Conversation

alexcrichton
Copy link
Member

This commit adds a Rust type V128 which corresponds to the wasm v128 type. This is intended to perhaps one day have accessors for lanes of various sizes but in the meantime only supports conversion back and forth between u128. The intention of this type is to allow platforms to perform typed between to functions that take or return v128 wasm values.

Previously this was not implemented because it's a bit tricky ABI-wise. Typed functions work by passing arguments in registers which requires the calling convention to match in both Cranelift and in Rust. This should be the case for supported platforms and the default calling convention, especially now that the wasm calling convention is separate from the platform calling convention. This does mean, however, that this feature can only be supported on x86_64 and AArch64. Currently neither s390x nor RISC-V have a means of supporting the vector calling convention since the vector types aren't available on stable in Rust itself. This means that it's now unfortunately possible to write a Wasmtime embedding that compiles on x86_64 that doesn't compile on s390x for example, but given how niche this feature is that seems like an ok tradeoff for now and by the time it's a problem Rust might have native stable support for vector types on these platforms.

prtest:full

This commit adds a Rust type `V128` which corresponds to the wasm
`v128` type. This is intended to perhaps one day have accessors for
lanes of various sizes but in the meantime only supports conversion back
and forth between `u128`. The intention of this type is to allow
platforms to perform typed between to functions that take or return
`v128` wasm values.

Previously this was not implemented because it's a bit tricky ABI-wise.
Typed functions work by passing arguments in registers which requires
the calling convention to match in both Cranelift and in Rust. This
should be the case for supported platforms and the default calling
convention, especially now that the wasm calling convention is separate
from the platform calling convention. This does mean, however, that this
feature can only be supported on x86_64 and AArch64. Currently neither
s390x nor RISC-V have a means of supporting the vector calling
convention since the vector types aren't available on stable in Rust
itself. This means that it's now unfortunately possible to write a
Wasmtime embedding that compiles on x86_64 that doesn't compile on s390x
for example, but given how niche this feature is that seems like an ok
tradeoff for now and by the time it's a problem Rust might have native
stable support for vector types on these platforms.

prtest:full
@alexcrichton alexcrichton requested a review from a team as a code owner September 12, 2023 21:04
@alexcrichton alexcrichton requested review from fitzgen and removed request for a team September 12, 2023 21:04
@github-actions github-actions bot added fuzzing Issues related to our fuzzing infrastructure wasmtime:api Related to the API of the `wasmtime` crate itself wasmtime:c-api Issues pertaining to the C API. labels Sep 12, 2023
@github-actions
Copy link

Subscribe to Label Action

cc @fitzgen, @peterhuene

This issue or pull request has been labeled: "fuzzing", "wasmtime:api", "wasmtime:c-api"

Thus the following users have been cc'd because of the following labels:

  • fitzgen: fuzzing
  • peterhuene: wasmtime:api, wasmtime:c-api

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

crates/wasmtime/src/v128.rs Outdated Show resolved Hide resolved
Comment on lines +54 to +57
union Reinterpret {
abi: Abi,
u128: u128,
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat trick

tests/all/func.rs Outdated Show resolved Hide resolved
@alexcrichton
Copy link
Member Author

I should also mention before I forget, it is theoretically possible to support v128 on all platforms. We are in control of our own ABI and we could say that the v128 type always maps to memory or a pair of u64 values. This would require rejiggering not only the Cranelift trampolines generated but additionally the WasmTy trait and how arguments are passed in the ABI. I figured that was a big lift for now where it'd be "much easier" to wait for Rust to add stable support for vector types on s390x and RISC-V in theory (in the assumption that users desiring that will coincide with a much later date in the future)

@alexcrichton alexcrichton added this pull request to the merge queue Sep 14, 2023
Merged via the queue into bytecodealliance:main with commit 8d7a2b8 Sep 14, 2023
32 checks passed
@alexcrichton alexcrichton deleted the typed-v128 branch September 14, 2023 16:44
eduardomourar pushed a commit to eduardomourar/wasmtime that referenced this pull request Sep 15, 2023
* Add support for `v128` to the typed function API

This commit adds a Rust type `V128` which corresponds to the wasm
`v128` type. This is intended to perhaps one day have accessors for
lanes of various sizes but in the meantime only supports conversion back
and forth between `u128`. The intention of this type is to allow
platforms to perform typed between to functions that take or return
`v128` wasm values.

Previously this was not implemented because it's a bit tricky ABI-wise.
Typed functions work by passing arguments in registers which requires
the calling convention to match in both Cranelift and in Rust. This
should be the case for supported platforms and the default calling
convention, especially now that the wasm calling convention is separate
from the platform calling convention. This does mean, however, that this
feature can only be supported on x86_64 and AArch64. Currently neither
s390x nor RISC-V have a means of supporting the vector calling
convention since the vector types aren't available on stable in Rust
itself. This means that it's now unfortunately possible to write a
Wasmtime embedding that compiles on x86_64 that doesn't compile on s390x
for example, but given how niche this feature is that seems like an ok
tradeoff for now and by the time it's a problem Rust might have native
stable support for vector types on these platforms.

prtest:full

* Fix compile of C API

* Conditionally enable typed v128 tests

* Review comments

* Fix compiler warnings
eduardomourar pushed a commit to eduardomourar/wasmtime that referenced this pull request Sep 22, 2023
* Add support for `v128` to the typed function API

This commit adds a Rust type `V128` which corresponds to the wasm
`v128` type. This is intended to perhaps one day have accessors for
lanes of various sizes but in the meantime only supports conversion back
and forth between `u128`. The intention of this type is to allow
platforms to perform typed between to functions that take or return
`v128` wasm values.

Previously this was not implemented because it's a bit tricky ABI-wise.
Typed functions work by passing arguments in registers which requires
the calling convention to match in both Cranelift and in Rust. This
should be the case for supported platforms and the default calling
convention, especially now that the wasm calling convention is separate
from the platform calling convention. This does mean, however, that this
feature can only be supported on x86_64 and AArch64. Currently neither
s390x nor RISC-V have a means of supporting the vector calling
convention since the vector types aren't available on stable in Rust
itself. This means that it's now unfortunately possible to write a
Wasmtime embedding that compiles on x86_64 that doesn't compile on s390x
for example, but given how niche this feature is that seems like an ok
tradeoff for now and by the time it's a problem Rust might have native
stable support for vector types on these platforms.

prtest:full

* Fix compile of C API

* Conditionally enable typed v128 tests

* Review comments

* Fix compiler warnings
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fuzzing Issues related to our fuzzing infrastructure wasmtime:api Related to the API of the `wasmtime` crate itself wasmtime:c-api Issues pertaining to the C API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants