Fix RISC-V C function ABI when passing/returning structs containing floats #139340
+537
−134
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 in compiler/rustc_codegen_gcc Some changes occurred in compiler/rustc_codegen_ssa Some changes occurred in compiler/rustc_codegen_cranelift cc @bjorn3 |
This comment has been minimized.
This comment has been minimized.
725f521 to
2b5c6a9
Compare
bjorn3
reviewed
Apr 4, 2025
2b5c6a9 to
5f18664
Compare
|
Ping reviewer @workingjubilee |
|
@rustbot author |
|
Reminder, once the PR becomes ready for a review, use |
5f18664 to
86b24b8
Compare
This comment has been minimized.
This comment has been minimized.
86b24b8 to
5d9726a
Compare
|
(I had to rebase to fix a merge conflict.) @rustbot ready |
This comment has been minimized.
This comment has been minimized.
5d9726a to
65531ad
Compare
This comment has been minimized.
This comment has been minimized.
65531ad to
6df68f3
Compare
|
☔ The latest upstream changes (presumably #142550) made this pull request unmergeable. Please resolve the merge conflicts. |
workingjubilee
approved these changes
Jun 16, 2025
|
r=me once you fix the merge conflicts again (sorry about that!) @bors delegate+ |
|
✌️ @beetrees, you can now approve this pull request! If @workingjubilee told you to " |
6df68f3 to
5723c99
Compare
bors
added a commit
that referenced
this pull request
Jun 16, 2025
Rollup of 8 pull requests Successful merges: - #139340 (Fix RISC-V C function ABI when passing/returning structs containing floats) - #142341 (Don't suggest converting `///` to `//` when expecting `,`) - #142414 (ignore `run-make` tests that need `std` on targets without `std`) - #142498 (Port `#[rustc_as_ptr]` to the new attribute system) - #142554 (Fix `PathSource` lifetimes.) - #142562 (Update the `backtrace` submodule) - #142565 (Test naked asm for wasm32-unknown-unknown) - #142573 (`fn candidate_is_applicable` to method) r? `@ghost` `@rustbot` modify labels: rollup
rust-timer
added a commit
that referenced
this pull request
Jun 16, 2025
Rollup merge of #139340 - beetrees:riscv-float-struct-abi, r=workingjubilee Fix RISC-V C function ABI when passing/returning structs containing floats RISC-V passes structs containing only one or two floats (or a float and integer pair) in registers, as long as the individual floats/integers fit in a single corresponding register (see [the ABI specification](https://github.com/riscv-non-isa/riscv-elf-psabi-doc/releases/download/v1.0/riscv-abi.pdf) for details). Before this PR, Rust would not check what offset the second float/integer was at, instead assuming that it was at the standard offset for its default alignment. However, as the offset can be affected by `#[repr(align(N))]` and `#[repr(packed)]`, this caused miscompilations (see #115609). To fix this, this PR introduces a `rest_offset` field to `CastTarget` that can be used to explicitly specify at what offset the `rest` part of the cast is located at. While fixing this, I discovered another bug: the size of the cast target was being used as the size of the MIR return place (when the function was using a `PassMode::Cast` return type). However, the cast target is allowed to be smaller than the size of the actual type, causing a miscompilation. This PR fixes this issue by using the largest of the size of the type and the size of the cast target as the size of the MIR return place, ensuring all reads/writes will be inbounds. Fixes the RISC-V part of #115609. cc target maintainers of `riscv64gc-unknown-linux-gnu`: `@kito-cheng` `@michaelmaitland` `@robin-randhawa-sifive` `@topperc` r? `@workingjubilee`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
RISC-V passes structs containing only one or two floats (or a float and integer pair) in registers, as long as the individual floats/integers fit in a single corresponding register (see the ABI specification for details). Before this PR, Rust would not check what offset the second float/integer was at, instead assuming that it was at the standard offset for its default alignment. However, as the offset can be affected by
#[repr(align(N))]and#[repr(packed)], this caused miscompilations (see #115609). To fix this, this PR introduces arest_offsetfield toCastTargetthat can be used to explicitly specify at what offset therestpart of the cast is located at.While fixing this, I discovered another bug: the size of the cast target was being used as the size of the MIR return place (when the function was using a
PassMode::Castreturn type). However, the cast target is allowed to be smaller than the size of the actual type, causing a miscompilation. This PR fixes this issue by using the largest of the size of the type and the size of the cast target as the size of the MIR return place, ensuring all reads/writes will be inbounds.Fixes the RISC-V part of #115609.
cc target maintainers of
riscv64gc-unknown-linux-gnu: @kito-cheng @michaelmaitland @robin-randhawa-sifive @toppercr? @workingjubilee