Skip to content

Stabilize the avx512 target features #138940

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Stabilize the avx512 target features #138940

wants to merge 2 commits into from
+63 −73

Conversation

sayantn
Copy link
Contributor

@sayantn sayantn commented Mar 25, 2025 •
edited
Loading

This PR stabilizes the AVX512 target features - see this comment.

Tracking Issue - #44839

DO NOT MERGE now, because the runtime detection for 5 features are guarded on avx512_target_feature, so rust-lang/stdarch#1757 has a temporary fix which changes the feature name of these. We need to FCP these together with all the AVX512 target features, after that and merge of rust-lang/stdarch#1760, we can again do a stdarch update and merge this.

The target feature UI tests have been changed to x87 (chosen because this is very unlikely to stablize ever, please comment if some other feature will be better)

related: #111137

@rustbot
Copy link
Collaborator

rustbot commented Mar 25, 2025

r? @davidtwco

rustbot has assigned @davidtwco.
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
Copy link
Collaborator

rustbot commented Mar 25, 2025

⚠️ Warning ⚠️

  • These commits modify submodules.

@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. labels Mar 25, 2025
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@sayantn

This comment was marked as resolved.

Sign in to view
@sayantn sayantn force-pushed the stabilize-avx512 branch 2 times, most recently from 1be9f75 to 94d0e36 Compare March 25, 2025 19:51
@rustbot
Copy link
Collaborator

rustbot commented Mar 25, 2025

The Miri subtree was changed

cc @rust-lang/miri

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@sayantn sayantn mentioned this pull request Mar 25, 2025
Open
@Amanieu Amanieu added the I-lang-nominated Nominated for discussion during a lang team meeting. label Mar 25, 2025
@Amanieu
Copy link
Member

Amanieu commented Mar 25, 2025

Nominating for T-lang FCP. This stabilizes the AVX512 target features which is a per-requisite for stabilizing the AVX-512 intrinsics tracked in #111137.

@Amanieu Amanieu added T-lang Relevant to the language team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. 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 Mar 25, 2025
@ehuss
Copy link
Contributor

ehuss commented Mar 26, 2025

Can you also send a PR to update https://github.com/rust-lang/reference/blob/master/src/attributes/codegen.md#x86-or-x86_64?

@sayantn sayantn mentioned this pull request Apr 2, 2025
Open
@sayantn sayantn force-pushed the stabilize-avx512 branch from 94d0e36 to a3c822a Compare April 3, 2025 12:13
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@Amanieu Amanieu mentioned this pull request Apr 8, 2025
Open
2 tasks
Amanieu
Amanieu approved these changes Apr 9, 2025
View reviewed changes
Copy link
Member

@Amanieu Amanieu left a comment

Choose a reason for hiding this comment

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

These features are for the various AVX512 sub-features. We have already stabilized some of these names in is_x86_feature_detected! so the language level name should match.

@rustbot
Copy link
Collaborator

rustbot commented Apr 10, 2025

⚠️ Warning ⚠️

  • Some commits in this PR modify submodules.

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@sayantn sayantn force-pushed the stabilize-avx512 branch 3 times, most recently from bd21299 to 308b82c Compare April 12, 2025 14:25
@tgross35
Copy link
Contributor

Requesting a lang reviewer since this needs their FCP.

r? lang

@rustbot rustbot assigned nikomatsakis and unassigned davidtwco Apr 14, 2025
@traviscross traviscross removed the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Apr 16, 2025
@joshtriplett
Copy link
Member

Looks good.

@rfcbot merge

@rfcbot
Copy link
Collaborator

rfcbot commented Apr 16, 2025 •
edited by scottmcm
Loading

Team member @joshtriplett 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 Apr 16, 2025
@traviscross
Copy link
Contributor

@rfcbot reviewed

1 similar comment
@tmandry
Copy link
Member

tmandry commented Apr 16, 2025

@rfcbot reviewed

@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Apr 16, 2025
@rfcbot
Copy link
Collaborator

rfcbot commented Apr 16, 2025

🔔 This is now entering its final comment period, as per the review above. 🔔

@joshtriplett joshtriplett added the relnotes Marks issues that should be documented in the release notes of the next release. label Apr 16, 2025
@rustbot rustbot mentioned this pull request Apr 16, 2025
Open
@traviscross traviscross removed the I-lang-nominated Nominated for discussion during a lang team meeting. label Apr 16, 2025
@nikomatsakis
Copy link
Contributor

@rfcbot reviewed

@bors
Copy link
Collaborator

bors commented Apr 16, 2025

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

This was referenced Apr 17, 2025
Open
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RalfJung RalfJung RalfJung left review comments

@Amanieu Amanieu Amanieu approved these changes

Assignees

@nikomatsakis nikomatsakis

Labels
disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.