add notes about non-compliant FP behavior on 32bit x86 targets

[RalfJung]

Based on ton of prior discussion (see all the issues linked from rust-lang/unsafe-code-guidelines#237), the consensus seems to be that these targets are simply cursed and we cannot implement the desired semantics for them. I hope I properly understood what exactly the extent of the curse is here, let's make sure people with more in-depth FP knowledge take a close look!

In particular for the tier 3 targets I have no clue which target is affected by which particular variant of the x86_32 FP curse. I assumed that i686 meant SSE is used so the "floating point return value" is the only problem, while everything lower (i586, i386) meant x87 is used.

I opened #114479 to concisely describe and track the issue.

Cc @workingjubilee @thomcc @chorman0773 @rust-lang/opsem
Fixes #73288
Fixes #72327

[rustbot]

r? @JohnTitor

(rustbot has picked a reviewer for you, use r? to override)

[chorman0773]

Note that even on targets with SSE enabled, extern "C"(et. al) functions at a minimum will still return in the x87 stack because of the abi. I don't know if you can rely on llvm not generating x87 ops in that case (even behind a potentially inlined extern "Rust" function), because moving from SSE to x87 requires spilling to memory and reloading.

[RalfJung]

My assumption was that this would mangle returned NANs but not affect computation/rounding, i.e. SSE is used for all arithmetic. IIRC this is what has been stated in prior discussions.

[JohnTitor]

r? @workingjubilee @thomcc

[chorman0773]

My assumption was that this would mangle returned NANs but not affect computation/rounding, i.e. SSE is used for all arithmetic.

https://godbolt.org/z/av8PK14j9
Apparently it does. Interesting. This seems suboptimal, though, due to the requirement of moving back into memory.

[RalfJung]

It seems pretty good from a "get sensible predictable behavior" perspective so I'd say we take this and go complain if they ever change it. ;)

[bors]

☔ The latest upstream changes (presumably #113049) made this pull request unmergeable. Please resolve the merge conflicts.

[thomcc]

Did a quick check and this to be right everywhere (and if I'm mistaken people can file followups), so it seems fine to me now.

Assuming we don't need lang or opsem approval for this (my hunch is we don't as this simply documents the current state of the world, and does not introduce any guarantees), you can r=me after you fix the conflicts.

[RalfJung]

Looking at #72327, turns out that even on i686-unknown-linux-gnu one can run into the "no-SSE" version of the problem by setting -C target-cpu=pentium. What would be the best way to reflect this in the target docs?

[Muon]

That issue is straight up a codegen bug, cf. llvm/llvm-project#44218.

Also, re: the note, loading a NaN onto the x87 stack only destroys its payload if it's a signaling NaN.

Do these changes preclude future improvements to the Rust calling convention to avoid the x87 stack for function returns when SSE registers are available?

Also with the exception of the C calling convention making it almost impossible to return an sNaN, the desired semantics are entirely implementable. I was actually planning on starting to fix that stuff in LLVM once I return to my desk in August.

[chorman0773]

Do these changes preclude future improvements to the Rust calling convention to avoid the x87 stack for function returns when SSE registers are available?

Rust Compilers can general do whatever they want with extern "Rust", and returning in x87 vs. SSE isn't an exception. One issue that implicates forcing SSE, though, is #[target_feature(enable="sse")] vs. -C target-feature=sse.

I'd prefer if this isn't necessarily required, though.

[RalfJung]

That issue is straight up a codegen bug, cf. llvm/llvm-project#44218.

Well, codegen is using the target FP operations, but those just don't have the right behavior. As far as I understand this is a long-standing problem affecting pretty much all C compilers on that target, which is why I think it is a target bug.

Do these changes preclude future improvements to the Rust calling convention to avoid the x87 stack for function returns when SSE registers are available?

No, we can always improve a target's compliance later. Even a custom Rust calling convention won't entirely solve the problem though as the C calling convention can still be used for Rust-to-Rust calls.

I was actually planning on starting to fix that stuff in LLVM once I return to my desk in August.

That would be great, being able to remove these footnotes would make me very happy. :) I am proposing we add them because this has been the situation for years and it's pretty much outside our control -- at least that was my impression.

[Muon]

Yes, the problems are fundamentally LLVM bugs and not Rust's fault beyond using a buggy backend. However, note that the target operations aren't completely at fault. Both issues can be fixed by setting the precision flag correctly. In fact, those specific code samples won't reproduce the bug on Windows, since the MSVCRT sets the x87 unit to 53-bit precision, which matches the destination.

[chorman0773]

Isn't modifying the FPCW slow off of Windows, since IIRC you have to reset the precision whenever you call another function.

[Muon]

I'm given to understand that changing the control word is somewhat slow, but not fatally so. However, code typically doesn't use multiple precisions at once, and predictability and correctness trump performance regardless. If you want both top performance and predictability on pre-SSE hardware, you should stick to 80 bit floats anyway. You'll get better numerical results too. Regarding calling conventions, I was pretty sure that none of them specified the state of the control word?

[chorman0773]

Sys-V relies on it being 80-bit to do long double computations (and to satisfy #define FLT_EVAL_METHOD 2), so I'd expect it specifies the eval precision field to be 80-bit (haven't checked, haven't seen Sys-V x86 in forever).

predictability and correctness trump performance regardless

That being said, it seems like a considerable pessimization to me, as having to change the control word twice arround function calls adds up.

[Muon]

According to https://raw.githubusercontent.com/wiki/hjl-tools/x86-psABI/intel386-psABI-1.1.pdf and https://gitlab.com/x86-psABIs/i386-ABI/-/blob/hjl/master/low-level-sys-info.tex the callee (not the caller) must preserve the control word across calls. In theory, this pessimization should already be in effect.

[chorman0773]

It also gives the state of the register on startup, but doesn't say anything about it's value on function entry. That being said, compilers do assume the state of the fpcw. Otherwise, this code would be compiled wrong.

[Muon]

To be clear, it is in fact compiled wrong according to that document, at least if the compiler claims to implement the i386 SYSV ABI. Section 2.2.1 Registers, page 10 says "The x87 status word register is caller-saved, whereas the x87 control word is callee-saved.". Additionally, the last row of table 2.3 (Register usage) also says that the callee must preserve the control word.

[chorman0773]

Being callee-saved vs. caller-saved is moot unless the function modifies the value of the register.

[RalfJung]

One thing that's currently not mentioned is that even on some tier 1 targets, with the right flags you can run into the x87 issue -- e.g. by setting -C target-cpu=pentium on a i686 target, which disables SSE2. I'm not sure what the status is of such flags (is that even still a tier 1 target?) and how to best put this into the docs.

[bors]

☔ The latest upstream changes (presumably #115769) made this pull request unmergeable. Please resolve the merge conflicts.

[workingjubilee]

Apologies, allow me to catch up...

[Muon]

Does #115919 affect anything in here? I've lost track...

[workingjubilee]

@RalfJung r=me with this rebased, if we have to correct this again then we have to correct this again.

[RalfJung]

Does #115919 affect anything in here? I've lost track...

That PR will partially fix #115567, but only for "Rust" ABI functions. So when #115919 lands we can update the footnotes to say that this only applies to non-Rust ABIs.

[RalfJung]

Two more i686 targets have been added recently:

• i686-pc-windows-gnullvm
• i686-unknown-hurd-gnu

I assume both of these have SSE and are thus affected by the return-val issue but not the x87-precision issue.

[RalfJung]

@bors r=workingjubilee rollup

[bors]

📌 Commit df911df has been approved by workingjubilee

It is now in the queue for this repository.