From 4cca436e300db5e6429aa8f041f4dbbcfdecf1b1 Mon Sep 17 00:00:00 2001 From: Jamie Cunliffe Date: Tue, 17 Jan 2023 15:49:20 +0000 Subject: [PATCH 1/4] Tie neon with fp-armv8. In #91608 the fp-armv8 feature was removed as it's tied to the neon feature. However disabling neon didn't actually disable the use of floating point registers and instructions, for this `-fp-armv8` is required. --- compiler/rustc_codegen_llvm/src/llvm_util.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/llvm_util.rs b/compiler/rustc_codegen_llvm/src/llvm_util.rs index 994addf12eb24..abf46b914b787 100644 --- a/compiler/rustc_codegen_llvm/src/llvm_util.rs +++ b/compiler/rustc_codegen_llvm/src/llvm_util.rs @@ -163,8 +163,10 @@ pub fn to_llvm_features<'a>(sess: &Session, s: &'a str) -> SmallVec<[&'a str; 2] ("aarch64", "pmuv3") => smallvec!["perfmon"], ("aarch64", "paca") => smallvec!["pauth"], ("aarch64", "pacg") => smallvec!["pauth"], - // Rust ties fp and neon together. In LLVM neon implicitly enables fp, - // but we manually enable neon when a feature only implicitly enables fp + // Rust ties fp and neon together. + ("aarch64", "neon") => smallvec!["neon", "fp-armv8"], + // In LLVM neon implicitly enables fp, but we manually enable + // neon when a feature only implicitly enables fp ("aarch64", "f32mm") => smallvec!["f32mm", "neon"], ("aarch64", "f64mm") => smallvec!["f64mm", "neon"], ("aarch64", "fhm") => smallvec!["fp16fml", "neon"], From aab0757c666504f36c5a382304ec5390fc4f7454 Mon Sep 17 00:00:00 2001 From: Jamie Cunliffe Date: Mon, 23 Jan 2023 13:15:53 +0000 Subject: [PATCH 2/4] Only disable folded features when it makes sense. Some features that are tied together only make sense to be folded together when enabling the feature. For example on AArch64 sve and neon are tied together, however it doesn't make sense to disable neon when disabling sve. --- compiler/rustc_codegen_llvm/src/llvm_util.rs | 36 +++++++++++++++++--- tests/codegen/tied-features-strength.rs | 25 ++++++++++++++ 2 files changed, 56 insertions(+), 5 deletions(-) create mode 100644 tests/codegen/tied-features-strength.rs diff --git a/compiler/rustc_codegen_llvm/src/llvm_util.rs b/compiler/rustc_codegen_llvm/src/llvm_util.rs index abf46b914b787..a75c15ee4be90 100644 --- a/compiler/rustc_codegen_llvm/src/llvm_util.rs +++ b/compiler/rustc_codegen_llvm/src/llvm_util.rs @@ -147,6 +147,9 @@ pub fn time_trace_profiler_finish(file_name: &Path) { // Though note that Rust can also be build with an external precompiled version of LLVM // which might lead to failures if the oldest tested / supported LLVM version // doesn't yet support the relevant intrinsics +// +// Note: The first feature in the list that is returned is the mapping to the feature that is +// provided from the `s` parameter. pub fn to_llvm_features<'a>(sess: &Session, s: &'a str) -> SmallVec<[&'a str; 2]> { let arch = if sess.target.arch == "x86_64" { "x86" } else { &*sess.target.arch }; match (arch, s) { @@ -182,6 +185,23 @@ pub fn to_llvm_features<'a>(sess: &Session, s: &'a str) -> SmallVec<[&'a str; 2] } } +pub enum TargetFeatureFoldStrength { + // The feature is only tied when enabling the feature, disabling + // this feature shouldn't disable the tied feature. + EnableOnly, + // The feature is tied for both enabling and disabling this feature. + Both, +} + +// Determines how the features are folded together, some features are +// linked a lot more than some others. +pub fn feature_fold_strength<'a>(feats: &SmallVec<[&'a str; 2]>) -> TargetFeatureFoldStrength { + match (feats.get(0), feats.get(1)) { + (Some(&"neon"), Some(&"fp-armv8")) => TargetFeatureFoldStrength::Both, + _ => TargetFeatureFoldStrength::EnableOnly, + } +} + /// Given a map from target_features to whether they are enabled or disabled, /// ensure only valid combinations are allowed. pub fn check_tied_features( @@ -471,11 +491,17 @@ pub(crate) fn global_llvm_features(sess: &Session, diagnostics: bool) -> Vec { + Some(format!("{}{}", enable_disable, f)) + } + _ if idx == 0 => Some(format!("{}{}", enable_disable, f)), + _ => None, + }, + )) }) .flatten(); features.extend(feats); diff --git a/tests/codegen/tied-features-strength.rs b/tests/codegen/tied-features-strength.rs new file mode 100644 index 0000000000000..36fd717e914e0 --- /dev/null +++ b/tests/codegen/tied-features-strength.rs @@ -0,0 +1,25 @@ +// ignore-tidy-linelength +// revisions: ENABLE_SVE DISABLE_SVE DISABLE_NEON ENABLE_NEON +// compile-flags: --crate-type=rlib --target=aarch64-unknown-linux-gnu +// needs-llvm-components: aarch64 + +// [ENABLE_SVE] compile-flags: -C target-feature=+sve +// ENABLE_SVE: attributes #0 = { {{.*}} "target-features"="+outline-atomics,+sve,+neon,+v8a" } + +// [DISABLE_SVE] compile-flags: -C target-feature=-sve +// DISABLE_SVE: attributes #0 = { {{.*}} "target-features"="+outline-atomics,-sve,+v8a" } + +// [DISABLE_NEON] compile-flags: -C target-feature=-neon +// DISABLE_NEON: attributes #0 = { {{.*}} "target-features"="+outline-atomics,-neon,-fp-armv8,+v8a" } + +// [ENABLE_NEON] compile-flags: -C target-feature=+neon +// ENABLE_NEON: attributes #0 = { {{.*}} "target-features"="+outline-atomics,+neon,+fp-armv8,+v8a" } + + +#![feature(no_core, lang_items)] +#![no_core] + +#[lang = "sized"] +trait Sized {} + +pub fn test() {} From d51db4275b5271b0349b3d6f21e5eea2d0ee62fc Mon Sep 17 00:00:00 2001 From: Jamie Cunliffe Date: Wed, 25 Jan 2023 16:05:24 +0000 Subject: [PATCH 3/4] Make v8a match optional in the test feature list. --- tests/codegen/tied-features-strength.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/codegen/tied-features-strength.rs b/tests/codegen/tied-features-strength.rs index 36fd717e914e0..5dc9ae47e7934 100644 --- a/tests/codegen/tied-features-strength.rs +++ b/tests/codegen/tied-features-strength.rs @@ -3,17 +3,21 @@ // compile-flags: --crate-type=rlib --target=aarch64-unknown-linux-gnu // needs-llvm-components: aarch64 +// The "+v8a" feature is matched as optional as it isn't added when we +// are targeting older LLVM versions. Once the min supported version +// is LLVM-14 we can remove the regex matching for this feature. + // [ENABLE_SVE] compile-flags: -C target-feature=+sve -// ENABLE_SVE: attributes #0 = { {{.*}} "target-features"="+outline-atomics,+sve,+neon,+v8a" } +// ENABLE_SVE: attributes #0 = { {{.*}} "target-features"="+outline-atomics,+sve,+neon{{(,\+v8a)?}}" } // [DISABLE_SVE] compile-flags: -C target-feature=-sve -// DISABLE_SVE: attributes #0 = { {{.*}} "target-features"="+outline-atomics,-sve,+v8a" } +// DISABLE_SVE: attributes #0 = { {{.*}} "target-features"="+outline-atomics,-sve{{(,\+v8a)?}}" } // [DISABLE_NEON] compile-flags: -C target-feature=-neon -// DISABLE_NEON: attributes #0 = { {{.*}} "target-features"="+outline-atomics,-neon,-fp-armv8,+v8a" } +// DISABLE_NEON: attributes #0 = { {{.*}} "target-features"="+outline-atomics,-neon,-fp-armv8{{(,\+v8a)?}}" } // [ENABLE_NEON] compile-flags: -C target-feature=+neon -// ENABLE_NEON: attributes #0 = { {{.*}} "target-features"="+outline-atomics,+neon,+fp-armv8,+v8a" } +// ENABLE_NEON: attributes #0 = { {{.*}} "target-features"="+outline-atomics,+neon,+fp-armv8{{(,\+v8a)?}}" } #![feature(no_core, lang_items)] From a059e68d118512063e8e3517344178667df951b0 Mon Sep 17 00:00:00 2001 From: Jamie Cunliffe Date: Mon, 22 May 2023 14:46:40 +0100 Subject: [PATCH 4/4] Create a structure to define the features from to_llvm_features. Rather than returning an array of features from to_llvm_features, return a structure that contains the dependencies. This also contains metadata on how the features depend on each other to allow for the correct enabling and disabling. --- compiler/rustc_codegen_llvm/src/lib.rs | 1 + compiler/rustc_codegen_llvm/src/llvm_util.rs | 192 ++++++++++++------- tests/codegen/tied-features-strength.rs | 10 +- 3 files changed, 133 insertions(+), 70 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/lib.rs b/compiler/rustc_codegen_llvm/src/lib.rs index 6a86237d79e07..805843e5863e6 100644 --- a/compiler/rustc_codegen_llvm/src/lib.rs +++ b/compiler/rustc_codegen_llvm/src/lib.rs @@ -10,6 +10,7 @@ #![feature(iter_intersperse)] #![feature(let_chains)] #![feature(never_type)] +#![feature(impl_trait_in_assoc_type)] #![recursion_limit = "256"] #![allow(rustc::potential_query_instability)] #![deny(rustc::untranslatable_diagnostic)] diff --git a/compiler/rustc_codegen_llvm/src/llvm_util.rs b/compiler/rustc_codegen_llvm/src/llvm_util.rs index a75c15ee4be90..03be0654b50bb 100644 --- a/compiler/rustc_codegen_llvm/src/llvm_util.rs +++ b/compiler/rustc_codegen_llvm/src/llvm_util.rs @@ -16,7 +16,6 @@ use rustc_session::config::PrintRequest; use rustc_session::Session; use rustc_span::symbol::Symbol; use rustc_target::spec::{MergeFunctions, PanicStrategy}; -use smallvec::{smallvec, SmallVec}; use std::ffi::{CStr, CString}; use std::path::Path; @@ -132,6 +131,60 @@ pub fn time_trace_profiler_finish(file_name: &Path) { } } +pub enum TargetFeatureFoldStrength<'a> { + // The feature is only tied when enabling the feature, disabling + // this feature shouldn't disable the tied feature. + EnableOnly(&'a str), + // The feature is tied for both enabling and disabling this feature. + Both(&'a str), +} + +impl<'a> TargetFeatureFoldStrength<'a> { + fn as_str(&self) -> &'a str { + match self { + TargetFeatureFoldStrength::EnableOnly(feat) => feat, + TargetFeatureFoldStrength::Both(feat) => feat, + } + } +} + +pub struct LLVMFeature<'a> { + pub llvm_feature_name: &'a str, + pub dependency: Option>, +} + +impl<'a> LLVMFeature<'a> { + pub fn new(llvm_feature_name: &'a str) -> Self { + Self { llvm_feature_name, dependency: None } + } + + pub fn with_dependency( + llvm_feature_name: &'a str, + dependency: TargetFeatureFoldStrength<'a>, + ) -> Self { + Self { llvm_feature_name, dependency: Some(dependency) } + } + + pub fn contains(&self, feat: &str) -> bool { + self.iter().any(|dep| dep == feat) + } + + pub fn iter(&'a self) -> impl Iterator { + let dependencies = self.dependency.iter().map(|feat| feat.as_str()); + std::iter::once(self.llvm_feature_name).chain(dependencies) + } +} + +impl<'a> IntoIterator for LLVMFeature<'a> { + type Item = &'a str; + type IntoIter = impl Iterator; + + fn into_iter(self) -> Self::IntoIter { + let dependencies = self.dependency.into_iter().map(|feat| feat.as_str()); + std::iter::once(self.llvm_feature_name).chain(dependencies) + } +} + // WARNING: the features after applying `to_llvm_features` must be known // to LLVM or the feature detection code will walk past the end of the feature // array, leading to crashes. @@ -147,58 +200,65 @@ pub fn time_trace_profiler_finish(file_name: &Path) { // Though note that Rust can also be build with an external precompiled version of LLVM // which might lead to failures if the oldest tested / supported LLVM version // doesn't yet support the relevant intrinsics -// -// Note: The first feature in the list that is returned is the mapping to the feature that is -// provided from the `s` parameter. -pub fn to_llvm_features<'a>(sess: &Session, s: &'a str) -> SmallVec<[&'a str; 2]> { +pub fn to_llvm_features<'a>(sess: &Session, s: &'a str) -> LLVMFeature<'a> { let arch = if sess.target.arch == "x86_64" { "x86" } else { &*sess.target.arch }; match (arch, s) { - ("x86", "sse4.2") => smallvec!["sse4.2", "crc32"], - ("x86", "pclmulqdq") => smallvec!["pclmul"], - ("x86", "rdrand") => smallvec!["rdrnd"], - ("x86", "bmi1") => smallvec!["bmi"], - ("x86", "cmpxchg16b") => smallvec!["cx16"], - ("aarch64", "rcpc2") => smallvec!["rcpc-immo"], - ("aarch64", "dpb") => smallvec!["ccpp"], - ("aarch64", "dpb2") => smallvec!["ccdp"], - ("aarch64", "frintts") => smallvec!["fptoint"], - ("aarch64", "fcma") => smallvec!["complxnum"], - ("aarch64", "pmuv3") => smallvec!["perfmon"], - ("aarch64", "paca") => smallvec!["pauth"], - ("aarch64", "pacg") => smallvec!["pauth"], + ("x86", "sse4.2") => { + LLVMFeature::with_dependency("sse4.2", TargetFeatureFoldStrength::EnableOnly("crc32")) + } + ("x86", "pclmulqdq") => LLVMFeature::new("pclmul"), + ("x86", "rdrand") => LLVMFeature::new("rdrnd"), + ("x86", "bmi1") => LLVMFeature::new("bmi"), + ("x86", "cmpxchg16b") => LLVMFeature::new("cx16"), + ("aarch64", "rcpc2") => LLVMFeature::new("rcpc-immo"), + ("aarch64", "dpb") => LLVMFeature::new("ccpp"), + ("aarch64", "dpb2") => LLVMFeature::new("ccdp"), + ("aarch64", "frintts") => LLVMFeature::new("fptoint"), + ("aarch64", "fcma") => LLVMFeature::new("complxnum"), + ("aarch64", "pmuv3") => LLVMFeature::new("perfmon"), + ("aarch64", "paca") => LLVMFeature::new("pauth"), + ("aarch64", "pacg") => LLVMFeature::new("pauth"), // Rust ties fp and neon together. - ("aarch64", "neon") => smallvec!["neon", "fp-armv8"], + ("aarch64", "neon") => { + LLVMFeature::with_dependency("neon", TargetFeatureFoldStrength::Both("fp-armv8")) + } // In LLVM neon implicitly enables fp, but we manually enable // neon when a feature only implicitly enables fp - ("aarch64", "f32mm") => smallvec!["f32mm", "neon"], - ("aarch64", "f64mm") => smallvec!["f64mm", "neon"], - ("aarch64", "fhm") => smallvec!["fp16fml", "neon"], - ("aarch64", "fp16") => smallvec!["fullfp16", "neon"], - ("aarch64", "jsconv") => smallvec!["jsconv", "neon"], - ("aarch64", "sve") => smallvec!["sve", "neon"], - ("aarch64", "sve2") => smallvec!["sve2", "neon"], - ("aarch64", "sve2-aes") => smallvec!["sve2-aes", "neon"], - ("aarch64", "sve2-sm4") => smallvec!["sve2-sm4", "neon"], - ("aarch64", "sve2-sha3") => smallvec!["sve2-sha3", "neon"], - ("aarch64", "sve2-bitperm") => smallvec!["sve2-bitperm", "neon"], - (_, s) => smallvec![s], - } -} - -pub enum TargetFeatureFoldStrength { - // The feature is only tied when enabling the feature, disabling - // this feature shouldn't disable the tied feature. - EnableOnly, - // The feature is tied for both enabling and disabling this feature. - Both, -} - -// Determines how the features are folded together, some features are -// linked a lot more than some others. -pub fn feature_fold_strength<'a>(feats: &SmallVec<[&'a str; 2]>) -> TargetFeatureFoldStrength { - match (feats.get(0), feats.get(1)) { - (Some(&"neon"), Some(&"fp-armv8")) => TargetFeatureFoldStrength::Both, - _ => TargetFeatureFoldStrength::EnableOnly, + ("aarch64", "f32mm") => { + LLVMFeature::with_dependency("f32mm", TargetFeatureFoldStrength::EnableOnly("neon")) + } + ("aarch64", "f64mm") => { + LLVMFeature::with_dependency("f64mm", TargetFeatureFoldStrength::EnableOnly("neon")) + } + ("aarch64", "fhm") => { + LLVMFeature::with_dependency("fp16fml", TargetFeatureFoldStrength::EnableOnly("neon")) + } + ("aarch64", "fp16") => { + LLVMFeature::with_dependency("fullfp16", TargetFeatureFoldStrength::EnableOnly("neon")) + } + ("aarch64", "jsconv") => { + LLVMFeature::with_dependency("jsconv", TargetFeatureFoldStrength::EnableOnly("neon")) + } + ("aarch64", "sve") => { + LLVMFeature::with_dependency("sve", TargetFeatureFoldStrength::EnableOnly("neon")) + } + ("aarch64", "sve2") => { + LLVMFeature::with_dependency("sve2", TargetFeatureFoldStrength::EnableOnly("neon")) + } + ("aarch64", "sve2-aes") => { + LLVMFeature::with_dependency("sve2-aes", TargetFeatureFoldStrength::EnableOnly("neon")) + } + ("aarch64", "sve2-sm4") => { + LLVMFeature::with_dependency("sve2-sm4", TargetFeatureFoldStrength::EnableOnly("neon")) + } + ("aarch64", "sve2-sha3") => { + LLVMFeature::with_dependency("sve2-sha3", TargetFeatureFoldStrength::EnableOnly("neon")) + } + ("aarch64", "sve2-bitperm") => LLVMFeature::with_dependency( + "sve2-bitperm", + TargetFeatureFoldStrength::EnableOnly("neon"), + ), + (_, s) => LLVMFeature::new(s), } } @@ -296,18 +356,17 @@ fn print_target_features(sess: &Session, tm: &llvm::TargetMachine) { let mut rustc_target_features = supported_target_features(sess) .iter() .map(|(feature, _gate)| { - let desc = if let Some(llvm_feature) = to_llvm_features(sess, *feature).first() { - // LLVM asserts that these are sorted. LLVM and Rust both use byte comparison for these strings. + // LLVM asserts that these are sorted. LLVM and Rust both use byte comparison for these strings. + let llvm_feature = to_llvm_features(sess, *feature).llvm_feature_name; + let desc = match llvm_target_features.binary_search_by_key(&llvm_feature, |(f, _d)| f).ok() { Some(index) => { known_llvm_target_features.insert(llvm_feature); llvm_target_features[index].1 } None => "", - } - } else { - "" - }; + }; + (*feature, desc) }) .collect::>(); @@ -491,17 +550,20 @@ pub(crate) fn global_llvm_features(sess: &Session, diagnostics: bool) -> Vec { - Some(format!("{}{}", enable_disable, f)) - } - _ if idx == 0 => Some(format!("{}{}", enable_disable, f)), - _ => None, - }, - )) + let llvm_feature = to_llvm_features(sess, feature); + + Some( + std::iter::once(format!("{}{}", enable_disable, llvm_feature.llvm_feature_name)) + .chain(llvm_feature.dependency.into_iter().filter_map(move |feat| { + match (enable_disable, feat) { + ('-' | '+', TargetFeatureFoldStrength::Both(f)) + | ('+', TargetFeatureFoldStrength::EnableOnly(f)) => { + Some(format!("{}{}", enable_disable, f)) + } + _ => None, + } + })), + ) }) .flatten(); features.extend(feats); diff --git a/tests/codegen/tied-features-strength.rs b/tests/codegen/tied-features-strength.rs index 5dc9ae47e7934..51334c1215827 100644 --- a/tests/codegen/tied-features-strength.rs +++ b/tests/codegen/tied-features-strength.rs @@ -5,19 +5,19 @@ // The "+v8a" feature is matched as optional as it isn't added when we // are targeting older LLVM versions. Once the min supported version -// is LLVM-14 we can remove the regex matching for this feature. +// is LLVM-14 we can remove the optional regex matching for this feature. // [ENABLE_SVE] compile-flags: -C target-feature=+sve -// ENABLE_SVE: attributes #0 = { {{.*}} "target-features"="+outline-atomics,+sve,+neon{{(,\+v8a)?}}" } +// ENABLE_SVE: attributes #0 = { {{.*}} "target-features"="{{((\+outline-atomics,?)|(\+v8a,?)?|(\+sve,?)|(\+neon,?))*}}" } // [DISABLE_SVE] compile-flags: -C target-feature=-sve -// DISABLE_SVE: attributes #0 = { {{.*}} "target-features"="+outline-atomics,-sve{{(,\+v8a)?}}" } +// DISABLE_SVE: attributes #0 = { {{.*}} "target-features"="{{((\+outline-atomics,?)|(\+v8a,?)?|(-sve,?)|(\+neon,?))*}}" } // [DISABLE_NEON] compile-flags: -C target-feature=-neon -// DISABLE_NEON: attributes #0 = { {{.*}} "target-features"="+outline-atomics,-neon,-fp-armv8{{(,\+v8a)?}}" } +// DISABLE_NEON: attributes #0 = { {{.*}} "target-features"="{{((\+outline-atomics,?)|(\+v8a,?)?|(-fp-armv8,?)|(-neon,?))*}}" } // [ENABLE_NEON] compile-flags: -C target-feature=+neon -// ENABLE_NEON: attributes #0 = { {{.*}} "target-features"="+outline-atomics,+neon,+fp-armv8{{(,\+v8a)?}}" } +// ENABLE_NEON: attributes #0 = { {{.*}} "target-features"="{{((\+outline-atomics,?)|(\+v8a,?)?|(\+fp-armv8,?)|(\+neon,?))*}}" } #![feature(no_core, lang_items)]