diff options
| author | Dylan DPC <99973273+Dylan-DPC@users.noreply.github.com> | 2022-03-23 03:05:28 +0100 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2022-03-23 03:05:28 +0100 |
| commit | 67d6cc6ef3883b6598ee6347f119cdfd7c4029b8 (patch) | |
| tree | cd7c42898e6b8471c2732159900687c3637978da | |
| parent | a4a5e79814fb4d1568fb0ea5ca50f810b071ae12 (diff) | |
| parent | 6c19dc9a8662999e28b2084f072532630a4c2fc9 (diff) | |
| download | rust-67d6cc6ef3883b6598ee6347f119cdfd7c4029b8.tar.gz rust-67d6cc6ef3883b6598ee6347f119cdfd7c4029b8.zip | |
Rollup merge of #91608 - workingjubilee:fold-neon-fp, r=nagisa,Amanieu
Fold aarch64 feature +fp into +neon Arm's FEAT_FP and Feat_AdvSIMD describe the same thing on AArch64: The Neon unit, which handles both floating point and SIMD instructions. Moreover, a configuration for AArch64 must include both or neither. Arm says "entirely proprietary" toolchains may omit floating point: https://developer.arm.com/documentation/102374/0101/Data-processing---floating-point In the Programmer's Guide for Armv8-A, Arm says AArch64 can have both FP and Neon or neither in custom implementations: https://developer.arm.com/documentation/den0024/a/AArch64-Floating-point-and-NEON In "Bare metal boot code for Armv8-A", enabling Neon and FP is just disabling the same trap flag: https://developer.arm.com/documentation/dai0527/a In an unlikely future where "Neon and FP" become unrelated, we can add "[+-]fp" as its own feature flag. Until then, we can simplify programming with Rust on AArch64 by folding both into "[+-]neon", which is valid as it supersets both. "[+-]neon" is retained for niche uses such as firmware, kernels, "I just hate floats", and so on. I am... pretty sure no one is relying on this. An argument could be made that, as we are not an "entirely proprietary" toolchain, we should not support AArch64 without floats at all. I think that's a bit excessive. However, I want to recognize the intent: programming for AArch64 should be simplified where possible. For x86-64, programmers regularly set up illegal feature configurations because it's hard to understand them, see https://github.com/rust-lang/rust/issues/89586. And per the above notes, plus the discussion in https://github.com/rust-lang/rust/issues/86941, there should be no real use cases for leaving these features split: the two should in fact always go together. - Fixes rust-lang/rust#95002. - Fixes rust-lang/rust#95064. - Fixes rust-lang/rust#95122.
| -rw-r--r-- | compiler/rustc_codegen_llvm/src/llvm_util.rs | 10 | ||||
| -rw-r--r-- | compiler/rustc_codegen_ssa/src/target_features.rs | 5 | ||||
| -rw-r--r-- | compiler/rustc_target/src/asm/aarch64.rs | 2 | ||||
| -rw-r--r-- | library/std/tests/run-time-detect.rs | 1 | ||||
| -rw-r--r-- | src/test/run-make-fulldeps/simd-ffi/Makefile | 2 | ||||
| -rw-r--r-- | src/test/ui/asm/aarch64/bad-reg.rs | 2 | ||||
| -rw-r--r-- | src/test/ui/target-feature/aarch64-neon-works.rs | 23 | ||||
| -rw-r--r-- | src/test/ui/target-feature/feature-hierarchy.rs | 58 | ||||
| -rw-r--r-- | src/test/ui/target-feature/no-llvm-leaks.rs | 64 |
9 files changed, 155 insertions, 12 deletions
diff --git a/compiler/rustc_codegen_llvm/src/llvm_util.rs b/compiler/rustc_codegen_llvm/src/llvm_util.rs index 3ce594b945a..abcdb81c0e2 100644 --- a/compiler/rustc_codegen_llvm/src/llvm_util.rs +++ b/compiler/rustc_codegen_llvm/src/llvm_util.rs @@ -187,7 +187,6 @@ pub fn to_llvm_features<'a>(sess: &Session, s: &'a str) -> SmallVec<[&'a str; 2] ("x86", "avx512vaes") => smallvec!["vaes"], ("x86", "avx512gfni") => smallvec!["gfni"], ("x86", "avx512vpclmulqdq") => smallvec!["vpclmulqdq"], - ("aarch64", "fp") => smallvec!["fp-armv8"], ("aarch64", "rcpc2") => smallvec!["rcpc-immo"], ("aarch64", "dpb") => smallvec!["ccpp"], ("aarch64", "dpb2") => smallvec!["ccdp"], @@ -230,6 +229,8 @@ pub fn check_tied_features( None } +// Used to generate cfg variables and apply features +// Must express features in the way Rust understands them pub fn target_features(sess: &Session) -> Vec<Symbol> { let target_machine = create_informational_target_machine(sess); let mut features: Vec<Symbol> = @@ -239,13 +240,14 @@ pub fn target_features(sess: &Session) -> Vec<Symbol> { if sess.is_nightly_build() || gate.is_none() { Some(feature) } else { None } }) .filter(|feature| { + // check that all features in a given smallvec are enabled for llvm_feature in to_llvm_features(sess, feature) { let cstr = SmallCStr::new(llvm_feature); - if unsafe { llvm::LLVMRustHasFeature(target_machine, cstr.as_ptr()) } { - return true; + if !unsafe { llvm::LLVMRustHasFeature(target_machine, cstr.as_ptr()) } { + return false; } } - false + true }) .map(|feature| Symbol::intern(feature)) .collect(); diff --git a/compiler/rustc_codegen_ssa/src/target_features.rs b/compiler/rustc_codegen_ssa/src/target_features.rs index dd214d96166..ba1e1862227 100644 --- a/compiler/rustc_codegen_ssa/src/target_features.rs +++ b/compiler/rustc_codegen_ssa/src/target_features.rs @@ -43,10 +43,8 @@ const ARM_ALLOWED_FEATURES: &[(&str, Option<Symbol>)] = &[ ]; const AARCH64_ALLOWED_FEATURES: &[(&str, Option<Symbol>)] = &[ - // FEAT_AdvSimd + // FEAT_AdvSimd & FEAT_FP ("neon", None), - // FEAT_FP - ("fp", None), // FEAT_FP16 ("fp16", None), // FEAT_SVE @@ -143,7 +141,6 @@ const AARCH64_ALLOWED_FEATURES: &[(&str, Option<Symbol>)] = &[ ]; const AARCH64_TIED_FEATURES: &[&[&str]] = &[ - &["fp", "neon"], // Silicon always has both, so avoid needless complications &["paca", "pacg"], // Together these represent `pauth` in LLVM ]; diff --git a/compiler/rustc_target/src/asm/aarch64.rs b/compiler/rustc_target/src/asm/aarch64.rs index 7fb4dbdf2b1..fba8cc6ef8b 100644 --- a/compiler/rustc_target/src/asm/aarch64.rs +++ b/compiler/rustc_target/src/asm/aarch64.rs @@ -64,7 +64,7 @@ impl AArch64InlineAsmRegClass { match self { Self::reg => types! { _: I8, I16, I32, I64, F32, F64; }, Self::vreg | Self::vreg_low16 => types! { - fp: I8, I16, I32, I64, F32, F64, + neon: I8, I16, I32, I64, F32, F64, VecI8(8), VecI16(4), VecI32(2), VecI64(1), VecF32(2), VecF64(1), VecI8(16), VecI16(8), VecI32(4), VecI64(2), VecF32(4), VecF64(2); }, diff --git a/library/std/tests/run-time-detect.rs b/library/std/tests/run-time-detect.rs index 54873f5549b..a57a52d9bb0 100644 --- a/library/std/tests/run-time-detect.rs +++ b/library/std/tests/run-time-detect.rs @@ -29,7 +29,6 @@ fn aarch64_linux() { println!("neon: {}", is_aarch64_feature_detected!("neon")); println!("asimd: {}", is_aarch64_feature_detected!("asimd")); println!("pmull: {}", is_aarch64_feature_detected!("pmull")); - println!("fp: {}", is_aarch64_feature_detected!("fp")); println!("fp16: {}", is_aarch64_feature_detected!("fp16")); println!("sve: {}", is_aarch64_feature_detected!("sve")); println!("crc: {}", is_aarch64_feature_detected!("crc")); diff --git a/src/test/run-make-fulldeps/simd-ffi/Makefile b/src/test/run-make-fulldeps/simd-ffi/Makefile index 38f2fcd18c5..e9c974a0137 100644 --- a/src/test/run-make-fulldeps/simd-ffi/Makefile +++ b/src/test/run-make-fulldeps/simd-ffi/Makefile @@ -41,7 +41,7 @@ define MK_TARGETS # now. $(1): simd.rs $$(RUSTC) --target=$(1) --emit=llvm-ir,asm simd.rs \ - -C target-feature='+fp,+neon,+sse2' -C extra-filename=-$(1) + -C target-feature='+neon,+sse2' -C extra-filename=-$(1) endef $(foreach targetxxx,$(TARGETS),$(eval $(call MK_TARGETS,$(targetxxx)))) diff --git a/src/test/ui/asm/aarch64/bad-reg.rs b/src/test/ui/asm/aarch64/bad-reg.rs index 8619b3960a6..1a314101916 100644 --- a/src/test/ui/asm/aarch64/bad-reg.rs +++ b/src/test/ui/asm/aarch64/bad-reg.rs @@ -1,5 +1,5 @@ // only-aarch64 -// compile-flags: -C target-feature=+fp +// compile-flags: -C target-feature=+neon #![feature(asm_const, asm_sym)] diff --git a/src/test/ui/target-feature/aarch64-neon-works.rs b/src/test/ui/target-feature/aarch64-neon-works.rs new file mode 100644 index 00000000000..3878806fd02 --- /dev/null +++ b/src/test/ui/target-feature/aarch64-neon-works.rs @@ -0,0 +1,23 @@ +// only-aarch64 +// run-pass +#![allow(dead_code)] +use std::arch::*; +use std::arch::aarch64::*; + +// Smoke test to verify aarch64 code that enables NEON compiles. +fn main() { + let _zero = if is_aarch64_feature_detected!("neon") { + unsafe { + let zeros = zero_vector(); + vgetq_lane_u8::<1>(zeros) + } + } else { + 0 + }; +} + + +#[target_feature(enable = "neon")] +unsafe fn zero_vector() -> uint8x16_t { + vmovq_n_u8(0) +} diff --git a/src/test/ui/target-feature/feature-hierarchy.rs b/src/test/ui/target-feature/feature-hierarchy.rs new file mode 100644 index 00000000000..5fbd5e8a28d --- /dev/null +++ b/src/test/ui/target-feature/feature-hierarchy.rs @@ -0,0 +1,58 @@ +// revisions: aarch64-neon aarch64-sve2 +// [aarch64-neon] compile-flags: -Ctarget-feature=+neon --target=aarch64-unknown-linux-gnu +// [aarch64-neon] needs-llvm-components: aarch64 +// [aarch64-sve2] compile-flags: -Ctarget-feature=-neon,+sve2 --target=aarch64-unknown-linux-gnu +// [aarch64-sve2] needs-llvm-components: aarch64 +// build-pass +#![no_core] +#![crate_type = "rlib"] +#![feature(intrinsics, rustc_attrs, no_core, lang_items, staged_api)] +#![stable(feature = "test", since = "1.0.0")] + +// Tests vetting "feature hierarchies" in the cases where we impose them. + +// Supporting minimal rust core code +#[lang = "sized"] +trait Sized {} +#[lang = "copy"] +trait Copy {} +impl Copy for bool {} + +extern "rust-intrinsic" { + #[rustc_const_stable(feature = "test", since = "1.0.0")] + fn unreachable() -> !; +} + +#[rustc_builtin_macro] +macro_rules! cfg { + ($($cfg:tt)*) => {}; +} + +// Test code +const fn do_or_die(cond: bool) { + if cond { + } else { + unsafe { unreachable() } + } +} + +macro_rules! assert { + ($x:expr $(,)?) => { + const _: () = do_or_die($x); + }; +} + + +#[cfg(aarch64_neon)] +fn check_neon_not_sve2() { + // This checks that a normal aarch64 target doesn't suddenly jump up the feature hierarchy. + assert!(cfg!(target_feature = "neon")); + assert!(cfg!(not(target_feature = "sve2"))); +} + +#[cfg(aarch64_sve2)] +fn check_sve2_includes_neon() { + // This checks that aarch64's sve2 includes neon + assert!(cfg!(target_feature = "neon")); + assert!(cfg!(target_feature = "sve2")); +} diff --git a/src/test/ui/target-feature/no-llvm-leaks.rs b/src/test/ui/target-feature/no-llvm-leaks.rs new file mode 100644 index 00000000000..5a71b2166c3 --- /dev/null +++ b/src/test/ui/target-feature/no-llvm-leaks.rs @@ -0,0 +1,64 @@ +// revisions: aarch64 x86-64 +// [aarch64] compile-flags: -Ctarget-feature=+neon,+fp16,+fhm --target=aarch64-unknown-linux-gnu +// [aarch64] needs-llvm-components: aarch64 +// [x86-64] compile-flags: -Ctarget-feature=+sse4.2,+rdrand --target=x86_64-unknown-linux-gnu +// [x86-64] needs-llvm-components: x86 +// build-pass +#![no_core] +#![crate_type = "rlib"] +#![feature(intrinsics, rustc_attrs, no_core, lang_items, staged_api)] +#![stable(feature = "test", since = "1.0.0")] + +// Supporting minimal rust core code +#[lang = "sized"] +trait Sized {} +#[lang = "copy"] +trait Copy {} +impl Copy for bool {} + +extern "rust-intrinsic" { + #[rustc_const_stable(feature = "test", since = "1.0.0")] + fn unreachable() -> !; +} + +#[rustc_builtin_macro] +macro_rules! cfg { + ($($cfg:tt)*) => {}; +} + +// Test code +const fn do_or_die(cond: bool) { + if cond { + } else { + unsafe { unreachable() } + } +} + +macro_rules! assert { + ($x:expr $(,)?) => { + const _: () = do_or_die($x); + }; +} + + +#[cfg(target_arch = "aarch64")] +fn check_aarch64() { + // This checks that the rustc feature name is used, not the LLVM feature. + assert!(cfg!(target_feature = "neon")); + assert!(cfg!(not(target_feature = "fp-armv8"))); + assert!(cfg!(target_feature = "fhm")); + assert!(cfg!(not(target_feature = "fp16fml"))); + assert!(cfg!(target_feature = "fp16")); + assert!(cfg!(not(target_feature = "fullfp16"))); +} + +#[cfg(target_arch = "x86_64")] +fn check_x86_64() { + // This checks that the rustc feature name is used, not the LLVM feature. + assert!(cfg!(target_feature = "rdrand")); + assert!(cfg!(not(target_feature = "rdrnd"))); + + // Likewise: We enable LLVM's crc32 feature with SSE4.2, but Rust says it's just SSE4.2 + assert!(cfg!(target_feature = "sse4.2")); + assert!(cfg!(not(target_feature = "crc32"))); +} |
