Repository navigation
Fix the floating point clobbers of RISC-V clobber_abi - #163839
Conversation
|
rustbot has assigned @petrochenkov. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
Relevant stabilization PR: #161385 |
|
r? @Amanieu |
@Amanieu will you have time to review this so we can backport it before 1.100? Or would you prefer a revert? |
|
Yes, adding more clobbers is an entirely conservative change, and should be safe to backport. @bors r+ |
… r=Amanieu
Fix the floating point clobbers of RISC-V `clobber_abi`
On RISC-V, callee-saved floating point registers are only callee-saved up to ABI_FLEN bits; the rest of the register is considered temporary. For `clobber_abi("C")`, as LLVM doesn't support partial register clobbers, this requires the callee-saved floating point registers to be clobbered.
These issues are noticeable in beta but not stable due to the recent stabilisation of the f and d target features (tracking issue: rust-lang#150257) so this probably needs a beta backport unless the stabilisation is reverted.
… r=Amanieu
Fix the floating point clobbers of RISC-V `clobber_abi`
On RISC-V, callee-saved floating point registers are only callee-saved up to ABI_FLEN bits; the rest of the register is considered temporary. For `clobber_abi("C")`, as LLVM doesn't support partial register clobbers, this requires the callee-saved floating point registers to be clobbered.
These issues are noticeable in beta but not stable due to the recent stabilisation of the f and d target features (tracking issue: rust-lang#150257) so this probably needs a beta backport unless the stabilisation is reverted.
…uwer Rollup of 21 pull requests Successful merges: - #164851 (Add behavior tests for `Arc` and `Rc`) - #164989 (rustdoc: Fix performance regression for private documented items) - #165079 (Remove GenericArgRef) - #161781 (Don't emit InlineAsm terminator if input operand is `!`.) - #162754 (optimize large elements for slice rotating) - #163252 (Remove windows-targets dependency from the root workspace) - #163839 (Fix the floating point clobbers of RISC-V `clobber_abi`) - #165104 (Use getrandom instead of /scheme/rand on Redox OS) - #165132 (Implement `advance_back_by` on `Fuse`) - #165142 (Remove the unused generic Graphviz writer) - #163371 (Don't suggest replacing foreign format specifiers in `concat!` output) - #163802 (fix RegionEq direction in canonical query responses) - #163986 (omit unconditionally invoking crate level limits query) - #164920 (Point at prior token on `let` when expression is expected) - #164987 (use adt_async_drop_tys for nested ADTs in needs_async_drop) - #165061 (Use a fixed-length encoding for hash disambiguators in v0 symbols) - #165114 (use native interpreter impl for funnel shifts intrinsics) - #165119 (remove unnecessary fast path from borrowck) - #165122 (fix MaybeDangling validity logic for ZST at usize::MAX) - #165128 (Move `doc(keyword)` and `doc(attribute)` attributes checks from `rustc_passes` to `rustc_attr_parsing`) - #165129 (Fix ICE in prelude edition lint) Failed merges: - #164981 (Reflection add TypeId::name)
… r=Amanieu
Fix the floating point clobbers of RISC-V `clobber_abi`
On RISC-V, callee-saved floating point registers are only callee-saved up to ABI_FLEN bits; the rest of the register is considered temporary. For `clobber_abi("C")`, as LLVM doesn't support partial register clobbers, this requires the callee-saved floating point registers to be clobbered.
These issues are noticeable in beta but not stable due to the recent stabilisation of the f and d target features (tracking issue: rust-lang#150257) so this probably needs a beta backport unless the stabilisation is reverted.
…uwer Rollup of 22 pull requests Successful merges: - #164851 (Add behavior tests for `Arc` and `Rc`) - #164989 (rustdoc: Fix performance regression for private documented items) - #165079 (Remove GenericArgRef) - #161781 (Don't emit InlineAsm terminator if input operand is `!`.) - #162754 (optimize large elements for slice rotating) - #163252 (Remove windows-targets dependency from the root workspace) - #163839 (Fix the floating point clobbers of RISC-V `clobber_abi`) - #165104 (Use getrandom instead of /scheme/rand on Redox OS) - #165132 (Implement `advance_back_by` on `Fuse`) - #165142 (Remove the unused generic Graphviz writer) - #165148 (ci: increase free disk space on macos-26 jobs) - #163371 (Don't suggest replacing foreign format specifiers in `concat!` output) - #163802 (fix RegionEq direction in canonical query responses) - #163986 (omit unconditionally invoking crate level limits query) - #164920 (Point at prior token on `let` when expression is expected) - #164987 (use adt_async_drop_tys for nested ADTs in needs_async_drop) - #165061 (Use a fixed-length encoding for hash disambiguators in v0 symbols) - #165114 (use native interpreter impl for funnel shifts intrinsics) - #165119 (remove unnecessary fast path from borrowck) - #165122 (fix MaybeDangling validity logic for ZST at usize::MAX) - #165128 (Move `doc(keyword)` and `doc(attribute)` attributes checks from `rustc_passes` to `rustc_attr_parsing`) - #165129 (Fix ICE in prelude edition lint) Failed merges: - #164981 (Reflection add TypeId::name) > [!WARNING] > This rollup conflicts with pending auto build #165205 and may need to be recreated if the pending build succeeds.
…uwer Rollup of 22 pull requests Successful merges: - #164851 (Add behavior tests for `Arc` and `Rc`) - #164989 (rustdoc: Fix performance regression for private documented items) - #165079 (Remove GenericArgRef) - #161781 (Don't emit InlineAsm terminator if input operand is `!`.) - #162754 (optimize large elements for slice rotating) - #163252 (Remove windows-targets dependency from the root workspace) - #163839 (Fix the floating point clobbers of RISC-V `clobber_abi`) - #165104 (Use getrandom instead of /scheme/rand on Redox OS) - #165132 (Implement `advance_back_by` on `Fuse`) - #165142 (Remove the unused generic Graphviz writer) - #165148 (ci: increase free disk space on macos-26 jobs) - #163371 (Don't suggest replacing foreign format specifiers in `concat!` output) - #163802 (fix RegionEq direction in canonical query responses) - #163986 (omit unconditionally invoking crate level limits query) - #164920 (Point at prior token on `let` when expression is expected) - #164987 (use adt_async_drop_tys for nested ADTs in needs_async_drop) - #165061 (Use a fixed-length encoding for hash disambiguators in v0 symbols) - #165114 (use native interpreter impl for funnel shifts intrinsics) - #165119 (remove unnecessary fast path from borrowck) - #165122 (fix MaybeDangling validity logic for ZST at usize::MAX) - #165128 (Move `doc(keyword)` and `doc(attribute)` attributes checks from `rustc_passes` to `rustc_attr_parsing`) - #165129 (Fix ICE in prelude edition lint) Failed merges: - #164981 (Reflection add TypeId::name) > [!WARNING] > This rollup conflicts with pending auto build #165205 and may need to be recreated if the pending build succeeds.
…uwer Rollup of 22 pull requests Successful merges: - #164851 (Add behavior tests for `Arc` and `Rc`) - #164989 (rustdoc: Fix performance regression for private documented items) - #165079 (Remove GenericArgRef) - #161781 (Don't emit InlineAsm terminator if input operand is `!`.) - #162754 (optimize large elements for slice rotating) - #163252 (Remove windows-targets dependency from the root workspace) - #163839 (Fix the floating point clobbers of RISC-V `clobber_abi`) - #165104 (Use getrandom instead of /scheme/rand on Redox OS) - #165132 (Implement `advance_back_by` on `Fuse`) - #165142 (Remove the unused generic Graphviz writer) - #165148 (ci: increase free disk space on macos-26 jobs) - #163371 (Don't suggest replacing foreign format specifiers in `concat!` output) - #163802 (fix RegionEq direction in canonical query responses) - #163986 (omit unconditionally invoking crate level limits query) - #164920 (Point at prior token on `let` when expression is expected) - #164987 (use adt_async_drop_tys for nested ADTs in needs_async_drop) - #165061 (Use a fixed-length encoding for hash disambiguators in v0 symbols) - #165114 (use native interpreter impl for funnel shifts intrinsics) - #165119 (remove unnecessary fast path from borrowck) - #165122 (fix MaybeDangling validity logic for ZST at usize::MAX) - #165128 (Move `doc(keyword)` and `doc(attribute)` attributes checks from `rustc_passes` to `rustc_attr_parsing`) - #165129 (Fix ICE in prelude edition lint) Failed merges: - #164981 (Reflection add TypeId::name) > [!WARNING] > This rollup conflicts with pending auto build #165205 and may need to be recreated if the pending build succeeds.
…uwer Rollup of 22 pull requests Successful merges: - #164851 (Add behavior tests for `Arc` and `Rc`) - #164989 (rustdoc: Fix performance regression for private documented items) - #165079 (Remove GenericArgRef) - #161781 (Don't emit InlineAsm terminator if input operand is `!`.) - #162754 (optimize large elements for slice rotating) - #163252 (Remove windows-targets dependency from the root workspace) - #163839 (Fix the floating point clobbers of RISC-V `clobber_abi`) - #165104 (Use getrandom instead of /scheme/rand on Redox OS) - #165132 (Implement `advance_back_by` on `Fuse`) - #165142 (Remove the unused generic Graphviz writer) - #165233 (Remove test-aarch64-apple-* jobs temporarily) - #163371 (Don't suggest replacing foreign format specifiers in `concat!` output) - #163802 (fix RegionEq direction in canonical query responses) - #163986 (omit unconditionally invoking crate level limits query) - #164920 (Point at prior token on `let` when expression is expected) - #164987 (use adt_async_drop_tys for nested ADTs in needs_async_drop) - #165061 (Use a fixed-length encoding for hash disambiguators in v0 symbols) - #165114 (use native interpreter impl for funnel shifts intrinsics) - #165119 (remove unnecessary fast path from borrowck) - #165122 (fix MaybeDangling validity logic for ZST at usize::MAX) - #165128 (Move `doc(keyword)` and `doc(attribute)` attributes checks from `rustc_passes` to `rustc_attr_parsing`) - #165129 (Fix ICE in prelude edition lint) > [!WARNING] > This rollup conflicts with pending auto build #165217 and may need to be recreated if the pending build succeeds.
Rollup merge of #163839 - beetrees:fix-riscv-clobber-abi-fd, r=Amanieu Fix the floating point clobbers of RISC-V `clobber_abi` On RISC-V, callee-saved floating point registers are only callee-saved up to ABI_FLEN bits; the rest of the register is considered temporary. For `clobber_abi("C")`, as LLVM doesn't support partial register clobbers, this requires the callee-saved floating point registers to be clobbered. These issues are noticeable in beta but not stable due to the recent stabilisation of the f and d target features (tracking issue: #150257) so this probably needs a beta backport unless the stabilisation is reverted.
|
Note This PR was benchmarked as part of triage of its containing rollup: triage URL. Finished benchmarking commit (b614c5b): comparison URL. Overall result: ❌✅ regressions and improvements - please read:Our benchmarks found a performance regression caused by this PR. Next Steps:
@rustbot label: +perf-regression Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)This perf run didn't have relevant results for this metric. CyclesResults (secondary 3.3%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis perf run didn't have relevant results for this metric. Artifact size: 406.61 MiB -> 406.81 MiB (0.05%) |
…Amanieu Fix RISC-V inline ASM `clobber_abi` when extra target features are enabled Waiting on rust-lang#163839, a beta-backportable subset of this PR. This PR fixes several issues with `clobber_abi("C")` on RISC-V: - Whether to clobber the registers for `ilp32e`/`lp64e` or not was previously decided based on whether the `e` target feature was enabled, as opposed to the actual ABI in use. - `ilp32e`/`lp64e` failed to clobber all floating point registers and the upper integer registers as specified in the ABI specification. As a side effect of fixing this, `x16`-`x31` can now be used as clobbers when the `e` feature is enabled (as RISC-V inline ASM is stable, I think this might require a FCP?). - Callee-saved floating point registers are only callee-saved up to ABI_FLEN bits; the rest of the register is considered temporary. As LLVM doesn't support partial register clobbers, this requires the callee-saved floating point registers to be clobbered.
|
@davidtwco @BoxyUwU @Mark-Simulacrum this was meant to be included in the imminent release, where some risc-v target features are becoming stable so we should ensure the clobbers are correct. Is that still possible? |
Rollup merge of #163833 - beetrees:fix-riscv-clobber-abi, r=Amanieu Fix RISC-V inline ASM `clobber_abi` when extra target features are enabled Waiting on #163839, a beta-backportable subset of this PR. This PR fixes several issues with `clobber_abi("C")` on RISC-V: - Whether to clobber the registers for `ilp32e`/`lp64e` or not was previously decided based on whether the `e` target feature was enabled, as opposed to the actual ABI in use. - `ilp32e`/`lp64e` failed to clobber all floating point registers and the upper integer registers as specified in the ABI specification. As a side effect of fixing this, `x16`-`x31` can now be used as clobbers when the `e` feature is enabled (as RISC-V inline ASM is stable, I think this might require a FCP?). - Callee-saved floating point registers are only callee-saved up to ABI_FLEN bits; the rest of the register is considered temporary. As LLVM doesn't support partial register clobbers, this requires the callee-saved floating point registers to be clobbered.
…uwer Rollup of 22 pull requests Successful merges: - rust-lang/rust#164851 (Add behavior tests for `Arc` and `Rc`) - rust-lang/rust#164989 (rustdoc: Fix performance regression for private documented items) - rust-lang/rust#165079 (Remove GenericArgRef) - rust-lang/rust#161781 (Don't emit InlineAsm terminator if input operand is `!`.) - rust-lang/rust#162754 (optimize large elements for slice rotating) - rust-lang/rust#163252 (Remove windows-targets dependency from the root workspace) - rust-lang/rust#163839 (Fix the floating point clobbers of RISC-V `clobber_abi`) - rust-lang/rust#165104 (Use getrandom instead of /scheme/rand on Redox OS) - rust-lang/rust#165132 (Implement `advance_back_by` on `Fuse`) - rust-lang/rust#165142 (Remove the unused generic Graphviz writer) - rust-lang/rust#165233 (Remove test-aarch64-apple-* jobs temporarily) - rust-lang/rust#163371 (Don't suggest replacing foreign format specifiers in `concat!` output) - rust-lang/rust#163802 (fix RegionEq direction in canonical query responses) - rust-lang/rust#163986 (omit unconditionally invoking crate level limits query) - rust-lang/rust#164920 (Point at prior token on `let` when expression is expected) - rust-lang/rust#164987 (use adt_async_drop_tys for nested ADTs in needs_async_drop) - rust-lang/rust#165061 (Use a fixed-length encoding for hash disambiguators in v0 symbols) - rust-lang/rust#165114 (use native interpreter impl for funnel shifts intrinsics) - rust-lang/rust#165119 (remove unnecessary fast path from borrowck) - rust-lang/rust#165122 (fix MaybeDangling validity logic for ZST at usize::MAX) - rust-lang/rust#165128 (Move `doc(keyword)` and `doc(attribute)` attributes checks from `rustc_passes` to `rustc_attr_parsing`) - rust-lang/rust#165129 (Fix ICE in prelude edition lint) > [!WARNING] > This rollup conflicts with pending auto build #165217 and may need to be recreated if the pending build succeeds.
Fix RISC-V inline ASM `clobber_abi` when extra target features are enabled Waiting on rust-lang/rust#163839, a beta-backportable subset of this PR. This PR fixes several issues with `clobber_abi("C")` on RISC-V: - Whether to clobber the registers for `ilp32e`/`lp64e` or not was previously decided based on whether the `e` target feature was enabled, as opposed to the actual ABI in use. - `ilp32e`/`lp64e` failed to clobber all floating point registers and the upper integer registers as specified in the ABI specification. As a side effect of fixing this, `x16`-`x31` can now be used as clobbers when the `e` feature is enabled (as RISC-V inline ASM is stable, I think this might require a FCP?). - Callee-saved floating point registers are only callee-saved up to ABI_FLEN bits; the rest of the register is considered temporary. As LLVM doesn't support partial register clobbers, this requires the callee-saved floating point registers to be clobbered.
|
beta backport declined as per compiler team on Zulip. |
Revert <rust-lang#161385> on beta to let the fix <rust-lang#163839> and the stabilization itself bake for another release cycle. Discussed whether to backport a fix or postpone stabilization in <https://rust-lang.zulipchat.com/#narrow/channel/474880-t-compiler.2Fbackports/topic/.23163839.3A.20beta-nominated/with/636835799>. The call there was to be safer and postpone the stabilization by a release cycle. This reverts commit 430e887, reversing changes made to d9b3213.
|
@rustbot label: +perf-regression-triaged |
…uwer Rollup of 22 pull requests Successful merges: - rust-lang/rust#164851 (Add behavior tests for `Arc` and `Rc`) - rust-lang/rust#164989 (rustdoc: Fix performance regression for private documented items) - rust-lang/rust#165079 (Remove GenericArgRef) - rust-lang/rust#161781 (Don't emit InlineAsm terminator if input operand is `!`.) - rust-lang/rust#162754 (optimize large elements for slice rotating) - rust-lang/rust#163252 (Remove windows-targets dependency from the root workspace) - rust-lang/rust#163839 (Fix the floating point clobbers of RISC-V `clobber_abi`) - rust-lang/rust#165104 (Use getrandom instead of /scheme/rand on Redox OS) - rust-lang/rust#165132 (Implement `advance_back_by` on `Fuse`) - rust-lang/rust#165142 (Remove the unused generic Graphviz writer) - rust-lang/rust#165233 (Remove test-aarch64-apple-* jobs temporarily) - rust-lang/rust#163371 (Don't suggest replacing foreign format specifiers in `concat!` output) - rust-lang/rust#163802 (fix RegionEq direction in canonical query responses) - rust-lang/rust#163986 (omit unconditionally invoking crate level limits query) - rust-lang/rust#164920 (Point at prior token on `let` when expression is expected) - rust-lang/rust#164987 (use adt_async_drop_tys for nested ADTs in needs_async_drop) - rust-lang/rust#165061 (Use a fixed-length encoding for hash disambiguators in v0 symbols) - rust-lang/rust#165114 (use native interpreter impl for funnel shifts intrinsics) - rust-lang/rust#165119 (remove unnecessary fast path from borrowck) - rust-lang/rust#165122 (fix MaybeDangling validity logic for ZST at usize::MAX) - rust-lang/rust#165128 (Move `doc(keyword)` and `doc(attribute)` attributes checks from `rustc_passes` to `rustc_attr_parsing`) - rust-lang/rust#165129 (Fix ICE in prelude edition lint) > [!WARNING] > This rollup conflicts with pending auto build #165217 and may need to be recreated if the pending build succeeds.
On RISC-V, callee-saved floating point registers are only callee-saved up to ABI_FLEN bits; the rest of the register is considered temporary. For
clobber_abi("C"), as LLVM doesn't support partial register clobbers, this requires the callee-saved floating point registers to be clobbered.These issues are noticeable in beta but not stable due to the recent stabilisation of the f and d target features (tracking issue: #150257) so this probably needs a beta backport unless the stabilisation is reverted.