linking with C library changes the behavior of _Float16

[usamoi]

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: llvm/llvm-project#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]

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

[Noratrieb]

cc @Amanieu for compiler builtins shenanigans

[nikic]

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

[apiraino]

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-high

[Amanieu]

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]

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]

@beetrees put up rust-lang/compiler-builtins#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]

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

[Veykril]

Do I understand this right that this has been fixed by #125016?

[beetrees]

Correct. Specifically, rust-lang/compiler-builtins#593 means that compiler-builtins no longer (incorrectly) compiles the compiler-rt versions of the builtins.