Update list of allowed aarch64 features by adamgemmell · Pull Request #84665 · rust-lang/rust

adamgemmell · 2021-04-28T17:36:36Z

I recently added these features to std_detect for aarch64 linux, pending review.

I have commented any features not supported by LLVM 9, the current minimum version for Rust. Some (PAuth at least) were renamed between 9 & 12 and I've left them disabled. TME, however, is not in LLVM 9 but I've left it enabled.

See rust-lang/stdarch#993

These features were recently added to std_detect. Features not supported by LLVM 9, the current minimum version for Rust, are commented.

rust-highfive · 2021-04-28T17:36:39Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @davidtwco (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

adamgemmell · 2021-04-28T17:40:56Z

r? @Amanieu

est31 · 2021-04-28T17:46:32Z

I have commented any features not supported by LLVM 9,

Btw it's 10 (at least on master) #83387

Amanieu · 2021-04-29T10:50:20Z

+ // FEAT_CRC
 ("crc", Some(sym::aarch64_target_feature)),
+ // Cryptographic extension
 ("crypto", Some(sym::aarch64_target_feature)),


LLVM has moved away from a combined "crypto" feature and now uses separate "aes" and "sha2" features. We should do the same here.

I agree - the crypto feature doesn't really mean much any more and the separate features are better. Wouldn't removing this here affect any users detecting this feature? Likewise with removing it from std_detect.

AArch64 target features are still unstable, so it's fine to rename/remove them.

Amanieu · 2021-04-29T11:58:37Z

+ // FEAT_SVE2_BitPerm
+ ("sve2-bitperm", Some(sym::aarch64_target_feature)),
+ // FEAT_FRINTTS
+ ("fptoint", Some(sym::aarch64_target_feature)),


"fptoint" is a poor name for this feature, I prefer the original ARM name "frintts".

Happy to rename this

Amanieu · 2021-04-29T12:06:35Z

 ("aarch64", "fp") => "fp-armv8",
 ("aarch64", "fp16") => "fullfp16",
+ ("aarch64", "fhm") => "fp16fml",
+ ("aarch64", "lse2") => "outline-atomics",


lse2 doesn't have an LLVM equivalent feature: it doesn't add any new instructions, it just relaxes the restrictions on existing atomic instructions.
outline-atomics is something different: it's a codegen option on LLVM to call external functions for atomic operations so that they can take advantage of LSE on targets that support it.

Nice catch, thanks a lot!

adamgemmell · 2021-04-29T16:55:20Z

Btw it's 10 (at least on master) #83387

There happens to be no relevant features added in v10. Though it does mean TME is supported on all supported LLVM versions.

Dylan-DPC-zz · 2021-05-19T11:20:22Z

@adamgemmell any updates on the changes requested?

…m/aarch64

adamgemmell · 2021-05-19T15:32:28Z

@Dylan-DPC here we go

Removing crypto is a little weird as there's a dependency on it in stdarch which is a submodule of rust, and of course we're bootstrapping.

We commit this, which adds AES/SHA2 features in the compiler for arm/aarch64, and removes a test for the crypto feature in std_detect.
We commit the stdarch patch once there's a nightly build of this available.
I have a 2nd patch which removes crypto from the list of arm/aarch64 allowed features and adds a few more std tests for the new features (which are really just duplicates of the ones in std_detect, but consistency is nice). This can be committed when or after the submodule for stdarch is updated.

Amanieu · 2021-05-19T16:56:37Z

@bors r+

bors · 2021-05-19T16:56:38Z

📌 Commit d3737a6 has been approved by Amanieu

…nieu Update list of allowed aarch64 features I recently added these features to std_detect for aarch64 linux, pending [review](rust-lang/stdarch#1146). I have commented any features not supported by LLVM 9, the current minimum version for Rust. Some (PAuth at least) were renamed between 9 & 12 and I've left them disabled. TME, however, is not in LLVM 9 but I've left it enabled. See rust-lang/stdarch#993

bors · 2021-05-20T03:52:15Z

⌛ Testing commit d3737a6 with merge feda7ff79fcad3576c95c6336638c2398ac44b29...

bors · 2021-05-20T04:58:09Z

💔 Test failed - checks-actions

rust-log-analyzer · 2021-05-20T04:58:11Z

A job failed! Check out the build log: (web)(plain)

Click to see the possible cause of the failure (guessed by this bot)

adamgemmell · 2021-05-20T11:52:48Z

This looks like https://rust-lang.zulipchat.com/#narrow/stream/242791-t-infra/topic/No.20macos.20runners.20for.20bors.3F

Could I get an @bors retry please?

bors · 2021-05-20T11:52:49Z

@adamgemmell: 🔑 Insufficient privileges: not in try users

est31 · 2021-05-20T11:54:36Z

@bors retry

bors · 2021-05-20T13:07:41Z

⌛ Testing commit d3737a6 with merge 8a57820...

bors · 2021-05-20T15:49:26Z

☀️ Test successful - checks-actions
Approved by: Amanieu
Pushing 8a57820 to master...

Update list of allowed aarch64 features
3f5f54c
These features were recently added to std_detect. Features not supported by LLVM 9, the current minimum version for Rust, are commented.

rust-highfive assigned davidtwco Apr 28, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 28, 2021

adamgemmell mentioned this pull request Apr 28, 2021
Update aarch64 linux feature detection rust-lang/stdarch#1146
Merged

rust-highfive assigned Amanieu and unassigned davidtwco Apr 28, 2021

Amanieu reviewed Apr 29, 2021
View reviewed changes

JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 17, 2021

Dylan-DPC-zz added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 19, 2021

adamgemmell added 4 commits May 19, 2021 16:11

Remove LSE2
523b4d1

Rename fptoint to frintts
c71e58d

Ensure all crypto components (AES, PMULL, SHA1/2) are available on ar…
904467a
…m/aarch64

Remove test for crypto feature ahead of its removal
d3737a6

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 19, 2021

Dylan-DPC-zz mentioned this pull request May 19, 2021
Rollup of 13 pull requests #85483
Closed

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 20, 2021

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 May 20, 2021

bors added the merged-by-bors This PR was explicitly merged by bors. label May 20, 2021

bors merged commit 8a57820 into rust-lang:masterMay 20, 2021

rustbot added this to the 1.54.0 milestone May 20, 2021

adamgemmell deleted the aarch64-features branch March 1, 2022 13:10

taiki-e mentioned this pull request Apr 1, 2023
Remove uses of arm crypto target feature rust-lang/stdarch#1407
Merged

Uh oh!

Uh oh!

Conversation

adamgemmell commented Apr 28, 2021

Uh oh!

rust-highfive commented Apr 28, 2021

Uh oh!

adamgemmell commented Apr 28, 2021

Uh oh!

est31 commented Apr 28, 2021

Uh oh!

AmanieuApr 29, 2021

Choose a reason for hiding this comment

Uh oh!

adamgemmellApr 29, 2021

Choose a reason for hiding this comment

Uh oh!

AmanieuApr 29, 2021

Choose a reason for hiding this comment

Uh oh!

AmanieuApr 29, 2021

Choose a reason for hiding this comment

Uh oh!

adamgemmellApr 29, 2021

Choose a reason for hiding this comment

Uh oh!

AmanieuApr 29, 2021

Choose a reason for hiding this comment

Uh oh!

adamgemmellApr 29, 2021

Choose a reason for hiding this comment

Uh oh!

adamgemmell commented Apr 29, 2021

Uh oh!

Dylan-DPC-zz commented May 19, 2021

Uh oh!

adamgemmell commented May 19, 2021

Uh oh!

Amanieu commented May 19, 2021

Uh oh!

bors commented May 19, 2021

Uh oh!

bors commented May 20, 2021

Uh oh!

bors commented May 20, 2021

Uh oh!

rust-log-analyzer commented May 20, 2021

Uh oh!

adamgemmell commented May 20, 2021

Uh oh!

bors commented May 20, 2021

Uh oh!

est31 commented May 20, 2021

Uh oh!

bors commented May 20, 2021

Uh oh!

bors commented May 20, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants