-
Notifications
You must be signed in to change notification settings - Fork 13.8k
Skip cleanups on unsupported targets #146457
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
Merged
Merged
+86
−27
Conversation
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
Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt |
rustbot has assigned @petrochenkov. Use |
This comment has been minimized.
This comment has been minimized.
This commit is an update to the `AbortUnwindingCalls` MIR pass in the compiler. Specifically a new boolean is added for "can this target possibly unwind" and if that's `false` then terminators are all adjusted to be unreachable/not present. The end result is that this fixes 140293 for wasm targets. The motivation for this PR is that currently on WebAssembly targets the usage of the `C-unwind` ABI can lead LLVM to either (a) emit exception-handling instructions or (b) hit a LLVM-ICE-style codegen error. WebAssembly as a base instruction set does not support unwinding at all, and a later proposal to WebAssembly, the exception-handling proposal, was what enabled this. This means that the current intent of WebAssembly targets is that they maintain the baseline of "don't emit exception-handling instructions unless enabled". The commit here is intended to restore this behavior by skipping these instructions even when `C-unwind` is present. Exception-handling is a relatively tricky and also murky topic in WebAssembly, however. There are two sets of instructions LLVM can emit for WebAssembly exceptions, Rust's Emscripten target supports exceptions, WASI targets do not, the LLVM flags to enable this are not always obvious, and additionally this all touches on "changing exception-handling behavior should be a target-level concern, not a feature". Effectively WebAssembly's exception-handling integration into Rust is not finalized at this time. The best idea at this time is that a parallel set of targets will eventually be added which support exceptions, but it's not clear if/when to do this. In the meantime the goal is to keep existing targets working while still enabling experimentation with exception-handling with `-Zbuild-std` and various permutations of LLVM flags. To that extent this commit does not blanket disable these landing pads and cleanup routines for WebAssembly but instead checks to see if panic=unwind is enabled or if `+exception-handling` is enabled. Tests are updated here as well to account for this where, by default, using a `C-unwind` ABI won't affect Rust codegen at all. If `+exception-handling` is enabled, however, then Rust codegen will look like native platforms where exceptions are caught and the program aborts. More-or-less I've done my best to keep exceptions working on wasm where it's possible to have them work, but turned them off where they're not supposed to be emitted.
356f832
to
88d7d20
Compare
This was referenced Sep 11, 2025
r? @bjorn3 maybe |
ping @bjorn3 would you be ok taking a look at this? |
bjorn3
reviewed
Sep 29, 2025
r=me with the test fixed if necessary. |
@bors: r=bjorn3 |
bors
added a commit
that referenced
this pull request
Sep 30, 2025
Rollup of 4 pull requests Successful merges: - #145883 (Make macOS dist build configuration match where reasonable) - #146457 (Skip cleanups on unsupported targets) - #147152 (builtin `Fn`-trait impls: instantiate binder before the return type `Sized` check) - #147153 ([rustdoc] Move doc cfg propagation pass before items stripping passes) r? `@ghost` `@rustbot` modify labels: rollup
rust-timer
added a commit
that referenced
this pull request
Sep 30, 2025
Rollup merge of #146457 - alexcrichton:wasm-no-exn-instructions, r=bjorn3 Skip cleanups on unsupported targets This commit is an update to the `AbortUnwindingCalls` MIR pass in the compiler. Specifically a new boolean is added for "can this target possibly unwind" and if that's `false` then terminators are all adjusted to be unreachable/not present. The end result is that this fixes #140293 for wasm targets. The motivation for this PR is that currently on WebAssembly targets the usage of the `C-unwind` ABI can lead LLVM to either (a) emit exception-handling instructions or (b) hit a LLVM-ICE-style codegen error. WebAssembly as a base instruction set does not support unwinding at all, and a later proposal to WebAssembly, the exception-handling proposal, was what enabled this. This means that the current intent of WebAssembly targets is that they maintain the baseline of "don't emit exception-handling instructions unless enabled". The commit here is intended to restore this behavior by skipping these instructions even when `C-unwind` is present. Exception-handling is a relatively tricky and also murky topic in WebAssembly, however. There are two sets of instructions LLVM can emit for WebAssembly exceptions, Rust's Emscripten target supports exceptions, WASI targets do not, the LLVM flags to enable this are not always obvious, and additionally this all touches on "changing exception-handling behavior should be a target-level concern, not a feature". Effectively WebAssembly's exception-handling integration into Rust is not finalized at this time. The best idea at this time is that a parallel set of targets will eventually be added which support exceptions, but it's not clear if/when to do this. In the meantime the goal is to keep existing targets working while still enabling experimentation with exception-handling with `-Zbuild-std` and various permutations of LLVM flags. To that extent this commit does not blanket disable these landing pads and cleanup routines for WebAssembly but instead checks to see if panic=unwind is enabled or if `+exception-handling` is enabled. Tests are updated here as well to account for this where, by default, using a `C-unwind` ABI won't affect Rust codegen at all. If `+exception-handling` is enabled, however, then Rust codegen will look like native platforms where exceptions are caught and the program aborts. More-or-less I've done my best to keep exceptions working on wasm where it's possible to have them work, but turned them off where they're not supposed to be emitted. Closes #140293
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
S-waiting-on-bors
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
T-compiler
Relevant to the compiler team, which will review and decide on the PR/issue.
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 commit is an update to the
AbortUnwindingCalls
MIR pass in the compiler. Specifically a new boolean is added for "can this target possibly unwind" and if that'sfalse
then terminators are all adjusted to be unreachable/not present. The end result is that this fixes #140293 for wasm targets.The motivation for this PR is that currently on WebAssembly targets the usage of the
C-unwind
ABI can lead LLVM to either (a) emit exception-handling instructions or (b) hit a LLVM-ICE-style codegen error. WebAssembly as a base instruction set does not support unwinding at all, and a later proposal to WebAssembly, the exception-handling proposal, was what enabled this. This means that the current intent of WebAssembly targets is that they maintain the baseline of "don't emit exception-handling instructions unless enabled". The commit here is intended to restore this behavior by skipping these instructions even whenC-unwind
is present.Exception-handling is a relatively tricky and also murky topic in WebAssembly, however. There are two sets of instructions LLVM can emit for WebAssembly exceptions, Rust's Emscripten target supports exceptions, WASI targets do not, the LLVM flags to enable this are not always obvious, and additionally this all touches on "changing exception-handling behavior should be a target-level concern, not a feature". Effectively WebAssembly's exception-handling integration into Rust is not finalized at this time. The best idea at this time is that a parallel set of targets will eventually be added which support exceptions, but it's not clear if/when to do this. In the meantime the goal is to keep existing targets working while still enabling experimentation with exception-handling with
-Zbuild-std
and various permutations of LLVM flags.To that extent this commit does not blanket disable these landing pads and cleanup routines for WebAssembly but instead checks to see if panic=unwind is enabled or if
+exception-handling
is enabled. Tests are updated here as well to account for this where, by default, using aC-unwind
ABI won't affect Rust codegen at all. If+exception-handling
is enabled, however, then Rust codegen will look like native platforms where exceptions are caught and the program aborts. More-or-less I've done my best to keep exceptions working on wasm where it's possible to have them work, but turned them off where they're not supposed to be emitted.Closes #140293