Skip to content
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

Update aarch64 linux feature detection #1146

Merged
merged 9 commits into from
May 28, 2021

Conversation

adamgemmell
Copy link
Contributor

@adamgemmell adamgemmell commented Apr 28, 2021

All features supported by auxv (see https://github.com/torvalds/linux/blob/master/arch/arm64/include/uapi/asm/hwcap.h) / cpuinfo (https://github.com/torvalds/linux/blob/master/arch/arm64/kernel/cpuinfo.c) and in LLVM 12 are added.

See llvm-project/llvm/lib/Target/AArch64/AArch64.td for the LLVM feature names and dependencies. We've done a few passes for silly cross-referencing mistakes ourselves so I'm reasonably confident in its accuracy.

I had a couple notes after working on this:

  • Some features, though detection is easy, are still not added based on @Amanieu 's wording here. Is it possible to add detection for a few of the remaining features but not add them to the allowed list in rustc? I don't see why users shouldn't be able to check for these features themselves.
  • usize::BITS is stabilised but is not yet in stable Rust. I don't THINK this is a problem but wanted to highlight it.

See #993
See also: rust-lang/rust#84665

@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Amanieu (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.

@SparrowLii
Copy link
Member

This change makes your PR failed to compile. You can rebase to the latest master branch to avoid this error.

@FEATURE: #[unstable(feature = "stdsimd", issue = "27731")] crypto: "crypto";
/// Crypto: AES + PMULL + SHA1 + SHA2
/// Cryptographic Extension (AES + PMULL + SHA1 + SHA2-256)
Copy link
Member

Choose a reason for hiding this comment

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

As per my comment in rust-lang/rust#84665, we should remove crypto and instead only expose aes and sha2.

@adamgemmell
Copy link
Contributor Author

These changes will only work when my changes to rustc make it to the nightly compiler, see rust-lang/rust#84665

Hence no reason to run the CI yet

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request May 19, 2021
…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 added a commit to rust-lang-ci/rust that referenced this pull request May 20, 2021
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
@adamgemmell
Copy link
Contributor Author

I missed this in my testing but armv7 still requires crypto for the intrinsics. This was fixed in llvm/llvm-project@b8baa2a#diff-e65dd2f7194df68bc6e9b754449ae241a6717f77ca1a28ee9479990ffe292049R7230 but that commit needs to make it to the minimum LLVM version before it can be removed fully

@Amanieu
Copy link
Member

Amanieu commented May 27, 2021

We can cherry-pick that commit into rust-lang/llvm-project and then update the llvm-project submodule in rust-lang/rust.

@Amanieu
Copy link
Member

Amanieu commented May 27, 2021

Hmm on second thought that might be a breaking change that would prevent cross-lang LTO with Clang 12. So in the end we will most likely have to wait for LLVM 13.

@Amanieu Amanieu merged commit 0087304 into rust-lang:master May 28, 2021
@adamgemmell adamgemmell deleted the aarch64-detect branch April 18, 2023 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants