Repository navigation
Conversation
|
r? @chenyukang rustbot has assigned @chenyukang. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
@rustbot reroll |
33ff467 to
b0cd986
Compare
This comment has been minimized.
This comment has been minimized.
|
Or I just move the warning to creation time of the |
b0cd986 to
ebe9107
Compare
|
Per #t-compiler/major changes > Introduce new -C flag for cross-target c… compiler-team#1027 @ 💬 I think this needs an MCP, r? @davidtwco |
|
my understanding is that we need the MCP as soon as the |
|
My impression from the Zulip thread and the MCP RFC is that an MCP is generally for when the (unstable) flag is being added, with a rfcbot FCP required for stabilisation. |
|
Thanks for looking into this. From my perspective, a hard switch would be perfectly fine. The grace period actually caused some confusion on my side, and I don't think there's much value in carrying both variants for a transition period. While enabling Rust support for s390 in the Linux kernel, I ended up using this argument as well, and I'd be happy to adapt to the new form directly. Redirecting the argument from one version to the next sounds like a good solution to me. |
|
I think it'd be very confusing for |
ebe9107 to
369414c
Compare
|
Target features are being changed; ensure all ABI effects are being accounted for cc @RalfJung |
369414c to
74285e0
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
This comment has been minimized.
This comment has been minimized.
On s390x, the `backchain` LLVM function attribute saves the "unwind" (backchain) pointer (r15) into the caller's parameter area, enabling external stack unwinders (e.g. Linux perf) to walk the call chain. Previously this was enabled via `-Ctarget-feature=+backchain`, which piggy-backed on the target-feature machinery even though backchain is not a CPU or ISA feature. This commit introduces a cleaner mechanism: - `-Cforce-frame-pointers` now enables the `"backchain"` LLVM *function attribute* on s390x, rather than the generic `"frame-pointer"` attribute used on other architectures. - `-Ctarget-feature=+backchain` is completely removed and will be rejected. - When the backchain opt-in is active, the `"frame-pointer"` attribute is suppressed on s390x. Other reasons to set frame pointers (e.g. `-Zinstrument-mcount`) are unaffected and still emit `"frame-pointer"`. - All s390x-specific frame-pointer/backchain/packed-stack logic is consolidated in `s390x_fn_attrs()` in `attributes.rs`. Co-authored-by: Ralf Jung <post@ralfj.de>
74285e0 to
91aa84d
Compare
Summary
On s390x, the
backchainLLVM function attribute saves the "unwind" (backchain) pointer (r15) into the caller's parameter area, enabling external stack unwinders (e.g. Linux perf) to walk the call chain. Previously this was enabled via-Ctarget-feature=+backchain, which piggy-backed on the target-feature machinery even though backchain is not a CPU or ISA feature.This commit introduces a cleaner mechanism:
-Cforce-frame-pointersnow enables the"backchain"LLVM function attribute on s390x, rather than the generic"frame-pointer"attribute used on other architectures.-Ctarget-feature=+backchainemits a error pointing to the new mechanism.-Zinstrument-mcount) are unaffected and still emit"frame-pointer".s390x_fn_attrs()inattributes.rs.This implements the proposed solution discussed in here on Zulip.
Background
Frame pointers on s390x
Setting
-Cforce-frame-pointers=yeson s390x forces the"frame-pointer"LLVM attribute, but this does not form a traversable linked list. As [@uweigand] explained ([#150766 comment]):Therefore
-Cforce-frame-pointerson s390x provides no stack-walking benefit.Backchain
The s390x ELF ABI reserves backchain slot - a pointer back to the caller's frame. This can be used to walk the entire call chain without DWARF CFI. LLVM exposes this as the
"backchain"function attribute, equivalent to GCC's-mbackchain.Backchain is not a CPU capability. It is a whole-binary code-generation convention, similar in nature to
-fno-omit-frame-pointeron x86. This is reflected in LLVM modelling it as a function attribute, not a target feature [#150766 comment 2].Enabling backchain does not break ABI compatibility: backchain-enabled and -disabled functions can freely call each other.
The one notable exception is
packed-stack: combining-Zpacked-stackwith backchain on a non-softfloat target is rejected at compile time([#152432]).rustc arg discussion
Initially backchain was introduced as an unstable
target-featuremimicing the gcc and clang behavior. Within the grater effort of stabilizing arguments that are needed for Linux Kernel development, attempts where made to stabilize thetarget-feature=+backchain. This lead to discussions and the overall consent is that:frame-poitnerswill never be done by the user.frome-pointersthe user actually wants: stack-unwinding on x86 viaframe-pointersbut ons390xviabackchain-Cforce-frame-pointers=yesshould ons390xenablebackchainand NOT theframe-pointerattribute-Ctarget-feature=+backchainpattern should emit a nice error and point to-Cforce-frame-pointers=yesReferences
cfg(target_feature = "backchain")(s390x target feature) be enabled? #142412soft-floatandpacked-stack#150766 (comment)soft-floatandpacked-stack#150766 (comment)s390x-unknown-none-softfloatwithRustcAbi::Softfloat#151154-Ctarget-feature=+backchain#158014