compiler-builtins icon indicating copy to clipboard operation
compiler-builtins copied to clipboard

target-feature=+soft-float breaks things

Open m-ou-se opened this issue 3 years ago • 3 comments

A crate compiled on x86_64 with -Ctarget-feature=+soft-float will use floating point conversions from compiler-builtins. However, some of the floating point conversions in compiler-builtins are implemented using floating point instructions. This breaks when enabling the +soft-float target-feature. Should +soft-float be supported? Or should this have given an error? Right now, it silently gives wrong results:

fn main() {
    println!("1234 = {:e}", a(1234));
}

#[inline(never)]
fn a(x: u64) -> f64 {
    x as f64
}
$ rustc main.rs -Ctarget-feature=+soft-float && ./main 
1234 = 4.66101421257866e-310

https://godbolt.org/z/9zvecP

m-ou-se avatar Aug 18 '20 13:08 m-ou-se

Should +soft-float be supported? Or should this have given an error?

Could go either way. On one hand the SSE hardware is guaranteed to exist on x86_64, on the other hand it could be disabled (such as is the case during very early boot). Doubt that's an useful enough use-case to support and spend time on though.

nagisa avatar Aug 18 '20 13:08 nagisa

I suppose +soft-float would be used when compiling kernel code that shouldn't touch floating point registers. Not sure of all the other use cases though.

In any case, silently producing wrong results is very bad™. Not sure how to turn this into an error though, or even what part would be responsible for producing that error.

Looks like the compiler-builtins and core (etc.) should be compiled with the same target-features as the crate using it. Even with pure soft implementations, the return value still goes through a floating point register. +soft-float expects floats to be returned in a regular register (effectively changing the ABI). The only reason most conversions seem to work on x86_64, is because both compiler-builtins and core use xmm0 of the -soft-float ABI, which is left untouched by the +soft-float crate. So, interacting with the value directly from a +soft-float crate breaks, even when formatting it unmodified (using core) 'works':

fn main() {
	assert_eq!(a(1234), 1234.0);
}

#[inline(never)]
fn a(x: u32) -> f32 {
	x as f32
}
$ rustc main.rs -Ctarget-feature=+soft-float && ./main 
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `1234.0`,
 right: `1234.0`', main.rs:2:5

So this is probably a rust issue instead of a compiler-builtins issue.

m-ou-se avatar Aug 18 '20 14:08 m-ou-se

So this is probably a rust issue instead of a compiler-builtins issue.

Agreed. Interestingly, just copying in the compiler-builtins Rust implementation into the same file seems to have everything work (https://godbolt.org/z/Tqxd87), so it's probably just the fact that libcore is built with -soft-float. I don't think we build a fallback C implementation for __floatundidf, but I might be wrong about that.

It would be interesting to see if this still happens when doing RUSTFLAGS="-C target-feature=+soft-float" cargo build -Zbuild-std. Unfortunately, libcore for the x86_64-unknown-linux-gnu target seems to require -soft-float (otherwise you get LLVM errors).

The solution is to probably just ban +soft-float if you're using a precompiled libcore.

I suppose +soft-float would be used when compiling kernel code that shouldn't touch floating point registers. Not sure of all the other use cases though.

That's the only use I've seen on x86_64, but virtually all Rust OS kernels set this flag, including https://github.com/cloud-hypervisor/rust-hypervisor-firmware. So I agree that +soft-float is a use case we should care about.

josephlr avatar Oct 02 '20 08:10 josephlr