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 · 2026-11-04T18:39:59Z

@rustbot ready

beetrees

One nit, then LGTM (pending a clarification from @RalfJung).

View changes since this review

beetrees · 2026-11-05T00:40:04Z

+/// 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.


Suggested change

/// NOTE: this type MUST NOT be used anywhere except in the x87_f80 wrapper below.

This line can be removed now.

beetrees · 2026-11-05T00:53:54Z

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.

@RalfJung, to clarify are you saying that the current state of the tests is fine from your side, or that you'd like to see some tests that can be run via Miri before this PR merges?

RalfJung · 2026-11-05T07:09:08Z

A Miri test would be better to confirm that indeed the interpreter can work with this new "weird" type.

beetrees

See above.

View changes since this review

rustbot · 2026-11-05T11:03:11Z

miri is developed in its own repository. If the Miri part of this change can be broken out, consider making this change to rust-lang/miri instead. However, if Miri needs adjusting for rustc changes, just ignore this message.

cc @rust-lang/miri

RalfJung · 2026-11-05T11:05:42Z

Thanks! Looks good.

View changes since the review

Oh wait I didn't realize there were changes beyond the new test...

folkertdev

Some notes

@rustbot ready

View changes since this review

folkertdev · 2026-11-05T11:09:15Z

+            ty::Adt(..)
+                if let BackendRepr::Scalar(scalar) = value.layout.backend_repr
+                    && scalar.primitive() == Primitive::Float(Float::X87F80) =>
+            {
+                // The x87_f80_raw lang item should be treated as a scalar in this context.
+                //
+                // NOTE: Keep this in sync with the array optimization for int/float
+                // types below!
+                self.read_scalar(value, ExpectedKind::Float)?;
+                if self.reset_provenance_and_padding {
+                    self.ecx.clear_provenance(value)?;
+                    self.add_data_range_place(value);
+                }
+                interp_ok(true)
+            }


This is needed for accurate tracking of what bytes are initialized, because structurally x87_f80_raw is an ADT with no fields.

That makes it different from ppcf128 and f16b which have an integer inside of them and hit the branch right above this.

The comment and your text talks about x87_f80_raw but the code does not have "raw" in it? The comment talks about a lang item but the code does not? I am quite confused.

folkertdev · 2026-11-05T11:10:27Z

+/// NOTE: this type should not be used anywhere except in the x87_f80 wrapper below. The backend
+/// can handle it fine, but callconv calculation will likely panic.


I've kept but weakened the line here. It still does not make sense to use this type, and it will likely panic if you try, but should not otherwise do anything weird.

RalfJung · 2026-11-05T11:18:03Z

+            ty::Adt(..)
+                if let BackendRepr::Scalar(scalar) = value.layout.backend_repr
+                    && scalar.primitive() == Primitive::Float(Float::X87F80) =>


This will also kick in for transparent wrappers around the lang item, right?
That's not what we want here, this should only identify the actual primitive type.

View changes since the review

Sure, I've changed it to just check for the lang item instead

is that acceptable?

I didn't forget about this, I have other obligations besides reviewing Rust PRs. No reason to push.

Yes the logic looks good now. I am confused by this comment:

// NOTE: Keep this in sync with the array optimization for int/float // types below!

The array optimization does not consider X87F80Raw so this comment seems bogus?

Right, I just copied it from above but that optimization also only looks at the TyKind and would never fire for x87_f80. That's probably fine (for now), we don't expect usage of this type will ever really be a performance problem.

I didn't forget about this, I have other obligations besides reviewing Rust PRs. No reason to push.

Sorry, I was just trying to make the implicit question in my earlier comment explicit. Thanks for taking a look!

RalfJung · 2026-11-05T19:36:42Z

+            ty::Adt(adt_def, _)
+                if self.ecx.tcx.is_lang_item(adt_def.did(), LangItem::X87F80Raw) =>
+            {
+                // The x87_f80_raw lang item should be treated as a scalar in this context.


Suggested change

// The x87_f80_raw lang item should be treated as a scalar in this context.

// The x87_f80_raw lang item is effectively a primitive.

View changes since the review

Without it, reads are reported as UB because the tracking of what bytes are initialized is incorect

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

beetrees requested changes Nov 4, 2026

View reviewed changes

Comment thread compiler/rustc_codegen_llvm/src/abi.rs Resolved

Comment thread compiler/rustc_codegen_ssa/src/traits/type_.rs Resolved

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 force-pushed the x87_f80-simple branch from 5b52d2c to 18ceb9e Compare November 4, 2026 14:22

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

beetrees approved these changes Nov 5, 2026

View reviewed changes

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

folkertdev added 5 commits November 5, 2026 11:06

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

e537985

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

078f379

use type_padding_filler for x87_f80_raw in arrays and structs

807d237

add assert that we never cast to x87_f80

000aaeb

adjust comments on backend type functions

1eceba8

folkertdev force-pushed the x87_f80-simple branch from 18ceb9e to 0fd1c88 Compare November 5, 2026 11:03

RalfJung reviewed Nov 5, 2026

View reviewed changes

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

RalfJung reviewed Nov 5, 2026

View reviewed changes

folkertdev force-pushed the x87_f80-simple branch 2 times, most recently from 16dc967 to 27fa2c7 Compare November 5, 2026 18:37

RalfJung reviewed Nov 5, 2026

View reviewed changes

support x87_f80 in const eval's try_visit_primitive

d64f2fb

Without it, reads are reported as UB because the tracking of what bytes are initialized is incorect

folkertdev force-pushed the x87_f80-simple branch from 27fa2c7 to d64f2fb Compare November 5, 2026 21:23

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

		/// NOTE: this type should not be used anywhere except in the x87_f80 wrapper below. The backend
		/// can handle it fine, but callconv calculation will likely panic.

	// The x87_f80_raw lang item should be treated as a scalar in this context.
	// The x87_f80_raw lang item is effectively a primitive.

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!

Uh oh!

Uh oh!

folkertdev commented Nov 4, 2026

Uh oh!

beetrees left a comment • edited 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 Nov 5, 2026

Uh oh!

RalfJung commented Nov 5, 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!

rustbot commented Nov 5, 2026

Uh oh!

RalfJung Nov 5, 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!

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!

RalfJung Nov 5, 2026 • edited 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 Nov 5, 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!

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

beetrees left a comment •

edited

Loading

beetrees left a comment •

edited by rustbot

Loading

RalfJung Nov 5, 2026 •

edited by rustbot

Loading

folkertdev left a comment •

edited by rustbot

Loading

RalfJung Nov 5, 2026 •

edited

Loading

RalfJung Nov 5, 2026 •

edited by rustbot

Loading

RalfJung Nov 5, 2026 •

edited by rustbot

Loading