rustc_target: Add various aarch64 features by mrkajetanp · Pull Request #128192 · rust-lang/rust (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation42 Commits5 Checks6 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

mrkajetanp

Add various aarch64 features already supported by LLVM and Linux.
Additionally include some comment fixes to ensure consistency of feature names with the Arm ARM.
Compiler support for features added to stdarch by rust-lang/stdarch#1614.
Tracking issue for unstable aarch64 features is #127764.

List of added features:

FEAT_FPMR is added in the first commit and then removed in a separate one to highlight it being removed from upstream LLVM 19. The intention is for it to be detectable at runtime through stdarch but not have a corresponding Rust compile-time feature.

@rustbot

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @petrochenkov (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

@rustbot

⚠️ Warning ⚠️

@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

Jul 25, 2024

@rustbot

@rust-log-analyzer

This comment has been minimized.

@mrkajetanp

@mrkajetanp

If I bump stdarch to e98c63893ed6b713b63d1df4882be12cab22495c even on a clean checkout GitHub master this still fails with the same error so I think this is a stdarch problem unrelated to the substance of this PR.

@Amanieu

@Amanieu

It's not clear to me what the plan is for the FPMR feature. If it doesn't make sense to maintain it as a codegen feature in rustc, we could just only have it in std_detect like the v8.x features.

@mrkajetanp

It's not clear to me what the plan is for the FPMR feature. If it doesn't make sense to maintain it as a codegen feature in rustc, we could just only have it in std_detect like the v8.x features.

Yes, that's the approach we settled on as well. The issue is that LLVM 18 versions have it as an explicit feature, while from LLVM 19 onwards it's just supposed to be on by default. I made a separate commit for removing it from the codegen features to highlight this in case someone looks for it later.

With this PR it will not be a Rust codegen feature. We switch it on by default through target specs on LLVM 18, it's already switched on on LLVM 19+ and it doesn't exist on older LLVM versions. But we do keep the detection in std_detect.

@ehuss

Thanks! Will there be a separate tracking issue for these?

@mrkajetanp

Thanks! Will there be a separate tracking issue for these?

Yes, already here: #127764
At least on the detection side. I'm not sure on the compiler side but we could always open one.

@Amanieu

Rather than modifying the target specs, can you instead hard-code rustc to always add that feature when compiling on aarch64 for LLVM <= 18? That makes it easier to remove this in the future once we upgrade our minimum LLVM version to 19.

@nikic

Why do we have to add this feature now at all, when we haven't been enabling it before?

@bors

@mrkajetanp

can you instead hard-code rustc to always add that feature when compiling on aarch64 for LLVM <= 18?

Sure, done. This also made the changes to llvm_util unnecessary so I dropped those as well.

@mrkajetanp

Why do we have to add this feature now at all, when we haven't been enabling it before?

It's a target feature. If we don't handle it explicitly in some way then people who want to make use of it in Rust will not be able to do so until LLVM is bumped to 19 which would not be ideal. Explicitly enabling it ensures consistent behaviour with the upcoming changes.

@rust-log-analyzer

This comment has been minimized.

@bors

@Amanieu

LGTM but this will conflict with #128221. I want to get #128221 merged first, then you can also review the feature dependencies on AArch64.

@rust-log-analyzer

This comment has been minimized.

@mrkajetanp

@Amanieu it's failing because a lot of these features were only added in LLVM 18, should I just make using them conditional on version >= 18?

@mrkajetanp

Convert to_llvm_features to return Option so that it can return None if the requested feature is not available for the current LLVM version.

Add match rules to filter out aarch64 features not available in LLVM 17.

@mrkajetanp

Additionally, remove optional matching for +v8a given that the minimum LLVM version is now past 14.

@mrkajetanp

@rustbot rustbot added S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

and removed S-waiting-on-author

Status: This is awaiting some action (such as code changes or more information) from the author.

labels

Aug 27, 2024

@mrkajetanp

I rebased the branch and updated the tied-features-strength.rs test to drop optional matching for +v8a as well since it looks like it was a leftover todo - we're well past LLVM 14 at this point.

Was a little surprised that the pipeline didn't catch it earlier, looks like it does not run aarch64 codegen tests on pushes to PRs. They're not mentioned here either for what that's worth.

@traviscross

This comment was marked as resolved.

@rustbot rustbot added I-lang-nominated

Nominated for discussion during a lang team meeting.

S-waiting-on-team

Status: Awaiting decision from the relevant subteam (see the T- label).

T-lang

Relevant to the language team, which will review and decide on the PR/issue.

labels

Aug 27, 2024

@traviscross

This comment was marked as resolved.

@rustbot rustbot removed T-lang

Relevant to the language team, which will review and decide on the PR/issue.

I-lang-nominated

Nominated for discussion during a lang team meeting.

S-waiting-on-team

Status: Awaiting decision from the relevant subteam (see the T- label).

labels

Aug 28, 2024

@mrkajetanp

This comment was marked as resolved.

@traviscross

This comment was marked as resolved.

@Amanieu

@bors

📌 Commit 0f871b5 has been approved by Amanieu

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

Aug 28, 2024

workingjubilee added a commit to workingjubilee/rustc that referenced this pull request

Aug 29, 2024

@workingjubilee

rustc_target: Add various aarch64 features

Add various aarch64 features already supported by LLVM and Linux. Additionally include some comment fixes to ensure consistency of feature names with the Arm ARM. Compiler support for features added to stdarch by rust-lang/stdarch#1614. Tracking issue for unstable aarch64 features is rust-lang#127764.

List of added features:

FEAT_FPMR is added in the first commit and then removed in a separate one to highlight it being removed from upstream LLVM 19. The intention is for it to be detectable at runtime through stdarch but not have a corresponding Rust compile-time feature.

This was referenced

Aug 29, 2024

bors added a commit to rust-lang-ci/rust that referenced this pull request

Aug 29, 2024

@bors

rust-timer added a commit to rust-lang-ci/rust that referenced this pull request

Aug 29, 2024

@rust-timer

Rollup merge of rust-lang#128192 - mrkajetanp:feature-detect, r=Amanieu

rustc_target: Add various aarch64 features

Add various aarch64 features already supported by LLVM and Linux. Additionally include some comment fixes to ensure consistency of feature names with the Arm ARM. Compiler support for features added to stdarch by rust-lang/stdarch#1614. Tracking issue for unstable aarch64 features is rust-lang#127764.

List of added features:

FEAT_FPMR is added in the first commit and then removed in a separate one to highlight it being removed from upstream LLVM 19. The intention is for it to be detectable at runtime through stdarch but not have a corresponding Rust compile-time feature.

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.

T-libs

Relevant to the library team, which will review and decide on the PR/issue.