-
Notifications
You must be signed in to change notification settings - Fork 798
_AARCH64_APPLE_DARWIN_TARGETS_EXPECTED_FEATURES failure #2872
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
Open
mahogny
wants to merge
2
commits into
briansmith:main
Choose a base branch
from
mahogny:ci/target-cpu-regression
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+109
−22
Open
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| # Reproduces the aarch64-apple `-Ctarget-cpu` bug fixed in | ||
| # src/cpu/aarch64/darwin.rs. Without that fix these jobs fail with: | ||
| # | ||
| # error[E0080]: evaluation panicked: assertion failed: | ||
| # (CAPS_STATIC & MIN_STATIC_FEATURES) == MIN_STATIC_FEATURES | ||
| # | ||
| # `native` is what users pass, but which CPU LLVM picks for it depends on the | ||
| # runner (see https://github.com/rust-lang/rust/issues/93889), so `generic` is | ||
| # the deterministic case; both are tested. | ||
| name: target-cpu-regression | ||
| permissions: | ||
| contents: read | ||
| on: | ||
| pull_request: | ||
| push: | ||
| workflow_dispatch: | ||
| jobs: | ||
| apple: | ||
| runs-on: macos-15 | ||
|
|
||
| env: | ||
| CC_ENABLE_DEBUG_OUTPUT: 1 | ||
| RUSTFLAGS: -Ctarget-cpu=${{ matrix.target_cpu }} | ||
|
|
||
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| target_cpu: | ||
| - generic | ||
| - native | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 | ||
| with: | ||
| persist-credentials: false | ||
|
|
||
| - run: mk/install-build-tools.sh +stable --target=aarch64-apple-darwin | ||
| shell: sh | ||
|
|
||
| - run: | | ||
| echo "RUSTFLAGS=${RUSTFLAGS}" | ||
| rustc --print cfg --target=aarch64-apple-darwin ${RUSTFLAGS} | grep target_feature | ||
| mk/cargo.sh +stable test --locked -vv --lib --tests --target=aarch64-apple-darwin |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,17 +17,40 @@ use super::{Aes, CAPS_STATIC, Neon, PMull, Sha256}; | |
| use {super::Sha512, core::ffi::CStr}; | ||
|
|
||
| // ``` | ||
| // $ rustc +1.61.0 --print cfg --target=aarch64-apple-ios | grep -E "neon|aes|sha|pmull" | ||
| // $ rustc --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" | ||
| // $ rustc --print cfg --target=aarch64-apple-darwin | grep -E "neon|aes|sha|pmull" | ||
| // target_feature="aes" | ||
| // target_feature="neon" | ||
| // target_feature="sha2" | ||
| // target_feature="sha3" | ||
| // ``` | ||
| // | ||
| // Every aarch64-apple-* device has AES, PMULL and SHA-256, and by default the | ||
| // compiler tells us so. However, we cannot *rely* on being told, because | ||
| // `-Ctarget-cpu` can leave the target feature set *smaller* than the target's | ||
| // default: | ||
| // | ||
| // ``` | ||
| // $ rustc --print cfg --target=aarch64-apple-darwin -Ctarget-cpu=generic | grep -E "neon|aes|sha" | ||
| // target_feature="neon" | ||
| // ``` | ||
| // | ||
| // On Apple Silicon, `-Ctarget-cpu=native` makes LLVM pick an older CPU than | ||
| // the default one (`cyclone`, or `generic` on hosts whose CPU LLVM does not | ||
| // recognize, e.g. virtualized CI runners); see | ||
| // https://github.com/rust-lang/rust/issues/93889. Users also disable target | ||
| // features explicitly. Formerly such a configuration was a hard compile error | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If users want to do |
||
| // here; instead, detect at runtime whatever the compiler didn't promise us, | ||
| // like we do for the other operating systems. | ||
| // | ||
| // This is the mirror image of the workaround in `linux.rs`: there | ||
| // `-Ctarget-cpu` reports *more* features than the CPU actually has, so the | ||
| // static feature set cannot be trusted to be correct; here it reports *fewer*, | ||
| // so it cannot be trusted to be complete. | ||
| // | ||
| // XXX/TODO(coverage)/TODO(size): aarch64-apple-darwin is statically guaranteed to have "sha3" but | ||
| // other aarch64-apple-* targets require dynamic detection. Since we don't have test coverage for | ||
| // the other targets yet, we wouldn't have a way of testing the dynamic detection if we statically | ||
|
|
@@ -36,23 +59,8 @@ use {super::Sha512, core::ffi::CStr}; | |
| // | ||
| // This is particularly important because we haven't tested the ABI validity of any | ||
| // fallback implementations, especially for ARM64_32. | ||
| pub const MIN_STATIC_FEATURES: u32 = Neon::mask() | Aes::mask() | Sha256::mask() | PMull::mask(); | ||
| pub const FORCE_DYNAMIC_DETECTION: u32 = !MIN_STATIC_FEATURES; | ||
|
|
||
| // MSRV: Enforce 1.61.0 onaarch64-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. | ||
| #[allow(clippy::assertions_on_constants)] | ||
| const _AARCH64_APPLE_TARGETS_EXPECTED_FEATURES: () = | ||
| assert!((CAPS_STATIC & MIN_STATIC_FEATURES) == MIN_STATIC_FEATURES); | ||
|
|
||
| // Ensure we don't accidentally allow features statically beyond | ||
| // `MIN_STATIC_FEATURES` so that dynamic detection is done uniformly for | ||
| // all of these targets. | ||
| #[allow(clippy::assertions_on_constants)] | ||
| const _AARCH64_APPLE_DARWIN_TARGETS_EXPECTED_FEATURES: () = | ||
| assert!(CAPS_STATIC == MIN_STATIC_FEATURES); | ||
| pub const FORCE_DYNAMIC_DETECTION: u32 = | ||
| !(Neon::mask() | Aes::mask() | Sha256::mask() | PMull::mask()); | ||
|
|
||
| pub fn detect_features() -> u32 { | ||
| #[cfg(all(target_pointer_width = "64", not(target_os = "watchos")))] | ||
|
|
@@ -88,6 +96,26 @@ pub fn detect_features() -> u32 { | |
|
|
||
| #[cfg(all(target_pointer_width = "64", not(target_os = "watchos")))] | ||
| { | ||
| // Only ask the OS about the features the compiler didn't already | ||
| // promise us. The `cfg!`s are constant-folded, so a | ||
| // default-configured build makes no additional `sysctl` calls. | ||
| // | ||
| // The `hw.optional.arm.FEAT_*` names require macOS 12 / iOS 15 or | ||
| // later. On earlier versions the lookup fails and we fall back to the | ||
| // implementations that don't require the feature; slower, but correct. | ||
| if !cfg!(target_feature = "aes") { | ||
| // `STATIC_DETECTED` derives `PMull` from "aes" since there is no | ||
| // "pmull" target feature, but the OS reports them separately. | ||
| if detect_feature(c"hw.optional.arm.FEAT_AES") { | ||
| features |= Aes::mask(); | ||
| } | ||
| if detect_feature(c"hw.optional.arm.FEAT_PMULL") { | ||
| features |= PMull::mask(); | ||
| } | ||
| } | ||
| if !cfg!(target_feature = "sha2") && detect_feature(c"hw.optional.arm.FEAT_SHA256") { | ||
| features |= Sha256::mask(); | ||
| } | ||
| if detect_feature(c"hw.optional.armv8_2_sha512") { | ||
| features |= Sha512::mask(); | ||
| } | ||
|
|
@@ -111,11 +139,11 @@ mod tests { | |
| let has_sha512 = maybe_sha512.is_some(); | ||
|
|
||
| // Intentionally defer the assertion to runtime instead of compile time. | ||
| #[allow(clippy::assertions_on_constants)] | ||
| #[cfg(all(target_os = "macos", target_pointer_width = "64"))] | ||
| { | ||
| // All aarch64-apple-darwin targets have SHA3 enabled statically... | ||
| assert!(cfg!(target_feature = "sha3")); | ||
| // Whether "sha3" is enabled statically depends on `-Ctarget-cpu`, | ||
| // but every aarch64-apple-darwin device has SHA-512 regardless, so | ||
| // we must detect it either way. | ||
| assert_eq!(has_sha512, cfg!(target_pointer_width = "64")); | ||
| } | ||
|
|
||
|
|
@@ -124,4 +152,20 @@ mod tests { | |
| assert!(!has_sha512); | ||
| } | ||
| } | ||
|
|
||
| // Every aarch64-apple-* device has these, so we must find them whether or | ||
| // not the compiler enabled them statically; see the comment on | ||
| // `FORCE_DYNAMIC_DETECTION` above. | ||
| #[test] | ||
| #[cfg(all(target_pointer_width = "64", not(target_os = "watchos")))] | ||
| #[cfg(not(feature = "unstable-testing-arm-no-hw"))] | ||
| fn aes_pmull_sha256_detection() { | ||
| let cpu = cpu::features(); | ||
| let aes: Option<Aes> = cpu.get_feature(); | ||
| let pmull: Option<PMull> = cpu.get_feature(); | ||
| let sha256: Option<Sha256> = cpu.get_feature(); | ||
| assert!(aes.is_some()); | ||
| assert!(pmull.is_some()); | ||
| assert!(sha256.is_some()); | ||
| } | ||
| } | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't intend to support
-Ctarget-cpu=genericon apple hardware.