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.

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?

folkertdev

@rustbot ready

View changes since this review

folkertdev · 2026-11-02T00:03:09Z

+// ALIGN4: %"minicore::arch::x86::x87_f80" = type { [10 x i8], [2 x i8] }
+// ALIGN4: %WrappedStruct = type { %"minicore::arch::x86::x87_f80", i8, [3 x i8] }
+//
+// ALIGN16: %"minicore::arch::{{x86|x86_64}}::x87_f80" = type { [10 x i8], [6 x i8] }
+// ALIGN16: %WrappedStruct = type { %"minicore::arch::{{x86|x86_64}}::x87_f80", i8, [15 x i8] }


Not the most intuitive, but I guess it works.

folkertdev · 2026-11-02T00:05:54Z

+                cx.type_padding_filler(element_ty.size * count, Align::ONE)
+            } else {
+                cx.type_array(element_ty.llvm_type(cx), count)
+            }


This is a bit ugly. We could have element_ty.is_x87_f80_raw I guess, is that worth it?

rustbot · 2026-11-04T00:12:17Z

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 · 2026-11-04T00:13:57Z

+// ALIGN4: @WRAPPED = external global [12 x i8]
+// ALIGN4: @WRAPPED_ARRAY = external global [36 x i8]
+// ALIGN4: @WRAPPED_STRUCT = external global [16 x i8]
+//
+// ALIGN16: @WRAPPED = external global [16 x i8]
+// ALIGN16: @WRAPPED_ARRAY = external global [48 x i8]
+// ALIGN16: @WRAPPED_STRUCT = external global [32 x i8]


I've updated these now that #165371 has been merged

View changes since the review

tgross35 · 2026-11-04T00:40:36Z

Just noting that we should probably have @RalfJung's ack on the current approach

folkertdev · 2026-11-04T00:47:19Z

Absolutely. I think at this point the _raw type is no longer special (via a series of cleanups by Bee), so while we don't support having the _raw type by itself in callconv, the type itself is just a standard type, and no longer requires weird weird carveouts or modified assertions.

beetrees

Could you add the assertion in CastTarget::llvm_type that I mentioned in #163992 (comment).

View changes since this review

beetrees · 2026-11-04T01:38:36Z

This should be if cls.contains(&Some(Class::X87)) { [...] } else { [...] }, as the if cls.contains(&Some(Class::X87)) changes cls_or_mem to Err(Memory).

beetrees · 2026-11-04T01:48:43Z

The doc comments above LayoutTypeCodegenMethods::backend_type (in rustc_codegen_ssa) and TyAndLayout::llvm_type (in rustc_codegen_llvm) also need to be updated - they currently say that the returned type is the correct type for being stack-allocated/alloca, which it hasn't been used for for a while and definitely won't be true after this PR. There's also a bunch of other outdated stuff in the llvm_type doc comment (specifically, LLVM no longer has typed pointers, and the compiler no longer uses the type for indexing arrays or structs) but that's all unaffected by this PR so no need to update it in this PR.

beetrees · 2026-11-04T02:02:02Z

Also could you update the PR description to reflect the current state of the PR. I think the bit about breaking invariants no longer applies?

folkertdev

@rustbot ready

View changes since this review

folkertdev · 2026-11-04T11:40:49Z

+    /// For sized types, this is the in-memory type that is used for stores and loads.
+    /// The type used for immediates may be different, see [`Self::immediate_llvm_type`].


We can workshop this some more. I did just remove the line about the pointer type, it's blatantly wrong and didn't really fit in the reformatted text.

folkertdev · 2026-11-04T11:42:39Z

+    /// The backend type used for a rust type when it is being loaded from or
+    /// stored to memory.
+    ///
+    /// Note that this type may be different from the type used for allocas.
+    /// For instance for x87_f80 the store size and allocation size is different,
+    /// so different types are used to represent it in memory versus on the stack.


We basically never use a typed alloca any more (only for scalable vectors from what I can find), so really the point here is more the size than the exact type.

But intuitively talking about allocas concretely makes more sense to me than "stack allocation".

folkertdev · 2026-11-04T11:43:09Z

@@ -209,4 +215,4 @@
            if rest_count == 1 && !self.rest.is_consecutive {
                return rest_ll_unit;
            }

@@ -217,6 +223,11 @@
        let prefix_args = self.prefix.iter().map(|reg| reg.llvm_type(cx));
        let rest_args = (0..rest_count).map(|_| rest_ll_unit);
        let args: Vec<_> = prefix_args.chain(rest_args).collect();
+
+        // The x87_f80 type has a store size of 10 bytes, but an allocation size of 12 or 16 bytes.
+        // Ensure we don't cast to it, because that could have unintened behavior.
+        assert!(args.iter().all(|a| *a != cx.type_x87_f80()));
+


is this what you had in mind for the assert!s?

RalfJung · 2026-11-04T13:23:06Z


+        // The x87_f80 type has a store size of 10 bytes, but an allocation size of 12 or 16 bytes.
+        // Ensure we don't cast to it, because that could have unintened behavior.
+        assert!(rest_ll_unit != cx.type_x87_f80());


Is it worth introducing a more central predicate like equal_store_and_alloc_size or so?

View changes since the review

Ideally I don't think anywhere in the compiler should be relying on the alloc size of what is returned from llvm_type, so I don't think we should add a method that encourages people to rely on it (the compiler has been moving towards using [size x i8] when an LLVM type is needed for its allocation size) - the only reason the assert is needed now is that load_cast/store_cast directly load/store the LLVM aggregate to/from the scratch space (loading/storing LLVM aggregates is generally something that should be avoided). I plan to fix load_cast/store_cast to not do that, at which point this assertion can be removed, but didn't want to block this PR on it.

RalfJung · 2026-11-04T13:24:57Z

Just noting that we should probably have @RalfJung's ack on the current approach

Is the interpreter exercised with the new type (e.g. via Miri)?
It looks like all the existing assertions pass, so that looks good from that side. I haven't dug into the rest of the changes, happy to leave that to the assigned reviewer.

beetrees · 2026-11-04T13:26:05Z

            if rest_count == 1 && !self.rest.is_consecutive {
                return rest_ll_unit;
            }



The assert!(rest_ll_unit != cx.type_x87_f80()); needs to be here - the allocation size is only a problem at the moment when it's in an LLVM struct or array (as we currently just tell LLVM to load the LLVM struct/array itself unless CastTarget::offset_pair is used).

View changes since the review

beetrees · 2026-11-04T13:28:55Z

 pub trait LayoutTypeCodegenMethods<'tcx>: BackendTypes {
-    /// The backend type used for a rust type when it's in memory,
-    /// such as when it's stack-allocated or when it's being loaded or stored.
+    /// The backend type used for a rust type when it is being loaded from or


Suggested change

/// The backend type used for a rust type when it is being loaded from or

/// The backend type used for a Rust type when it is being loaded from or

View changes since the review

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

folkertdev force-pushed the x87_f80-simple branch from 458a970 to 96a7143 Compare November 2, 2026 00:00

folkertdev commented Nov 2, 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 Nov 2, 2026

folkertdev added 2 commits November 4, 2026 01:02

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

c3f5e5d

rename Reg::align and Uniform::align to scratch_align

0c1c3d6

folkertdev force-pushed the x87_f80-simple branch from 96a7143 to 1f53623 Compare November 4, 2026 00:12

folkertdev commented Nov 4, 2026

View reviewed changes

beetrees requested changes Nov 4, 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 Nov 4, 2026

use type_padding_filler for x87_f80_raw in arrays and structs

f5ac1b0

folkertdev force-pushed the x87_f80-simple branch from 1f53623 to 5b52d2c Compare November 4, 2026 11:28

folkertdev commented Nov 4, 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 Nov 4, 2026

This comment has been minimized.

Sign in to view

RalfJung reviewed Nov 4, 2026

View reviewed changes

beetrees requested changes Nov 4, 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 Nov 4, 2026

folkertdev added 2 commits November 4, 2026 15:19

add assert that we never cast to x87_f80

352accd

adjust comments on backend type functions

18ceb9e

folkertdev force-pushed the x87_f80-simple branch from 5b52d2c to 18ceb9e Compare November 4, 2026 14:22

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

		/// For sized types, this is the in-memory type that is used for stores and loads.
		/// The type used for immediates may be different, see [`Self::immediate_llvm_type`].

	/// The backend type used for a rust type when it is being loaded from or
	/// The backend type used for a Rust type when it is being loaded from or

Conversation

folkertdev commented Oct 8, 2026 • edited 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!

folkertdev left a comment • 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!

rustbot commented Nov 4, 2026

Uh oh!

folkertdev Nov 4, 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!

tgross35 commented Nov 4, 2026

Uh oh!

folkertdev commented Nov 4, 2026

Uh oh!

beetrees left a comment • 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!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

beetrees commented Nov 4, 2026

Uh oh!

beetrees commented Nov 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

folkertdev left a comment • 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!

This comment has been minimized.

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

Uh oh!

folkertdev commented Oct 8, 2026 •

edited

Loading

RalfJung commented Oct 9, 2026 •

edited

Loading

RalfJung Oct 9, 2026 •

edited by rustbot

Loading

RalfJung commented Oct 9, 2026 •

edited

Loading

folkertdev left a comment •

edited by rustbot

Loading

folkertdev Nov 4, 2026 •

edited by rustbot

Loading

beetrees left a comment •

edited by rustbot

Loading

beetrees commented Nov 4, 2026 •

edited

Loading

folkertdev left a comment •

edited by rustbot

Loading

RalfJung Nov 4, 2026 •

edited by rustbot

Loading

beetrees Nov 4, 2026 •

edited by rustbot

Loading

beetrees Nov 4, 2026 •

edited by rustbot

Loading