Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

platform-support: document CPU baseline for x86-32 targets #136493

Merged
merged 2 commits into from
Feb 4, 2025

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Feb 3, 2025

Also fixes the footnote for i686-unknown-hurd-gnu (which has the bad case of the x87 issue since it uses a non-SSE baseline) and adds the missing footnote for i686-unknown-redox. Both of those targets break our usual pattern by not using the Pentium 4 baseline, but fixing that is a much larger change that I will not pursue (see Zulip).

Cc @bjorn3

@rustbot
Copy link
Collaborator

rustbot commented Feb 3, 2025

r? @ehuss

rustbot has assigned @ehuss.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 3, 2025
[`i686-pc-windows-gnullvm`](platform-support/pc-windows-gnullvm.md) | ✓ | 32-bit x86 MinGW (Windows 10+, Pentium 4), LLVM ABI [^x86_32-floats-return-ABI]
[`i686-unknown-freebsd`](platform-support/freebsd.md) | ✓ | 32-bit x86 FreeBSD (Pentium 4) [^x86_32-floats-return-ABI]
`i686-unknown-linux-musl` | ✓ | 32-bit Linux with musl 1.2.3 (Pentium 4) [^x86_32-floats-return-ABI]
[`i686-unknown-uefi`](platform-support/unknown-uefi.md) | ? (softfloat) | 32-bit UEFI
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just verifying whether or not you meant to make this just "softfloat", or "Pentium 4, softfloat"?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should mention the base CPU, even if it's not that important because it doesn't have hard floats.

Copy link
Member Author

@RalfJung RalfJung Feb 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I omitted the base CPU because I don't know of it having any influence here, but I don't have a strong opinion so sure I can add it.

@ehuss
Copy link
Contributor

ehuss commented Feb 3, 2025

This looks correct to me, and seems like a reasonable thing to do. In the future we may want to consider changing the way this is presented so that the baseline CPU is explicit for each target (and preferably automated via something like #120745).

Since this is documenting new details, I would slightly prefer for this to be reviewed by compiler instead. I don't think this is documenting anything new exactly, but don't want to be making any guarantees that I am unaware of any hidden implications.

r? compiler

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Feb 3, 2025
@rustbot rustbot assigned estebank and unassigned ehuss Feb 3, 2025
Copy link
Member

@Noratrieb Noratrieb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r=me after adding the CPU to UEFI. I didn't check any of the values against the target specs (except apple which seemed wrong but is right) but this seems to be correct.

@RalfJung RalfJung force-pushed the x86-platform-support branch from eb6bde7 to 56725e8 Compare February 3, 2025 18:08
@RalfJung
Copy link
Member Author

RalfJung commented Feb 3, 2025

I did check this twice... ofc there could still be mistakes but then we'll just fix them when we notice them. :)
@bors r=Noratrieb rollup

@bors
Copy link
Contributor

bors commented Feb 3, 2025

📌 Commit 56725e8 has been approved by Noratrieb

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 3, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 3, 2025
…oratrieb

platform-support: document CPU baseline for x86-32 targets

Also fixes the footnote for i686-unknown-hurd-gnu (which has the bad case of the x87 issue since it uses a non-SSE baseline) and adds the missing footnote for i686-unknown-redox. Both of those targets break our usual pattern by not using the Pentium 4 baseline, but fixing that is a much larger change that I will not pursue (see [Zulip](https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/x86-32.20target.20names)).

Cc `@bjorn3`
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 4, 2025
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#136289 (OnceCell & OnceLock docs: Using (un)initialized consistently)
 - rust-lang#136299 (Ignore NLL boring locals in polonius diagnostics)
 - rust-lang#136411 (Omit argument names from function pointers that do not have argument names)
 - rust-lang#136430 (Use the type-level constant value `ty::Value` where needed)
 - rust-lang#136476 (Remove generic `//@ ignore-{wasm,wasm32,emscripten}` in tests)
 - rust-lang#136484 (Notes on types/traits used for in-memory query caching)
 - rust-lang#136493 (platform-support: document CPU baseline for x86-32 targets)
 - rust-lang#136498 (Update books)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 628ae40 into rust-lang:master Feb 4, 2025
6 checks passed
@rustbot rustbot added this to the 1.86.0 milestone Feb 4, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 4, 2025
Rollup merge of rust-lang#136493 - RalfJung:x86-platform-support, r=Noratrieb

platform-support: document CPU baseline for x86-32 targets

Also fixes the footnote for i686-unknown-hurd-gnu (which has the bad case of the x87 issue since it uses a non-SSE baseline) and adds the missing footnote for i686-unknown-redox. Both of those targets break our usual pattern by not using the Pentium 4 baseline, but fixing that is a much larger change that I will not pursue (see [Zulip](https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/x86-32.20target.20names)).

Cc ``@bjorn3``
@RalfJung RalfJung deleted the x86-platform-support branch February 4, 2025 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants