Skip to content

Conversation

@heiher
Copy link
Contributor

@heiher heiher commented Aug 21, 2025

Similar to RISC-V, LoongArch passes structs containing only one or two floats (or a float–integer pair) in registers, as long as each element fits into a single corresponding register. Before this PR, Rust did not check the actual offset of the second float or integer; instead, it assumed the standard offset based on the default alignment. However, since the offset can be affected by #[repr(align(N))] and #[repr(packed)], this led to miscompilations (see #145692). This PR fixes the issue by explicitly specifying the offset for the remainder of the cast.

@rustbot
Copy link
Collaborator

rustbot commented Aug 21, 2025

r? @jackh726

rustbot has assigned @jackh726.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@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 Aug 21, 2025
Copy link
Contributor

@Gelbpunkt Gelbpunkt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does fix the test failure for me.

View changes since this review

@heiher
Copy link
Contributor Author

heiher commented Aug 23, 2025

r? @workingjubilee

@rustbot rustbot assigned workingjubilee and unassigned jackh726 Aug 23, 2025
@rustbot
Copy link
Collaborator

rustbot commented Aug 23, 2025

workingjubilee is currently at their maximum review capacity.
They may take a while to respond.

@heiher
Copy link
Contributor Author

heiher commented Sep 4, 2025

a gentle ping. r? compiler

@rustbot rustbot assigned fee1-dead and unassigned workingjubilee Sep 4, 2025
@jackh726
Copy link
Member

jackh726 commented Sep 4, 2025

r? jackh726

Can you provide a bit of high-level context on the root issue and the fix here?

@rustbot rustbot assigned jackh726 and unassigned fee1-dead Sep 4, 2025
@jackh726
Copy link
Member

jackh726 commented Sep 4, 2025

Code-wise, this looks fine. I do think (if nothing else but for posterity) some context added to the PR would be good.

Though, r=me after that.

@bors delegate+

@bors
Copy link
Collaborator

bors commented Sep 4, 2025

✌️ @heiher, you can now approve this pull request!

If @jackh726 told you to "r=me" after making some further change, please make that change, then do @bors r=@jackh726

@heiher
Copy link
Contributor Author

heiher commented Sep 5, 2025

@bors r=@jackh726

@bors
Copy link
Collaborator

bors commented Sep 5, 2025

📌 Commit b65a177 has been approved by jackh726

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 5, 2025
tgross35 added a commit to tgross35/rust that referenced this pull request Sep 5, 2025
Fix LoongArch C function ABI when passing/returning structs containing floats

Similar to RISC-V, LoongArch passes structs containing only one or two floats (or a float–integer pair) in registers, as long as each element fits into a single corresponding register. Before this PR, Rust did not check the actual offset of the second float or integer; instead, it assumed the standard offset based on the default alignment. However, since the offset can be affected by `#[repr(align(N))]` and `#[repr(packed)]`, this led to miscompilations (see rust-lang#145692). This PR fixes the issue by explicitly specifying the offset for the remainder of the cast.
tgross35 added a commit to tgross35/rust that referenced this pull request Sep 5, 2025
Fix LoongArch C function ABI when passing/returning structs containing floats

Similar to RISC-V, LoongArch passes structs containing only one or two floats (or a float–integer pair) in registers, as long as each element fits into a single corresponding register. Before this PR, Rust did not check the actual offset of the second float or integer; instead, it assumed the standard offset based on the default alignment. However, since the offset can be affected by `#[repr(align(N))]` and `#[repr(packed)]`, this led to miscompilations (see rust-lang#145692). This PR fixes the issue by explicitly specifying the offset for the remainder of the cast.
tgross35 added a commit to tgross35/rust that referenced this pull request Sep 5, 2025
Fix LoongArch C function ABI when passing/returning structs containing floats

Similar to RISC-V, LoongArch passes structs containing only one or two floats (or a float–integer pair) in registers, as long as each element fits into a single corresponding register. Before this PR, Rust did not check the actual offset of the second float or integer; instead, it assumed the standard offset based on the default alignment. However, since the offset can be affected by `#[repr(align(N))]` and `#[repr(packed)]`, this led to miscompilations (see rust-lang#145692). This PR fixes the issue by explicitly specifying the offset for the remainder of the cast.
bors added a commit that referenced this pull request Sep 5, 2025
Rollup of 6 pull requests

Successful merges:

 - #144342 (add exact bitshifts)
 - #145709 (Fix LoongArch C function ABI when passing/returning structs containing floats)
 - #146152 (Unify and deduplicate algebraic float tests)
 - #146186 (Update cc-rs to 1.2.33, and switch rustc_codegen_ssa to use find-msvc-tools)
 - #146207 (std: Implement WASIp2-specific stdio routines)
 - #146217 (fix ICE when suggesting `::new`)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit that referenced this pull request Sep 5, 2025
Rollup of 5 pull requests

Successful merges:

 - #144342 (add exact bitshifts)
 - #145709 (Fix LoongArch C function ABI when passing/returning structs containing floats)
 - #146152 (Unify and deduplicate algebraic float tests)
 - #146207 (std: Implement WASIp2-specific stdio routines)
 - #146217 (fix ICE when suggesting `::new`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 3b8d411 into rust-lang:master Sep 5, 2025
10 checks passed
@rustbot rustbot added this to the 1.91.0 milestone Sep 5, 2025
rust-timer added a commit that referenced this pull request Sep 5, 2025
Rollup merge of #145709 - heiher:issue-145692-1, r=jackh726

Fix LoongArch C function ABI when passing/returning structs containing floats

Similar to RISC-V, LoongArch passes structs containing only one or two floats (or a float–integer pair) in registers, as long as each element fits into a single corresponding register. Before this PR, Rust did not check the actual offset of the second float or integer; instead, it assumed the standard offset based on the default alignment. However, since the offset can be affected by `#[repr(align(N))]` and `#[repr(packed)]`, this led to miscompilations (see #145692). This PR fixes the issue by explicitly specifying the offset for the remainder of the cast.
@bors
Copy link
Collaborator

bors commented Sep 5, 2025

⌛ Testing commit b65a177 with merge c559c4a...

@heiher heiher deleted the issue-145692-1 branch September 5, 2025 12:35
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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants