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.

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

Use cfg!(target_feature) for static ARM/AARCH64 feature detection. #1540

Merged
merged 1 commit into from
Oct 1, 2023
Merged

Use cfg!(target_feature) for static ARM/AARCH64 feature detection. #1540

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,8 +193,8 @@ jobs:

rust_channel:
- stable
# Some benchmarking dependencies require 1.60.
- 1.60.0 # MSRV
# Keep in sync with Cargo.toml and similar `rust_channel` sections.
- 1.61.0 # MSRV
# TODO: Move these to a daily/pre-release job.
# - nightly
# - beta
Expand Down Expand Up @@ -345,7 +345,8 @@ jobs:
rust_channel:
- stable
- nightly
- 1.60.0 # MSRV
# Keep in sync with Cargo.toml and similar `rust_channel` sections.
- 1.61.0 # MSRV

include:
- target: aarch64-unknown-linux-musl
Expand Down
5 changes: 3 additions & 2 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@ name = "ring"
readme = "doc/link-to-readme.md"
repository = "https://github.com/briansmith/ring"

# Keep in sync with .github/workflows/ci.yml ("MSRV").
rust-version = "1.60.0"
# Keep in sync with .github/workflows/ci.yml ("MSRV") and see the MSRV note
# in cpu/arm.rs
rust-version = "1.61.0"

# Keep in sync with `links` below.
version = "0.17.0-not-released-yet"
Expand Down
105 changes: 50 additions & 55 deletions src/cpu/arm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,8 @@ pub fn setup() {
#[cfg(all(target_os = "windows", target_arch = "aarch64"))]
pub fn setup() {
// We do not need to check for the presence of NEON, as Armv8-A always has it
let mut features = NEON.mask;
const _ASSERT_NEON_DETECTED: () = assert!((ARMCAP_STATIC & NEON.mask) == NEON.mask);
let mut features = ARMCAP_STATIC;

let result = unsafe {
windows_sys::Win32::System::Threading::IsProcessorFeaturePresent(
Expand All @@ -137,17 +138,8 @@ pub fn setup() {
macro_rules! features {
{
$(
$name:ident {
$target_feature_name:expr => $name:ident {
mask: $mask:expr,

// Should we assume that the feature is always available
// for aarch64-apple-* targets? The first AArch64 iOS
// device used the Apple A7 chip.
// TODO: When we can use `if` in const expressions:
// ```
// aarch64_apple: $aarch64_apple,
// ```
aarch64_apple: true,
}
),+
, // trailing comma is required.
Expand All @@ -159,33 +151,16 @@ macro_rules! features {
};
)+

// TODO: When we can use `if` in const expressions, do this:
// ```
// const ARMCAP_STATIC: u32 = 0
// $(
// | ( if $aarch64_apple &&
// cfg!(all(target_arch = "aarch64",
// target_vendor = "apple")) {
// $name.mask
// } else {
// 0
// }
// )
// )+;
// ```
//
// TODO: Add static feature detection to other targets.
// TODO: Combine static feature detection with runtime feature
// detection.
#[cfg(all(target_arch = "aarch64", target_vendor = "apple"))]
const ARMCAP_STATIC: u32 = 0
$( | $name.mask
)+;
#[cfg(not(all(target_arch = "aarch64", target_vendor = "apple")))]
const ARMCAP_STATIC: u32 = 0;

const _ALL_FEATURES_MASK: u32 = 0
$( | $name.mask
$(
| (
if cfg!(all(any(target_arch = "aarch64", target_arch = "arm"),
target_feature = $target_feature_name)) {
$name.mask
} else {
0
}
)
)+;

#[cfg(all(test, any(target_arch = "arm", target_arch = "aarch64")))]
Expand Down Expand Up @@ -227,29 +202,33 @@ impl Feature {
}
}

// Assumes all target feature names are the same for ARM and AAarch64.
features! {
// Keep in sync with `ARMV7_NEON`.
NEON {
"neon" => NEON {
mask: 1 << 0,
aarch64_apple: true,
},

// Keep in sync with `ARMV8_AES`.
AES {
"aes" => AES {
mask: 1 << 2,
aarch64_apple: true,
},

// Keep in sync with `ARMV8_SHA256`.
SHA256 {
"sha2" => SHA256 {
mask: 1 << 4,
aarch64_apple: true,
},

// Keep in sync with `ARMV8_PMULL`.
PMULL {
//
// TODO(MSRV): There is no "pmull" feature listed from
// `rustc --print cfg --target=aarch64-apple-darwin`. Originally ARMv8 tied
// PMULL detection into AES detection, but later versions split it; see
// https://developer.arm.com/downloads/-/exploration-tools/feature-names-for-a-profile
// "Features introduced prior to 2020." Change this to use "pmull" when
// that is supported.
"aes" => PMULL {
mask: 1 << 5,
aarch64_apple: true,
},
}

Expand All @@ -266,8 +245,32 @@ prefixed_export! {
static mut OPENSSL_armcap_P: u32 = ARMCAP_STATIC;
}

const _APPLE_TARGETS_HAVE_ALL_FEATURES: () = assert!(
(ARMCAP_STATIC == _ALL_FEATURES_MASK)
// MSRV: Enforce 1.61.0 on some aarch64-* targets (aarch64-apple-*, in particular) prior to. Earlier
// versions of Rust before did not report the AAarch64 CPU features correctly for these targets.
// Cargo.toml specifies `rust-version` but versions before Rust 1.56 don't know about it.
Copy link
Owner Author

Choose a reason for hiding this comment

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

I verified that cargo test +1.60.0 --target=aarch64-apple-darwin fails due to this, and cargo test +1.61.0 --target=aarch64-apple-darwin passes.

//
// ```
// $ rustc +1.61.0 --print cfg --target=aarch64-apple-ios | grep -E "neon|aes|sha|pmull"
// target_feature="aes"
// target_feature="neon"
// target_feature="sha2"
// $ rustc +1.61.0 --print cfg --target=aarch64-apple-darwin | grep -E "neon|aes|sha|pmull"
// target_feature="aes"
// target_feature="neon"
// target_feature="sha2"
// target_feature="sha3"
// ```
#[allow(clippy::assertions_on_constants)]
const _NON_AARCH64_HAS_ZERO_ARMCAP_STATIC: () =
assert!((ARMCAP_STATIC == 0) || cfg!(target_arch = "aarch64"));
#[allow(clippy::assertions_on_constants)]
const _AARCH64_HAS_NEON: () =
assert!(((ARMCAP_STATIC & NEON.mask) == NEON.mask) || !cfg!(target_arch = "aarch64"));
#[allow(clippy::assertions_on_constants)]
const _AARCH64_APPLE_FEATURES: u32 = NEON.mask | AES.mask | SHA256.mask | PMULL.mask;
#[allow(clippy::assertions_on_constants)]
const _AARCH64_APPLE_TARGETS_EXPECTED_FEATURES: () = assert!(
((ARMCAP_STATIC & _AARCH64_APPLE_FEATURES) == _AARCH64_APPLE_FEATURES)
|| !cfg!(all(target_arch = "aarch64", target_vendor = "apple"))
);

Expand All @@ -283,14 +286,6 @@ mod tests {
assert_eq!(PMULL.mask, 32);
}

#[cfg(target_vendor = "apple")]
#[test]
fn test_apple_minimum_features() {
ALL_FEATURES.iter().for_each(|feature| {
assert_eq!(ARMCAP_STATIC & feature.mask, feature.mask);
});
}

#[test]
fn test_armcap_static_is_subset_of_armcap_dynamic() {
// Ensure `OPENSSL_armcap_P` is initialized.
Expand Down