add `core::arch::{x86, x86_64}::x87_f80` by folkertdev · Pull Request #163992 · rust-lang/rust

folkertdev · 2026-10-08T14:12:16Z

Add x87_f80 in the same way as f16b and ppcf128.

This changes the signature of size because the size of x87_f80 depends on the target (12 bytes on 32-bit targets, 16 bytes on 64-bit targets).

It also breaks an invariant that scalars are the size of their fields: the x87_f80 scalar is 80 bits, but the field is the aforementioned 12 or 16 bytes. Let's hope we don't subtly rely on that somewhere.

It's unclear what to do with this type on softfloat targets. Clang and GCC reject the type when the x87 registers are not available. We should do the same, probably just with a cfg?

I don't think we have a good cfg for this though? x87 is still set on softfloat targets, while soft-float doesn't get set (that does not give a warning even, it probably should?). We also don't set cfg_abi.

future work

the ABI of Complex<x87_f80> has not yet been implemented
x87_f80 does not yet implement VaArgSafe

folkertdev · 2026-10-08T19:07:19Z

r? beetrees
cc @tgross35 @RalfJung

rustbot · 2026-10-08T19:07:28Z

stdarch is developed in its own repository. If possible, consider making this change to rust-lang/stdarch instead.

cc @Amanieu, @sayantn

Some changes occurred in compiler/rustc_attr_ir

cc @jdonszelmann, @JonathanBrouwer

rustc_codegen_cranelift is developed in its own repository. If possible, consider making this change to rust-lang/rustc_codegen_cranelift instead.

cc @bjorn3

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

This PR changes rustc_public

cc @oli-obk, @celinval, @ouz-a, @makai410

rustc_codegen_gcc is developed in its own repository. If possible, consider making this change to rust-lang/rustc_codegen_gcc instead.

cc @antoyo, @GuillaumeGomez

folkertdev · 2026-10-08T19:09:23Z

@bors try jobs=test-various,test-i686-*

add `core::arch::{x86, x86_64}::x87_f80` try-job: test-various try-job: test-i686-*

rust-bors · 2026-10-08T21:43:58Z

☀️ Try build successful (CI)
Build commit: 4923e95 (4923e95bc1c5973505d6b35dcddbc97d11ae69e5)
Base parent: 65efa19 (65efa194201c67ba29b7384255a534d61c2aa3cb)

RalfJung · 2026-10-09T08:28:39Z

This changes the signature of size because the size of x87_f80 depends on the target (12 bytes on 32-bit targets, 16 bytes on 64-bit targets).

"f80" is 128 bits large on 64-bit targets...?!?? How does that make any sense?

It also breaks an invariant that scalars are the size of their fields: the x87_f80 scalar is 80 bits, but the field is the aforementioned 12 or 16 bytes. Let's hope we don't subtly rely on that somewhere.

I'd be surprised if we didn't. That's been pretty much part of the definition of being a scalar -- if a type has padding, it is not a scalar type. The interpreter was built around that assumption.
Cc @scottmcm who recently generalized what a scalar can be, but even with that I don't think it can have padding.

It does not sound to me like this type can be a scalar, then.

RalfJung · 2026-10-09T08:32:01Z

+                // For `x87_f80` the primitive is 80 bits, but the field (after rounding up to
+                // the alignment) is 12 bytes (on 32-bit systems) or 16 bytes (64-bit).
+                // So we can't assert equality of size and layout.size.
+                assert_eq!(
+                    size.align_to(s.default_align(self).abi),
+                    mplace.layout.size,
+                    "abi::Scalar is larger than its layout"
+                );


The entire logic for dealing with scalars throughout the interpreter assumes that they have no padding. Changing this assertion is not enough, it just means the rest of the interpreter is now more or less silently wrong. Pretty much all codepaths that deal with scalars need to be audited. Same for every other MIR consumer.

I would strongly prefer if we didn't break the fundamental concept of a "scalar".

View changes since the review

To give just one example, here's one place we make that assumption:

rust/compiler/rustc_const_eval/src/interpret/place.rs

Lines 792 to 794 in 6aaabbb

Immediate::Scalar(scalar) => {

alloc.write_scalar(alloc_range(Size::ZERO, scalar.size()), scalar)?;

}

You can't even see it. But if you look at the ScalarPair path you can see that we have to ensure that padding gets reset to uninit, and in the Scalar path we don't have to worry about that.

Also, the basic APIs for reading/writing stuff in an Allocation are read_scalar/write_scalar. This again uses the term "scalar" to refer to something that has no padding. Same for the type Scalar. And so on.

RalfJung · 2026-10-09T08:40:37Z

It would probably be possible, with a lot of effort, to fix all the places where we assume scalars have no padding. But I think that's a bad idea. It makes life harder for every MIR consumer. (Most likely it'll mean many MIR consumers will stay buggy as nobody will expect something as outlandish as this weird type.) This new type is the weird one here, we should not shape everything around a legacy type that hardly anyone needs and that has no justification to exist in the first place other than "it has always been here".

beetrees · 2026-10-09T08:53:07Z

I think it might make sense to introduce something like BackendRepr::PaddedScalar (name bikesheddable)? This would be useful for more than just x87_f80: e.g. currently we can (don't have my laptop to check if we currently do) represent #[repr(align(16))] struct A(u8, u8); as a BackendRepr::ScalarPair, whereas #[repr(align(16))] struct A(u8); currently has to be represented as BackendRepr::Memory as it has padding.

tgross35 · 2026-10-09T08:56:55Z

For Rust's purposes could we just pretend it's a MaybeUninit<u128> on x86-64 rather than treating it as a padded scalar?

RalfJung · 2026-10-15T21:05:56Z

+                if layout.backend_repr.is_x87_f80() {
+                    // The lang item wraps a byte array, so it is not a newtype of a scalar.
+                    return;
+                }


Seems like this type is still breaking basic MIR assumptions that have been true since forever and that code inside and outside the compiler is likely to rely on in subtle ways. Why that?

View changes since the review

Well we don't have any 80-bit scalars available to wrap transparently. So we'd have to make this type fully magic. But I guess f32 kind of is (like, it doesn't wrap [u8; 4] or u32) so maybe that could just work. I'll have to look into that.

RalfJung · 2026-10-15T21:07:20Z

+/// alignment, raising the total size to 12 or 16.
+///
+/// NOTE: this type MUST NOT be used anywhere except in the x87_f80 wrapper below.
+#[cfg_attr(any(target_arch = "x86", target_arch = "x86_64"), lang = "x87_f80_raw")] // for docs


What does "for docs" mean here? The attribute seems quite important not just for docs...?

View changes since the review

Right, it's the cfg_attr that is important for a docs build: we now error on encountering the lang item on non-x86 targets. Just to make sure we never trigger that in a doc build, never declare the lang item on non-x86 targets.

beetrees · 2026-10-16T23:17:26Z

With this new implementation, Rust type x87_f80_raw is size 10 but the LLVM (non-immediate) backend type x86_fp80 has an allocation size of 12/16. While in the source code it is only ever interacted with via Rust type x87_fp80, AFAICT it can at minimum still end up as the type of a local variable via MIR passes like SROA. This feels like something that could cause issues, although I haven't been able to come up with an example, so maybe not? Would changing the non-immediate backend type (llvm_type) to [10 x i8] for anything with BackendRepr::Scalar(Float::X87F80) work to solve this?

scottmcm · 2026-10-18T05:36:32Z

Cc @scottmcm who recently generalized what a scalar can be, but even with that I don't think it can have padding.

I was working on allowing different alignments, but as you say, not changing anything about adding padding or similar.

There's definitely a bunch of places that assume that Scalars of matching size are sound to transmute, for example. Changing that would affect all kinds of places and I agree that we shouldn't muck with that.

beetrees · 2026-10-28T16:15:50Z

+    // Map `core::arch::{x86, x86_64}::x87_f80` to LLVM's `x86_fp80`.
+    if Ty::is_x87_f80_lang_item(layout, cx) {
+        return cx.type_x87_f80();
+    }


x87_f80 is BackendRepr::Memory, so we should never be loading or storing its LLVM type. Instead, when uncached_llvm_type is fetching the type of the field of a struct or array, it should replace any occurrences of x87_f80_raw with [10 x i8]. This will ensure the returned struct always has the right size, which I AFAICT is the problem this is here to solve. Additionally, could you add an assert with FIXME to CastTarget::llvm_type ensuring it never returns an LLVM struct/array containing an LLVM x86_fp80 - load_cast/store_cast currently load/store the LLVM cast target aggregate directly (which is not generally a good idea for reasons like this, but that can be fixed in a separate PR).

beetrees · 2026-10-28T17:53:25Z

+/// The `x87_f80` wrapper type below corresponds to the alloc size: it applies the target-specific
+/// alignment, raising the total size to 12 or 16.
+///
+/// NOTE: this type MUST NOT be used anywhere except in the x87_f80 wrapper below.


This note shouldn't be necessary if everything is working properly. If it is, it suggests there's a deeper problem that needs to be fixed before this is merged. x87_f80 should be like Complex<T>: is a lang item for ABI purposes, but otherwise is a regular struct.

View changes since the review

I'm aware of two issues with the type when it's used standalone

the array stride is wrong, so getting second element out of a [x87_f80_raw; 2] reads at the wrong offset (and reads out of bounds)

the ABI calculation in callconv can't handle this type because is_x87_f80_lang_item returns false for it, you can trigger an ICE this way

As far as I can tell, neither of these issues can be observed from the outside when x87_f80_raw is private and only used as the element type of x87_f80.

So (how) should that be fixed?

Reliably ICEing when it comes up in the ABI seems fine.

Why is the array stride wrong?

Is the array stride issue fixed by #165146?

Yes, I just checked locally. Bors is having a rough time but I'll add a test once that merges.

beetrees · 2026-10-28T18:30:44Z

                16 => dl.f16_align,
                32 => dl.f32_align,
                64 => dl.f64_align,
+                80 => dl.x87_f80_align,


Given that (80 / 8) % dl.x87_f80_align != 0, could you add a comment here explaining why this is correct (I think it is as the align is only used to affect the overall alignment of the cast alloca, but we should document that fact somewhere to avoid confusion).

View changes since the review

rustbot · 2026-10-31T23:00:39Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

folkertdev

@rustbot ready

for another look, anyway.

View changes since this review

folkertdev · 2026-10-31T23:01:25Z

+    // CHECK: getelementptr inbounds nuw i8, ptr %arr, {{i32|i64}} 10
+    let [_, b] = *arr;
+    b
+}


This now tests the size and stride of x87_f80_raw.

folkertdev · 2026-10-31T23:02:53Z

+    // Map `core::arch::{x86, x86_64}::x87_f80` to LLVM's `x86_fp80`.
+    if Ty::is_x87_f80_lang_item(layout, cx) {
+        return cx.type_x87_f80();
+    }


Is this still relevant with #165146 now being merged?

folkertdev added the F-c_longdouble `#![feature(c_longdouble)]` label Oct 8, 2026

This comment has been minimized.

Sign in to view

folkertdev force-pushed the x87_f80-simple branch from b4fafc4 to 16b2882 Compare October 8, 2026 15:25

This comment has been minimized.

Sign in to view

folkertdev force-pushed the x87_f80-simple branch from 16b2882 to 617db2d Compare October 8, 2026 16:37

folkertdev commented Oct 8, 2026

View reviewed changes

Comment thread compiler/rustc_ty_utils/src/layout/invariant.rs Resolved

Comment thread library/stdarch/crates/core_arch/src/x86/mod.rs Resolved

folkertdev force-pushed the x87_f80-simple branch from 617db2d to d582ca8 Compare October 8, 2026 19:07

rustbot assigned beetrees Oct 8, 2026

folkertdev marked this pull request as ready for review October 8, 2026 19:07

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 8, 2026

This comment has been minimized.

Sign in to view

rust-bors Bot pushed a commit that referenced this pull request Oct 8, 2026

Auto merge of #163992 - folkertdev:x87_f80-simple, r=<try>

4923e95

add `core::arch::{x86, x86_64}::x87_f80` try-job: test-various try-job: test-i686-*

tgross35 reviewed Oct 8, 2026

View reviewed changes

Comment thread library/stdarch/crates/core_arch/src/x86/mod.rs Resolved

Comment thread library/stdarch/crates/core_arch/src/x86/mod.rs Resolved

folkertdev force-pushed the x87_f80-simple branch from d582ca8 to a9a37a4 Compare October 8, 2026 21:29

RalfJung reviewed Oct 9, 2026

View reviewed changes

RalfJung reviewed Oct 15, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

folkertdev force-pushed the x87_f80-simple branch from 280df50 to 87ea41c Compare October 21, 2026 15:55

This comment has been minimized.

Sign in to view

folkertdev mentioned this pull request Oct 25, 2026

C interop: f80, f128 and c_longdouble rust-lang/goals#701

Open

folkertdev force-pushed the x87_f80-simple branch from 87ea41c to 1cf2718 Compare October 26, 2026 14:15

This comment has been minimized.

Sign in to view

folkertdev force-pushed the x87_f80-simple branch from 1cf2718 to 73c618c Compare October 26, 2026 23:07

This comment has been minimized.

Sign in to view

folkertdev force-pushed the x87_f80-simple branch from 73c618c to 40ddd5f Compare October 27, 2026 18:46

This comment has been minimized.

Sign in to view

beetrees requested changes Oct 28, 2026

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 28, 2026

folkertdev force-pushed the x87_f80-simple branch 3 times, most recently from e3ac46d to fbd679f Compare October 31, 2026 13:52

This comment has been minimized.

Sign in to view

folkertdev force-pushed the x87_f80-simple branch from fbd679f to 861c258 Compare October 31, 2026 14:05

add core::arch::{x86, x86_64}::x87_f80

458a970

folkertdev force-pushed the x87_f80-simple branch from 861c258 to 458a970 Compare October 31, 2026 23:00

folkertdev commented Oct 31, 2026

View reviewed changes

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 31, 2026

	Immediate::Scalar(scalar) => {
	alloc.write_scalar(alloc_range(Size::ZERO, scalar.size()), scalar)?;
	}

Conversation

folkertdev commented Oct 8, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

future work

Uh oh!

This comment has been minimized.

This comment has been minimized.

Uh oh!

Uh oh!

folkertdev commented Oct 8, 2026

Uh oh!

rustbot commented Oct 8, 2026

Uh oh!

folkertdev commented Oct 8, 2026

Uh oh!

This comment has been minimized.

Uh oh!

Uh oh!

rust-bors Bot commented Oct 8, 2026

Uh oh!

RalfJung commented Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RalfJung Oct 9, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RalfJung commented Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

beetrees commented Oct 9, 2026

Uh oh!

tgross35 commented Oct 9, 2026

Uh oh!

RalfJung Oct 15, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RalfJung Oct 15, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

beetrees commented Oct 16, 2026

Uh oh!

This comment has been minimized.

scottmcm commented Oct 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

beetrees Oct 28, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

folkertdev commented Oct 8, 2026 •

edited by rustbot

Loading

RalfJung commented Oct 9, 2026 •

edited

Loading

RalfJung Oct 9, 2026 •

edited by rustbot

Loading

RalfJung commented Oct 9, 2026 •

edited

Loading

RalfJung Oct 15, 2026 •

edited by rustbot

Loading

RalfJung Oct 15, 2026 •

edited by rustbot

Loading

scottmcm commented Oct 18, 2026 •

edited

Loading

beetrees Oct 28, 2026 •

edited by rustbot

Loading

beetrees Oct 28, 2026 •

edited by rustbot

Loading

folkertdev left a comment •

edited by rustbot

Loading