rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
94.86k stars 12.23k forks source link

linking with C library changes the behavior of _Float16 #118813

Open usamoi opened 6 months ago

usamoi commented 6 months ago

I tried this code:

[build-dependencies]
cc = "1.0.83"
// /build.rs
fn main() {
    cc::Build::new()
        .compiler("/usr/bin/clang-16")
        .file("./src/abc.c")
        .opt_level(0)
        .compile("abc");
}
// /src/abc.c
float test() {
  _Float16 y = 114;
  float z = y;
  return z;
}
// /src/main.rs
extern "C" {
    fn test() -> f32;
}

fn main() {
    println!("{}", unsafe { test() });
}

I expected to see this happen: It prints 114.

Instead, this happened: It prints an unknown value, 0.00017929077, -0.045898438 or other.

After debugging, I found both compiler_builtins and libgcc provide the symbol __extendhfsf2. __extendhfsf2 is a compiler-rt intrinsics for casting a _Float16 to float. In compiler_builtins, the only argument of __extendhfsf2 is passed in a general-proposed register, However, in libgcc, the only argument of __extendhfsf2 is passed in xmm.

related: https://github.com/llvm/llvm-project/issues/56854

Meta

rustc --version --verbose:

rustc 1.74.1 (a28077b28 2023-12-04)
binary: rustc
commit-hash: a28077b28a02b92985b3a3faecf92813155f1ea1
commit-date: 2023-12-04
host: x86_64-unknown-linux-gnu
release: 1.74.1
LLVM version: 17.0.4
Jules-Bertholet commented 6 months ago

@rustbot label A-floating-point A-linkage I-unsound

Nilstrieb commented 6 months ago

cc @Amanieu for compiler builtins shenanigans

nikic commented 6 months ago

We probably need to se COMPILER_RT_HAS_FLOAT16 during our compiler-rt build.

apiraino commented 6 months ago

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-high

Amanieu commented 6 months ago

We probably need to se COMPILER_RT_HAS_FLOAT16 during our compiler-rt build.

Yes, I think that would solve the problem. Is _Float16 supported by Clang on all targets? Or do we need some sort of per-target allowlist?

sagudev commented 4 months ago

Is _Float16 supported by Clang on all targets? Or do we need some sort of per-target allowlist?

I looks like it's not available on all targets: https://clang.llvm.org/docs/LanguageExtensions.html#half-precision-floating-point

tgross35 commented 2 months ago

@beetrees put up https://github.com/rust-lang/compiler-builtins/pull/593 which uses our nightly f16 type to provide the extend/trunc symbols and should fix this. Need to double check the edge cases because the ABI (integer or float) is apparently dependent on whether SSE2 is available.

beetrees commented 2 months ago

In general, LLVM will call builtins with the default C ABI, so any target features that affect the ABI (SSE2, soft-float, etc.) need to be the same when compiling compiler-builtins/compiler-rt as when compiling user code to avoid miscompilations. _Float16 is only available in clang and gcc when SSE2 is enabled, so as long as compiler-builtins was built with SSE2 enabled (as it is by default for all the not-bare-metal-or-UEFI i686-* and x86_64-* targets) the ABI should line up.

Related: #116558