Skip to content
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

riscv32imc-unknown-none-elf with --target-feature=+d: cannot link files with different floating-point ABI #117847

Closed
jneem opened this issue Nov 12, 2023 · 5 comments
Labels
C-bug Category: This is a bug. O-riscv Target: RISC-V architecture

Comments

@jneem
Copy link
Contributor

jneem commented Nov 12, 2023

I tried this minimal no-std code:

#![no_std]
#![no_main]

use core::panic::PanicInfo;

#[panic_handler]
fn panic(_panic: &PanicInfo<'_>) -> ! {
    loop {}
}

Compiling it with rustc src/main.rs --target riscv32imc-unknown-none-elf -C target-feature=+d gives the linker error rust-lld: error: main.main.2f941e1c5654685d-cgu.0.rcgu.o: cannot link object files with different floating-point ABI from /tmp/rustcnFzdMx/symbols.o.

I was expecting it to successfully compile a binary that uses the double-precision standard extension to the riscv ABI.

I see that this PR changed the behavior so that the float part of the ABI comes from llvm_abiname, and only the RVC part comes from target-feature. Is this the intended behavior? I read on reddit (I know, it isn't canon) that target-feature is the say we're supposed to use the standard riscv extensions.

Meta

rustc --version --verbose:

rustc 1.76.0-nightly (2c1b65ee1 2023-11-11)
binary: rustc
commit-hash: 2c1b65ee1431f8d3fe2142e821eb13c623bbf8a0
commit-date: 2023-11-11
host: x86_64-unknown-linux-gnu
release: 1.76.0-nightly
LLVM version: 17.0.4
@jneem jneem added the C-bug Category: This is a bug. label Nov 12, 2023
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Nov 12, 2023
@bjorn3
Copy link
Member

bjorn3 commented Nov 12, 2023

The standard library for riscv32imc-unknown-none-elf is compiled without the d target feature, but needs to be linked in. You have to recompile the standard library with the d target feature enabled using eg cargo build -Zbuild-std=core --target riscv32imc-unknown-none-elf

Edit: Yeah, llvm_abiname needs to be changed too. You would need to use a custom target. For example (untested):

{
  "arch": "riscv32",
  "atomic-cas": false,
  "cpu": "generic-rv32",
  "crt-objects-fallback": "false",
  "data-layout": "e-m:e-p:32:32-i64:64-n32-S128",
  "eh-frame-header": false,
  "emit-debug-gdb-scripts": false,
  "features": "+m,+c,+d",
  "linker": "rust-lld",
  "linker-flavor": "gnu-lld",
  "llvm-target": "riscv32",
  "llvm-abiname": "ilp32d",
  "max-atomic-width": 0,
  "panic-strategy": "abort",
  "relocation-model": "static",
  "target-pointer-width": "32"
}

You can put this in a .json file and then pass the path to it as --target argument.

@bjorn3 bjorn3 added A-linkage Area: linking into static, shared libraries and binaries O-riscv Target: RISC-V architecture and removed A-linkage Area: linking into static, shared libraries and binaries labels Nov 12, 2023
@jneem
Copy link
Contributor Author

jneem commented Nov 12, 2023

Thanks for the quick response! I guess this is already addressed in your edit, but just for completeness I did try adding build-std in .cargo/config.toml, and cargo build still gives the same linking error.

And thanks for the example target! I can confirm that this allows the build to complete, and the resulting binary has the double-float ABI. I still think that the compiler should "just work" when the +d feature is added, in the same way that it "just works" when +c is added.

My (non-working) .cargo/config.toml:

[build]
rustflags = [ "-C", "target-feature=+d" ]

target = "riscv32imc-unknown-none-elf"

[unstable]
build-std = ["core"]

@bjorn3
Copy link
Member

bjorn3 commented Nov 12, 2023

+c and +m don't affect the ABI while +f and +d do. On x86 target features affect the abi without the linker safeguard that you hit here and that gives a lot of pain which we are currently trying to fix: https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/Pre-RFC.20discussion.3A.20Forbidding.20SIMD.20types.20w.2Fo.20features and https://hackmd.io/e4bYAMh2RWG2yKZHivmF9Q

@jneem
Copy link
Contributor Author

jneem commented Nov 12, 2023

Makes sense, thanks for the links!

@jneem jneem closed this as completed Nov 12, 2023
@RalfJung
Copy link
Member

Yeah, riscv32imc-unknown-none-elf is a softfloat target. You can't just turn a softfloat target into a hardfloat target with -Ctarget-faeture, the difference between those targets is too big. If you need a hardfloat variant of this target, the only solution is a new target.

We should ideally already error about this on the Rust side so you don't just get a linker error; that is being worked on in the threads linked above.

@saethlin saethlin removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Nov 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. O-riscv Target: RISC-V architecture
Projects
None yet
Development

No branches or pull requests

5 participants