-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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 wasm exception handling to Emscripten target #131830
base: master
Are you sure you want to change the base?
Conversation
rustbot has assigned @compiler-errors. Use |
The job Click to see the possible cause of the failure (guessed by this bot)
|
Hm. I feel a little out-of-context here. Could you explain
Is "new" meaning the wasm exception-handling proposal with |
Thanks for the questions. The short answer is these are two different ABIs. Perhaps it would be most accurate to think of
Well the older system has existed for a lot longer and worked with the first version of WebAssembly deployed in browsers. The new exception handling uses native support and has only been universally available in engines since ~March 2022.
We need different IR, different object files, and different linker arguments. But it's transparent to Rust/C++/C code.
Yeah so we need a distinct build of the standard library because the object files are different. The compilation of both
Currently, Pyodide is stuck using the old ABI because Rust only supports the old ABI. If Rust switched to only supporting the new ABI, it would make the transition difficult. |
To make matters worse, the new exception handling support was changed in October of 2023 so there are now two new exception handling variants:
I think there is no difference in the IR the frontend generates between the old new and new new variants, though the lowering pass has distinct code for the two cases, the object file format is different, and so the new variant will require a different setting code generation time. It's a fairly simple transform to adjust an old object file to support the new variant and they plan "to provide a converter tool from the old binary to the new format to facilitate the use of the new instructions without the need for recompilation." So we may end up with three ABIs. |
That is true today but the plan is to make llvm support the new variant natively ASAP. However at the at point I imagine there will be a lowering pass added to binaryen to support the previous version, so you should still only need a single object file format / libc build. |
While its true emscripten supports a lot of different options almost all of them are link-time only settings and don't effect the object file ABI. There are only subset of settings that effect the object format (i.e. that are vaslid at compile time): https://github.com/emscripten-core/emscripten/blob/10cb9d46cdd17e7a96de68137c9649d9a630fbc7/tools/settings.py#L81-L112. Note that WASM_BIGINT is not one of them. |
Are |
Only in the sense that they enable |
In general, " So I would prefer that we not expose, nor even pretend that we wish to offer compatibility with, "tons of different ABIs". We already have quite enough problems with ABI-affecting flags. It does not really take many booleans to be able to count to a million, and a few million untested variants of Rust is not a thing we should be trying to support. Having said that, supporting the new ABI behind an unstable flag that will then be ditched at some point has precedent, especially for wasm targets, see #117919 |
If a While we Just Merged the soundness-fixing change of #131533 we usually do MCPs for Significant Changes To Preexisting Targets, especially ones that require this... shindig. A transition period and involvement of rustc's CLI. They're usually sensible enough that it's a bit of a formality, so don't overthink it, the answers to my questions are more-or-less what you would paste in there, with a bit of additional stitching up to explain why e.g. Pyodide needs a flag for this transition and we don't want to do just a hard cut over. |
Sounds good. Thanks again @workingjubilee for all of your help. I really appreciate it.
Yeah this is why I'm raising this as a concern. It seems clearly out of scope to support all of them. It's important to have a solution to this in mind ahead of time. I would prefer that Rust only supports the 2 configurations I care about and has real test coverage for them, but different projects will standardize on different options. It's a thorny issue coordination problem. It would be nice if Emscripten could formalize specific triples... |
The fact that the ABIs can be linearly sorted by runtime support date is helpful. Instead of 20 different booleans leading to 2^20 different options toggling each independently, there are 21 options when we sort. Then if you only support:
maybe it's good enough. But having the in-between steps available as |
Okay I opened a MCP: |
This does not answer the question: what happens when I link together crates that were compiled with different values for this flag? In particular, what if If the answer is Undefined Behavior, that must at least be very clearly documented as part of the flag docs, and that is a blocker for stabilizing this flag. |
@hoodmane I also have a question regarding the plan outlined in the MCP (I'd prefer to ask this on Zulip but could not find your handle there):
Given that (IIUC) the flag you choose must match the flag used to build the standard library, how is this supposed to work? |
I just signed up for Zulip. Thanks @RalfJung. |
@hoodmane Remember to tag this with rustbot review when you update it. |
This is a draft because we need some additional setting for the Emscripten target to select between the old exception handling and the new exception handling. I don't know how to add a setting like that, would appreciate advice from Rust folks. We could maybe choose to use the new exception handling if
Ctarget-feature=+exception-handling
is passed? I tried this but I get errors from llvm so I'm not doing it right.cc @kleisauke @kripken @sbc100 @workingjubilee @alexcrichton