Skip to content

Fix RISC-V C function ABI when passing/returning structs containing floats #139340

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

beetrees
Copy link
Contributor

@beetrees beetrees commented Apr 3, 2025

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 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

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 3, 2025
@rustbot
Copy link
Collaborator

rustbot commented Apr 3, 2025

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

Some changes occurred in compiler/rustc_codegen_ssa

cc @WaffleLapkin

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

@rust-log-analyzer

This comment has been minimized.

@beetrees beetrees force-pushed the riscv-float-struct-abi branch from 725f521 to 2b5c6a9 Compare April 3, 2025 23:38
@beetrees beetrees force-pushed the riscv-float-struct-abi branch from 2b5c6a9 to 5f18664 Compare April 4, 2025 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants