Skip to content

Stabilize sha512, sm3 and sm4 for x86 #140767

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

Merged
merged 1 commit into from
Jun 9, 2025
Merged

Conversation

sayantn
Copy link
Contributor

@sayantn sayantn commented May 7, 2025

This PR stabilizes the feature flag sha512_sm_x86 (tracking issue #126624).

Public API

The 3 x86 target features sha512, sm3 and sm4, and the associated intrinsics in stdarch.

These target features are very specialized, and are only used to signal the presence of the corresponding CPU instruction. They don't have any nontrivial interaction with the ABI (contrary to something like AVX), and serve the only purpose of enabling 10 stdarch intrinsics, all of which have been implemented and propagated to rustc via a stdarch submodule update.

Also, these were added in LLVM17, and as the minimum LLVM required for rustc is LLVM19, we are safe in that front too!

Associated PRs

As all of the required tasks have been done (adding the target features to rustc, implementing their runtime detection in std_detect and implementing the associated intrinsics in core_arch), these target features can be stabilized now.

cc @rust-lang/lang
cc @rust-lang/libs-api for the intrinsics and runtime detection

I don't think anyone else worked on this feature, so no one else to ping, maybe cc @Amanieu. I will send the reference pr soon.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. A-target-feature Area: Enabling/disabling target features like AVX, Neon, etc. I-lang-nominated Nominated for discussion during a lang team meeting. O-x86_32 Target: x86 processors, 32 bit (like i686-*) (IA-32) O-x86_64 Target: x86-64 processors (like x86_64-*) (also known as amd64 and x64) labels May 7, 2025
@rustbot

This comment has been minimized.

@rustbot rustbot added T-lang Relevant to the language team and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels May 7, 2025
@rust-log-analyzer

This comment has been minimized.

@sayantn sayantn force-pushed the stabilize-sha512 branch from e7b5f40 to 871162c Compare May 7, 2025 20:06
@traviscross
Copy link
Contributor

traviscross commented May 13, 2025

Stabilizing target features are somewhat minimal as far as stabilizations go, but still, this is going to need something more in the way of a stabilization report in the description of this PR. We need some narrative about what we're stabilizing here and a review of the evidence about why it's OK for us to stabilize this now. Have a look at our new template for this in rust-lang/rustc-dev-guide#2219.

Please think too about who else in the Project has worked on this or related things and who might therefore be interested in this, and please ping those people here.

This will also need a PR to the Reference documenting the change, and that should be linked from the stabilization report as well.

@rustbot labels +S-waiting-on-documentation

Please renominate when the stabilization report is available.

@rustbot rustbot added the S-waiting-on-documentation Status: Waiting on approved PRs to documentation before merging label May 13, 2025
@traviscross traviscross added I-lang-radar Items that are on lang's radar and will need eventual work or consideration. and removed I-lang-nominated Nominated for discussion during a lang team meeting. labels May 13, 2025
@traviscross
Copy link
Contributor

is this okay? Or do I have to give more details?

Thanks for adding the details above. @scottmcm, @RalfJung, @workingjubilee, anything more you want to see for this sort of thing?

@traviscross traviscross added I-lang-nominated Nominated for discussion during a lang team meeting. and removed I-lang-radar Items that are on lang's radar and will need eventual work or consideration. labels May 14, 2025
@RalfJung
Copy link
Member

The main question are indeed ABI concerns, or more generally "is there any potential for problems when mixing code built with and without these target features". It seems the answer to that is "no".

The only other issue I think we had is, "does our minimum LLVM version support these target features".

@sayantn
Copy link
Contributor Author

sayantn commented May 15, 2025

@RalfJung updated the text with the minimum llvm version

@traviscross
Copy link
Contributor

Sounds OK to me then. Let's propose FCP.

@rfcbot fcp merge

@rfcbot
Copy link
Collaborator

rfcbot commented May 15, 2025

Team member @traviscross 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.
See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels May 15, 2025
@sayantn sayantn changed the title Stabilize sha512. sm3 and sm4 for x86 Stabilize sha512, sm3 and sm4 for x86 May 31, 2025
@sayantn sayantn force-pushed the stabilize-sha512 branch from f34cee7 to cb1d70f Compare May 31, 2025 11:33
@rust-log-analyzer

This comment has been minimized.

@sayantn
Copy link
Contributor Author

sayantn commented May 31, 2025

The job aarch64-gnu-llvm-19-2 failed! Check out the build log: (web) (plain)
Click to see the possible cause of the failure (guessed by this bot)

cc @bjorn3, iirc you mentioned this in the stdarch PR 😅

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. to-announce Announce this issue on triage meeting and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels May 31, 2025
@rfcbot
Copy link
Collaborator

rfcbot commented May 31, 2025

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

@bors
Copy link
Collaborator

bors commented Jun 4, 2025

☔ The latest upstream changes (presumably #141984) made this pull request unmergeable. Please resolve the merge conflicts.

@sayantn
Copy link
Contributor Author

sayantn commented Jun 4, 2025

I am updating the stdarch submodule in #141964, so I will update this after that merges

@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Jun 5, 2025
@sayantn sayantn force-pushed the stabilize-sha512 branch from cb1d70f to 632396e Compare June 7, 2025 17:58
Copy link
Contributor

@traviscross traviscross left a comment

Choose a reason for hiding this comment

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

@rustbot labels -S-waiting-on-documentation

The FCP is complete. The Reference PR is approved. The impl looks OK to me, but let's...

r? tgross35

for final review.

@rustbot rustbot removed the S-waiting-on-documentation Status: Waiting on approved PRs to documentation before merging label Jun 7, 2025
@rustbot rustbot assigned tgross35 and unassigned scottmcm Jun 7, 2025
@tgross35
Copy link
Contributor

tgross35 commented Jun 9, 2025

Looks good to me as well, thanks @sayantn.

@bors r=traviscross,tgross35

@bors
Copy link
Collaborator

bors commented Jun 9, 2025

📌 Commit 632396e has been approved by traviscross,tgross35

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 Jun 9, 2025
bors added a commit that referenced this pull request Jun 9, 2025
Rollup of 5 pull requests

Successful merges:

 - #140767 (Stabilize `sha512`, `sm3` and `sm4` for x86)
 - #141001 (Make NonZero<char> possible)
 - #141993 (Use the in-tree `compiler-builtins` for the sysroot)
 - #142208 (Always consider `const _` items as live for dead code analysis)
 - #142238 (stabilize nonnull_provenance)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 2f26913 into rust-lang:master Jun 9, 2025
10 checks passed
@rustbot rustbot added this to the 1.89.0 milestone Jun 9, 2025
rust-timer added a commit that referenced this pull request Jun 9, 2025
Rollup merge of #140767 - sayantn:stabilize-sha512, r=traviscross,tgross35

Stabilize `sha512`, `sm3` and `sm4` for x86

This PR stabilizes the feature flag `sha512_sm_x86` (tracking issue #126624).

# Public API
The 3 `x86` target features `sha512`, `sm3` and `sm4`, and the associated intrinsics in stdarch.

These target features are very specialized, and are only used to signal the presence of the corresponding CPU instruction. They don't have any nontrivial interaction with the ABI (contrary to something like AVX), and serve the only purpose of enabling 10 stdarch intrinsics, all of which have been implemented and propagated to rustc via a stdarch submodule update.

Also, these were added in LLVM17, and as the minimum LLVM required for rustc is LLVM19, we are safe in that front too!

# Associated PRs
 - #126704
 - rust-lang/stdarch#1592
 - rust-lang/stdarch#1790
 - #140389 (stdarch submodule update)
 - rust-lang/stdarch#1796 (stabilizing the runtime detection and intrinsics)
 - #141964 (stdarch submodule update for the stabilization of the runtime detection and intrinsics)

As all of the required tasks have been done (adding the target features to rustc, implementing their runtime detection in std_detect and implementing the associated intrinsics in core_arch), these target features can be stabilized now.

cc `@rust-lang/lang`
cc `@rust-lang/libs-api` for the intrinsics and runtime detection

I don't think anyone else worked on this feature, so no one else to ping, maybe cc `@Amanieu.` I will send the reference pr soon.
tgross35 pushed a commit to tgross35/compiler-builtins that referenced this pull request Jun 9, 2025
Rollup of 5 pull requests

Successful merges:

 - rust-lang/rust#140767 (Stabilize `sha512`, `sm3` and `sm4` for x86)
 - rust-lang/rust#141001 (Make NonZero<char> possible)
 - rust-lang/rust#141993 (Use the in-tree `compiler-builtins` for the sysroot)
 - rust-lang/rust#142208 (Always consider `const _` items as live for dead code analysis)
 - rust-lang/rust#142238 (stabilize nonnull_provenance)

r? `@ghost`
`@rustbot` modify labels: rollup
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Jun 10, 2025
Rollup of 5 pull requests

Successful merges:

 - rust-lang/rust#140767 (Stabilize `sha512`, `sm3` and `sm4` for x86)
 - rust-lang/rust#141001 (Make NonZero<char> possible)
 - rust-lang/rust#141993 (Use the in-tree `compiler-builtins` for the sysroot)
 - rust-lang/rust#142208 (Always consider `const _` items as live for dead code analysis)
 - rust-lang/rust#142238 (stabilize nonnull_provenance)

r? `@ghost`
`@rustbot` modify labels: rollup
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. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. O-x86_32 Target: x86 processors, 32 bit (like i686-*) (IA-32) O-x86_64 Target: x86-64 processors (like x86_64-*) (also known as amd64 and x64) S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-lang Relevant to the language team
Projects
None yet
Development

Successfully merging this pull request may close these issues.