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

Forbid disabling SSE on x86 targets that have SSE in their "baseline" #133611

Open
RalfJung opened this issue Nov 29, 2024 · 0 comments
Open

Forbid disabling SSE on x86 targets that have SSE in their "baseline" #133611

RalfJung opened this issue Nov 29, 2024 · 0 comments
Labels
A-target-feature Area: Enabling/disabling target features like AVX, Neon, etc. C-discussion Category: Discussion or questions that doesn't represent real issues. O-x86_32 Target: x86 processors, 32 bit (like i686-*) O-x86_64 Target: x86-64 processors (like x86_64-*) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@RalfJung
Copy link
Member

RalfJung commented Nov 29, 2024

Passing -Ctarget-feature=-sse on an x86-64 target currently produces an ugly LLVM error.
Doing the same on a x86-32 target leads to unsound floating-point behavior.

Therefore, I think we should deprecate and eventually fully forbid toggling the sse/sse2 target features on x86 targets, except for those targets that do not have these features to begin with (e.g. i586-unknown-linux-gnu).

I am implementing some machinery here that could help with that, but properly implementing this will be tricky since one can also use -Ctarget-cpu to disable these target features.

Once this is implemented, we have some options for improving the Rust ABI on these targets as well:

  • on x86-32, we could use SSE registers to return float values, instead of PassMode::Indirect
  • on all x86 targets, we could pass SIMD vectors of up to 128 bits in registers rather than indirectly

Here, compiler team triage decided "Current Tier 1 x86 targets require SSE-based floats at minimum". The concrete proposal above got MCP'd in rust-lang/compiler-team#808.

Open questions

How do we best implement this? It's non trivial since -Ctarget-cpu can alter the sse/sse2 feature gates, so our usual approach of just checking which target features got toggled with -Ctarget-feature does not work.

To make things worse, the way we control whether sse/sse2 is available in the "basline" is via the base.cpu field, not via the explicit list of target features, so if we want to do things "only if the baseline has SSE", that's non-trivial to implement. Maybe we should just add a bool field to the target spec that directly controls "use SSE registers for Rust ABI" or "requires SSE registers" or so?

Cc @bjorn3 @workingjubilee @Amanieu

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Nov 29, 2024
@jieyouxu jieyouxu added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-target-feature Area: Enabling/disabling target features like AVX, Neon, etc. O-x86_32 Target: x86 processors, 32 bit (like i686-*) O-x86_64 Target: x86-64 processors (like x86_64-*) C-discussion Category: Discussion or questions that doesn't represent real issues. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Nov 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-target-feature Area: Enabling/disabling target features like AVX, Neon, etc. C-discussion Category: Discussion or questions that doesn't represent real issues. O-x86_32 Target: x86 processors, 32 bit (like i686-*) O-x86_64 Target: x86-64 processors (like x86_64-*) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants