Skip to content

LLVM 20 produces different assembly for rust/tests/assembly/riscv-soft-abi-with-float-features.rs #132139

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

Closed
krasimirgg opened this issue Oct 25, 2024 · 4 comments · Fixed by #132266
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-discussion Category: Discussion or questions that doesn't represent real issues. O-riscv Target: RISC-V architecture T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@krasimirgg
Copy link
Contributor

krasimirgg commented Oct 25, 2024

After #131807, we're seeing some test failures in our experimental rust + LLVM-at-HEAD build bot:

It seems that here the new assembly is just lw a0, 0(a0); not sure if that's OK.

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Oct 25, 2024
@krasimirgg
Copy link
Contributor Author

@rustbot label: A-LLVM

@rustbot rustbot added the A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. label Oct 25, 2024
@jieyouxu jieyouxu added the O-riscv Target: RISC-V architecture label Oct 25, 2024
@jieyouxu
Copy link
Member

cc @beetrees @workingjubilee as you may know more about the intended behavior

@jieyouxu jieyouxu added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Oct 25, 2024
@krasimirgg krasimirgg changed the title LLVM 20 produces different assembly for rust/tests/assembly /riscv-soft-abi-with-float-features.rs LLVM 20 produces different assembly for rust/tests/assembly/riscv-soft-abi-with-float-features.rs Oct 25, 2024
@hanna-kruppe
Copy link
Contributor

hanna-kruppe commented Oct 25, 2024

The change seems correct to me. It loads from the same location as before and puts the same bits in the return value register for the soft float ABI. Both variants just load 32 bits from memory into the low half of a0 (all of the involved instructions preserve non-canonical NaNs) and sign-extend to fill the upper 32 bits of a0 (previously in the fmv.x.w step, now it’s part of the “integer” load lw).

@beetrees
Copy link
Contributor

The change is correct; the soft-float ABI for RISC-V states "Scalars that are at most XLEN bits wide are passed in a single argument register, or on the stack by value if none is available. [..] When passed in registers or on the stack, floating-point types narrower than XLEN bits are widened to XLEN bits, with the upper bits undefined."

@jieyouxu jieyouxu added C-discussion Category: Discussion or questions that doesn't represent real issues. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Oct 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-discussion Category: Discussion or questions that doesn't represent real issues. O-riscv Target: RISC-V architecture T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants