-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Align with the "C" ABI #3454
Comments
One possible solution I've thought may work in the past is that there could be something like: pub trait IntoWasmAbi: WasmDescribe {
type Abi1: WasmAbi;
type Abi2: WasmAbi;
fn into_abi(self) -> (Self::Abi1, Self::Abi2);
} where most impls have |
Is this fixed with #3595? Or are there additional steps needed? |
See OP. |
@Liamolucko did #3595 check off the first part of the plan mentioned in this issue? If so, can you check it off in the issue as well |
Yes. |
I'm not an expert here, please excuse me for basic questions. The next step is written as "Move the wasm32-unknown-unknown target to the C ABI.". Does this mean that new C ABIs produced by |
This is a change that has to be done in rustc, see rust-lang/rust#71871. The output compiling for
My understanding is that this should actually be a fairly simple PR. |
If I, as someone who has never worked on rustc, were to take up on this, where would I even start? |
I left a comment at Rust repo to match the C ABI between |
There's a new PR at Rust that matches C ABI of It would be awesome if So currently, Rust's After talking with @daxpedda and @bjorn3, the best strategy seems to be accepting breaking changes and notifying the users in a proper way. I suggest that In this scenario, the new Rust(Possibly 1.75) can warn users using As a result, this strategy will solve two of the steps mentioned in OP at once.
This will also pave the way for a PR by @Rob2309 so that it can be finally merged. I hope we can reach a certain consensus first, because supporting |
I think I need to clarify that as of #3595, wasm-bindgen already supports the standard C ABI that
I don't think a breaking release is needed; that's only necessary if existing code might stop working after upgrading, which isn't the case since wasm-bindgen is still compatible with the old ABI. |
Thanks for the reply. Does that mean we can compile Rust for |
The main thing stopping you from doing that right now is that #3233 recently changed WASI to behave like a native target, making it so that trying to call imports from JS panics. That PR had a valid reason for doing so, so you'd need to make it configurable whether WASI is treated as a native or JS target. It might also be possible to do some shenanigans where wasm-bindgen generated imports panic by default, unless you run the CLI which replaces them with actual imports; that would mean pulling in the whole panic runtime though, which is a no-go unless
All of the ABI-related stuff is now redundant, but the WASI shims could still be useful. |
Hey all, is there a specific issue or PR matching the second item in the checklist, "Move the Would rust-lang/rust#117919 be a match? If so, could you update the description to include the link in the checkbox? I'm not sure if the box should be checked, though. Maybe wait for the correct ABI to hit stable and become the default without a flag? Also, the way they're rolling this out would probably match the third box, "Figure out a migration path for users": introduce a flag first, then add a lint warning for old versions of |
The situation has turned out to be much simpler. At some point, Rust will switch the rust-lang/rust#117919 introduced an unstable compiler flag, |
If you arrived here while searching for how to build a Rust + C WASM binary for the |
LLVM is about to enable |
Is it LLVM which will enable it by default or clang? Rustc doesn't enable any wasm features by default on any of the wasm targets (except for the features required for threads in case of the wasm32-wasip1-threads target), so I rustc should probably disable Edit: If you were referring to llvm/llvm-project#80923, then it wouldn't change the C abi, so disregard my comment other than the part about that rustc should probably disable the feature by default. |
Yeah I probably should've clarified that it does not change the ABI, however in at least #3552 it was necessary to use the standard C ABI for it to behave. For clang it already is enabled by default and thus probably emscripten as well. I'm not sure there've been any releases with it being enabled yet though. |
Last time I checked it does, see for example rust-lang/rust#109807.
We will have to see if this breaks ABI or not, I also guess it won't. However the reference type proposal will also be enabled by default, which is something we should probably disable in Rustc. |
I'm not sure whether or not Rust's non-standard C ABI will be affected, but I think the solution I mentioned in #3552 is still applicable if it is:
|
Motivation
We are currently using a non-standard ABI (there isn't really a standardized one) for Wasm exports, which currently Rust is mimicking: rust-lang/rust#71871.
This has the big disadvantage of making the
wasm32-unknown-unknown
backend ABI incompatible withwasm32-wasi
andwasm32-unknown-emscripten
. Which in turn makeswasm-bindgen
not compatible with any C/C++ dependency: #2317.Plan
wasm-bindgen
follow the C ABI outlined here: https://github.com/WebAssembly/tool-conventions/blob/main/BasicCABI.md. Which mainly involves removing splatting.wasm-c-abi
flag rust-lang/rust#117919:-Zwasm-c-abi
)wasm_c_abi
future-incompat
lint rust-lang/rust#117918)wasm32-unknown-unknown
target to the C ABI.(optional) Introduce a flag towasm-bindgen
that uses the unstable"wasm"
ABI: Tracking issue for the unstable "wasm" ABI rust-lang/rust#83788.Result
It should be possible to compile Rust with
wasm32-unknown-unknown
while consuming Emscripten and WASI dependencies correctly, which would also partially address #3421. I know nothing about Emscripten, but WASI would require some additional hook ups to support it's API, which we should probably (help) support as well.Downsides
There is a reason why the ABI differs, splatting is a performance improvement and the C ABI has no support for multi-value. Hopefully this can be addressed in the future and a better ABI can be agreed on. In the meantime users can use the
"wasm"
ABI on nightly to opt into splatting without us having to break C compatibility for everybody else.The text was updated successfully, but these errors were encountered: