The ABI of float types on can be changed by -Ctarget-feature
RalfJung opened this issue · 36 comments
A function that returns an f32/f64 is not ABI-compatible with other functions that have the same signature on i686 when certain target features differ. It looks like one can disable the x87 feature or enable the soft-float and then it will use different ways of passing floating-point arguments.
This is unsound as code calling methods from the standard library would now use the wrong registers to return results.
Fixing this properly will require identifying all target feature for all our targets that can affect the ABI of float values. (For target features affecting the ABI of SIMD vectors, see #116558. For target features affecting other aspects of the ABI it is probably best to file a new issue.)
- x86: uses float registers if
!soft-float && x87, so toggling either can be unsound - arm: uses float registers if
!soft-float && fpregs, so toggling either can be unsound - aarch64: uses float registers if
fp-armv8; Rust makes-neonimply-fp-armv8so we have to forbid both -- butneonis stable! See #131058 - hexagon
- powerpc
- mips
- riscv
- wasm
- bpf
- csky
- loongarch
- s390x
- more?
Cc @rust-lang/opsem
FTR, I think the -x87,+softfp and -x87,+sse codegens are wrong for at least the C abi, because Sys-V (and msabi) do prescribe that float/double are returned in st(0) and provide no other alternative - so I think rustc should reject this code in particular.
I actually wanted a similar prohibition retroactively on the simd types, but the x86_64-psabi list did not accept that request.
If I understand @chorman0773 correctly #115476 (comment), then a function that takes/returns an f32/f64 is not ABI-compatible with other functions that have the same signature on i686 when certain target features differ. It looks like one can disable the x87 feature and then it will use different ways of passing floating-point arguments.
This is correct.
So IMO we should consider certain features to be in the required baseline for i686 targets and just error out when they get disabled (or force-enable them, or refuse to codegen things involving floats, or something like that) -- in particular, x87 and sse2.
I believe I have vocalized that this is my desired solution as well.
Demonstrating the 3 different ways that rustc returns floats on x86: https://rust.godbolt.org/z/r83MbYh5n.
Although it seems f64 specifically is spared on sse and softfp (not between +x87 and -x87 though). Both cases it's returned in edx:eax (which is weird, I'd expect f64 to get returned in an xmm register otherwise).
So IMO we should consider certain features to be in the required baseline for i686 targets and just error out when they get disabled (or force-enable them, or refuse to codegen things involving floats, or something like that) -- in particular, x87 and sse2. Currently it may seem like --target i686-unknown-linux-gnu -C target-feature=-sse2,-sse is a tier 1 target but really it isn't.
Force enabling them (or blanket erroring) on i686-wide would affect kernel mode code that typically disables the FPU and vector extensions to avoid having to save that state every context switch. Refusing to codegen floats is a reasonable alternative, though. For sse in particular, llvm really loves to copy data arround using xmm registers, so this will either cause a #GP(0) when llvm starts putting movupss everywhere, or worse, silently clobber xmm registers when the kernel does something even cleverer with cr4.OSFXSAVE/cr4.OSXSAVE enabled.
Kernel-friendly targets need to be handled specially as always.
I was about to say this didn't need O-x86_64, but...
https://rust.godbolt.org/z/EzMhdsqx9
I just realized that with #[target_feature] we don't allow disabling features at all. That makes me quite surprised that we allow disabling features on stable with -C target-feature... was that a deliberate mismatch?
Force enabling them (or blanket erroring) on i686-wide would affect kernel mode code that typically disables the FPU and vector extensions to avoid having to save that state every context switch.
Force enabling them wouldn't affect that code if it doesn't use any floats. :)
Force enabling them wouldn't affect that code if it doesn't use any floats. :)
It does though, especially sse as mentioned.
llvm will happily fold a 16 byte 4 dword mov into movupss, which will #GP(0) if cr4.OSFXSAVE=0.
It does though, especially sse as mentioned.
It does?
llvm will happily fold a 16 byte 4 dword mov into movupss, which will #GP(0) if cr4.OSFXSAVE=0.
Bless you? To me it looks like you put the output of pwgen into the editor. ;) Can you explain this in higher-level terms?
If you tell llvm that it can use sse instructions, it will completely decide to fold scalar bytewise copies into sse copies and cause a general protection exception in kernel code that isn't configured to allow those instructions.
This is why it's considered undefined behaviour to merely enter code with an unavailable feature available.
Example of llvm using movups rather than scalar copies with sse enabled: https://rust.godbolt.org/z/jW8W54sc9
If you tell llvm that it can use sse instructions, it will completely decide to fold scalar bytewise copies into sse copies and cause a general protection exception in kernel code that isn't configured to allow those instructions.
Ah, bummer.
That sounds like we want -softfloat/-nofloat targets then. But disabling target features seems to have all sorts of bad side-effects and I wish we never allowed it -- and I wonder to what extend we can take it back...
Disabling target features is incredibly useful when writing all kinds of code. Kernel and driver code especially, but I write a lot of "Low-level user mode code" that also somestimes requires finagling with -C target-feature and -C target-cpu.
And sometimes you live before the kernel. A bootloader gratuitously opting arbitrary kernels into cr4.OSXFSAVE is even worse, because the instructions won't trap, just silently clobber user mode state.
You are describing a good motivation for a -nofloat/-softfloat target. In fact, we have some -softlofat targets.
You are not describing why we should offer the ability to disable target features, when perfectly valid alternatives exist; alternatives that do not also eat your kitttens. "It is useful" applies to many things that we very deliberately do not let people do because they just cause too many issues.
It is still possible to obtain what is desired for those by switching to an enable-only process, or virtually so (I realize that softfloat is technically a feature one must often disable to get correct codegen).
Note: #115919 would make this apply by toggling sse and not x87, which can be done without disabling any features on i586 targets.
#115919 could be adjusted to only kick in when the baseline features of the target include SSE. If we do that, does enabling SSE ever affect the ABI of f32/f64? If the answer is "no" then I think the i586 targets are good, right?
For softfloat targets, we'd have to ensure their f32/f64 ABI is unaffected by enabling x87 or SSE, or we have to reject enabling those features. The former should actually be possible, right? I would assume f32 is passed much like i32 and f64 like i64 on those targets, so we can tell LLVM to pass floats as i32/i64 and then we don't have to worry about target features at all?
Naive question: How far are we from being able to define extern "Rust+softfp" and extern "Rust+hardfp" and gate calling them on the static ABI plus #[target_feature]?
Naive question: How far are we from being able to define extern "Rust+softfp" and extern "Rust+hardfp" and gate calling them on the static ABI plus #[target_feature]?
One sufficiently motivated contributor away, I would say? ;) Though I don't know how good LLVM would support that.
But that doesn't really answer what should happen with all the other ABIs. I see only two sensible approaches:
- we entirely forbid disabling the x87 feature (a la #116584)
- when the x87 feature is missing, we reject functions and function calls that have a float-type by-value (similar to this for vector types, but we need to check all ABIs, maybe with the exception of the ons that explicitly say softfloat/hardfloat)
when the x87 feature is missing, we reject functions and function calls that have a float-type by-value (similar to #116558 (comment) for vector types, but we need to check all ABIs)
This will end up needing a big table that determines, for each target, which feature needs to be present to pass floats as arguments/return values. In fact, for some targets this will say which features need to be absent -- on softfloat targets, we have to ensure that the float feature does not get enabled! (If LLVM provided better control over the ABI used by call instructions, we could let users enable the x87 feature but not use it for passing arguments. But I don't think that is possible in LLVM currently.)
RISC-V has a similar ABI split. -F/-D/-Q is your softfloat/nofloat, but it also comes with the Zfinx/Zdinx/Zqinx variants where floating-point values are carried in the regular registers and the floating-point register file is deleted.
Your float-function-features would be +F,-Zfinx, +D,-Zdinx for riscv64gc-unknown-linux (linux does not permit finx). Although I don't think this is as much of a problem because the platform states that +F,+Zfinx is illegal?
I just found #63466... so +softfloat is a thing. We have target features that are subtractive?!?? This pit of despair has no bottom.^^
We obviously need to also reject enabling such features then.
If I understand this assembly correctly, then disabling the sse feature on x86-64 also changes the ABI of our float types there. Or is that just loading the on-stack float argument into different registers (x87 vs SSE register)?
Some more experimentation... if I understand this correctly, then +softfloat doesn't change the ABI, only -x87 does. (I wish there was a way to have LLVM describe the computed ABI to me, so that one doesn't have to reverse engineer that from the assembly.^^)
I don't think I understand the softfloat target feature at all; here the left compiler uses x87 instructions despite +softfloat and the right compiler uses softfloats (call __addsf3@PLT) even though that target feature is not set.
Oh d'oh, it's soft-float, not softfloat.
Yeah when both x87 and soft-float are set it seems to use the softfloat ABI.
if you use thumbv7em-none-eabi it passes the floats in r0, r1, ... and uses soft float to add them: https://godbolt.org/z/z5do34E81
if you use thumbv7em-none-eabi but add -Ctarget-feature=+vfp4 it still passes the floats in r0, r1, ... so it's ABI-compatible, but uses the FPU instructions to add them. https://godbolt.org/z/v9Wqoq74s
if you use thumbv7em-none-eabihf it passes floats in the FPU registers (s0, s1...) and uses the FPU to add them. https://godbolt.org/z/3rEn13MnE
And what happens when one disables that target feature on a hard-float target?
hehe, it seems it passes floats in FPU regs, but moves them out to do soft-float maths. So it seems to be correct :) https://godbolt.org/z/4zh9fbE81
+soft-float does cause the ABI to change tho... 😭 https://godbolt.org/z/eT5sf7fec
hehe, it seems it passes floats in FPU regs, but moves them out to do soft-float maths. So it seems to be correct :) https://godbolt.org/z/4zh9fbE81
That's awesome. :-) Much better than x86 where the x87 target feature changes ABI...
+soft-float does cause the ABI to change tho... 😭
Yeah, that's kind of expected (IMO a bad design decision by LLVM, but oh well). #129884 will make it an error to toggle soft-float via -Ctarget-feature.
Actually -Ctarget-feature=-vfp2sp,-fpregs also changes the ABI. https://godbolt.org/z/9747PYa1b
That seems a closer equivalent to -x87 in x86. Seems LLVM separates "FPU instructions" and "FPU regs", though both need the hardware to have a FPU.
Ah okay, damn. So we need to also block the fpregs target feature then.
WG-prioritization assigning priority (Zulip discussion).
@rustbot label -I-prioritize +P-high
I tried to figure out where in the LLVM sources this happens so that we can be sure to check all the right target features. However, I am a bit stuck...
For ARM, we found soft-float and fpregs can affect the float ABI, and that matches what I see here. However, for x86, things look much less clear... the file is this but GH doesn't show its contents. The (I think) relevant part is
bool UseX87 = !Subtarget.useSoftFloat() && Subtarget.hasX87();
// ...
if (!Subtarget.useSoftFloat() && Subtarget.hasSSE2()) {
// f16, f32 and f64 use SSE.
// Set up the FP register classes.
addRegisterClass(MVT::f16, Subtarget.hasAVX512() ? &X86::FR16XRegClass
: &X86::FR16RegClass);
addRegisterClass(MVT::f32, Subtarget.hasAVX512() ? &X86::FR32XRegClass
: &X86::FR32RegClass);
addRegisterClass(MVT::f64, Subtarget.hasAVX512() ? &X86::FR64XRegClass
: &X86::FR64RegClass);
// ...
} else if (!Subtarget.useSoftFloat() && Subtarget.hasSSE1() &&
(UseX87 || Is64Bit)) {
// Use SSE for f32, x87 for f64.
// Set up the FP register classes.
addRegisterClass(MVT::f32, &X86::FR32RegClass);
if (UseX87)
addRegisterClass(MVT::f64, &X86::RFP64RegClass);
// ...
} else if (UseX87) {
// f32 and f64 in x87.
// Set up the FP register classes.
addRegisterClass(MVT::f64, &X86::RFP64RegClass);
addRegisterClass(MVT::f32, &X86::RFP32RegClass);
// ...
}This can't be the ABI logic, right? "f16, f32 and f64 use SSE" refers to how FP math is computed. But how does the ABI logic decide whether to use float registers or general-purpose registers?
@nikic do you happen to you the right part of the code for this?
...people edit a 55,000 line file by hand?