x86: on targets that requires SSE, use those registers for ABI - #161583
x86: on targets that requires SSE, use those registers for ABI#161583RalfJung wants to merge 1 commit into
Conversation
7f487df to
06a4f6d
Compare
This comment has been minimized.
This comment has been minimized.
a01a5dd to
7d66bd1
Compare
|
@bors try jobs=i686,various |
This comment has been minimized.
This comment has been minimized.
x86: on targets that requires SSE, use those registers for ABI try-job: *i686* try-job: *various*
|
@bors try jobs=aarch64-linux |
This comment has been minimized.
This comment has been minimized.
x86: on targets that requires SSE, use those registers for ABI try-job: *aarch64-linux*
|
It is strange that PR CI passed. Looks like even on LLVM 21, tests/codegen-llvm/pclmulqdq-target-feature-inlining.rs works? How can that be if it does not have llvm/llvm-project#205106 ? |
This comment was marked as resolved.
This comment was marked as resolved.
|
Oh wait nvm, there was a t-lang ask here that I forgot about. :) |
|
I think we should make this change, including the breaking change and turning the non-silenceable warning for disabling SSE on i686 targets into a hard error. @rfcbot fcp merge lang |
|
@tmandry has proposed to merge this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns. |
|
Thanks @RalfJung. @rfcbot reviewed
It has been our practice to FCP restabilizations (see, e.g., #145954 (comment)). |
|
@bors try jobs=llvm |
This comment has been minimized.
This comment has been minimized.
x86: on targets that requires SSE, use those registers for ABI try-job: *llvm*
|
🔔 This is now entering its final comment period, as per the review above. 🔔 |
All right, will do. This is blocked on #161280 (FCP finished there but it hasn't been reviewed yet). |
This comment has been minimized.
This comment has been minimized.
Ah, it doesn't work in LLVM 21, we just don't run that in PR CI. :) (We run it on aarch64, but not on x86.) |
bc7dd85 to
02f819a
Compare
|
@bors try jobs=x86llvm |
This comment has been minimized.
This comment has been minimized.
x86: on targets that requires SSE, use those registers for ABI try-job: x86*llvm*
View all comments
Now that llvm/llvm-project#142321 is fixed, we can use the vector ABI without re-introducing #139029. So this effectively reverts #141309.
This can lead to new hard errors on code that previously compiled if all of the following apply:
Also this will cause performance regressions on LLVM 22 and 21 as those don't have the fix for llvm/llvm-project#142321. IMO that's fine. Programs will do the right thing, they will just be a bit slower due to #139029.
@rust-lang/lang please let me know if you're fine with just landing this or if you want to apply some sort of process here. (We have just landed this the first time we tried to use SSE in the ABI, but back then I didn't think of the fact that in some unsupported corner cases, this can lead to new hard errors.) If you do think that this needs FCP, please let me know and I will propose that we turn the target-feature ABI check into a hard error on x86-32 (similar to #161280).
TODO:
r? @nikic
Cc @tgross35