Ensure floats are returned losslessly by the C ABI on 32-bit x86 - #161950
Open
beetrees wants to merge 1 commit into
Open
Ensure floats are returned losslessly by the C ABI on 32-bit x86#161950beetrees wants to merge 1 commit into
beetrees wants to merge 1 commit into
Conversation
rustbot
added
A-LLVM
Collaborator
|
|
This comment has been minimized.
This comment has been minimized.
Contributor
|
@bors delegate try |
Contributor
|
✌️ @beetrees, you can now perform try builds on this pull request! You can now post |
beetrees
force-pushed
the
x86-ret-snan-c
branch
from
August 28, 2026 19:50
9749c87 to
477f3c8
Compare
Contributor
Author
|
@bors try |
This comment has been minimized.
This comment has been minimized.
rust-bors Bot
pushed a commit
that referenced
this pull request
Aug 28, 2026
Ensure floats are returned losslessly by the C ABI on 32-bit x86 try-job: i686-*
Collaborator
|
The job Click to see the possible cause of the failure (guessed by this bot) |
Contributor
RalfJung
reviewed
Aug 29, 2026
| C: HasDataLayout + HasTargetSpec, | ||
| { | ||
| if !fn_abi.ret.is_ignore() { | ||
| // "vectorcall" returns floats in `xmm0`, and soft float also does not use the x87 stack. |
Member
There was a problem hiding this comment.
vectorcall does not show up in the code below, so this comment is a bit confusing.
Comment on lines
17
to
19
| // FIXME(#114479): LLVM miscompiles loading and storing `f32` and `f64` when SSE is disabled on | ||
| // x86. | ||
| if cfg!(not(all(target_arch = "x86", not(target_feature = "sse2")))) { |
Member
There was a problem hiding this comment.
Shouldn't the FIXME and if be removed as this PR is supposed to fix exactly that?
| /// 32-bit x86. This is needed as LLVM needs to generate extra code to ensure that signalling | ||
| /// NaNs are passed losslessly on the x87 floating point stack. Only valid on return types on | ||
| /// 32-bit x86 with a cast target of either `Reg::f32()` or `Reg::f64()`. | ||
| pub x87_floating_point_stack: bool, |
Member
There was a problem hiding this comment.
This should mention that the LLVM-level type is then changed to x86_fp80.
RalfJung
reviewed
Aug 29, 2026
| // any initialized bytes within a partially uninitialized value survive the round trip, | ||
| // freeze the value. | ||
| let value = if no_undef { value } else { self.freeze(value) }; | ||
| let is_nan = self.fcmp(RealPredicate::RealUNO, value, value); |
Member
There was a problem hiding this comment.
Wouldn't the ideal test be "is it a signaling NaN"? Or do normal NaNs also get garbled?
Though I can imagine that that's annoying enough to implement that it's not worth it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The x86 C ABI returns
f32andf64using the x87 extended precision floating point format (henceforthx86_fp80) on the x87 floating point stack. Currently, LLVM uses regularfld/fstpinstructions to convertf32/f64to and fromx86_fp80when returning from a function with the C ABI, however these instructions perform floating point format conversions that quieten signalling NaNs, which breaks Rust's (and LLVM's) guarantees. This is a long-standing LLVM bug (llvm/llvm-project#66803) but is difficult to fix on the LLVM side as it ties into larger problems with the way LLVM handles x87 registers holdingf32/f64, combined with the general lack of developer interest in 32-bit x86 without SSE (although the ABI bug specifically also affects targets with SSE2 enabled). This problem has been tracked on the Rust side in #115567, with #123351 ensuring that Rustic ABIs avoid the x87 stack altogether and #113053 adding notes to the platform support page detailing how the targets are non-complaint (including the tier 1 targetsi686-unknown-linux-gnuandi686-pc-windows-msvc).This PR fixes #115567 by manually converting NaNs to and from
x86_fp80when a function returnsf32/f64(this was briefly discussed in #t-compiler > `x87_f80` is weird @ 💬).r? @tgross35
cc @RalfJung
try-job: i686-*