Use rust rather than LLVM target features in the target spec

This works better with non-LLVM codegen backends.

Author: bjorn3

compiler/rustc_codegen_gcc/src/gcc_util.rs

@@ -33,11 +33,7 @@ pub(crate) fn global_gcc_features(sess: &Session) -> Vec<String> {
     // should be taken in cases like these.
     let mut features = vec![];
 
-    // Features implied by an implicit or explicit `--target`.
-    features.extend(sess.target.features.split(',').filter(|v| !v.is_empty()).map(String::from));
-
-    // -Ctarget-features
-    target_features::flag_to_backend_features(sess, |feature, enable| {
+    let mut extend_backend_features = |feature: &str, enable: bool| {
         // We run through `to_gcc_features` when
         // passing requests down to GCC. This means that all in-language
         // features also work on the command line instead of having two
@@ -48,7 +44,13 @@ pub(crate) fn global_gcc_features(sess: &Session) -> Vec<String> {
                 .flat_map(|feat| to_gcc_features(sess, feat).into_iter())
                 .map(|feature| if !enable { format!("-{}", feature) } else { feature.to_string() }),
         );
-    });
+    };
+
+    // Features implied by an implicit or explicit `--target`.
+    target_features::target_spec_to_backend_features(sess, &mut extend_backend_features);
+
+    // -Ctarget-features
+    target_features::flag_to_backend_features(sess, extend_backend_features);
 
     gcc_features_by_flags(sess, &mut features);
 
@@ -66,6 +68,7 @@ pub fn to_gcc_features<'a>(sess: &Session, s: &'a str) -> SmallVec<[&'a str; 2]>
         (&Arch::X86 | &Arch::X86_64, "rdrand") => smallvec!["rdrnd"],
         (&Arch::X86 | &Arch::X86_64, "bmi1") => smallvec!["bmi"],
         (&Arch::X86 | &Arch::X86_64, "cmpxchg16b") => smallvec!["cx16"],
+        (&Arch::X86 | &Arch::X86_64, "lahfsahf") => smallvec!["sahf"],
         (&Arch::X86 | &Arch::X86_64, "avx512vaes") => smallvec!["vaes"],
         (&Arch::X86 | &Arch::X86_64, "avx512gfni") => smallvec!["gfni"],
         (&Arch::X86 | &Arch::X86_64, "avx512vpclmulqdq") => smallvec!["vpclmulqdq"],

compiler/rustc_codegen_llvm/src/llvm_util.rs

@@ -626,6 +626,10 @@ pub(crate) fn target_cpu(sess: &Session) -> &str {
 
 /// The target features for compiler flags other than `-Ctarget-features`.
 fn llvm_features_by_flags(sess: &Session, features: &mut Vec<String>) {
+    if wants_wasm_eh(sess) && sess.panic_strategy() == PanicStrategy::Unwind {
+        features.push("+exception-handling".into());
+    }
+
     target_features::retpoline_features_by_flags(sess, features);
 
     // -Zfixed-x18
@@ -691,36 +695,35 @@ pub(crate) fn global_llvm_features(sess: &Session, only_base_features: bool) ->
         Some(_) | None => {}
     };
 
-    // Features implied by an implicit or explicit `--target`.
-    features.extend(sess.target.features.split(',').filter(|v| !v.is_empty()).map(String::from));
+    let mut extend_backend_features = |feature: &str, enable: bool| {
+        let enable_disable = if enable { '+' } else { '-' };
+        // We run through `to_llvm_features` when
+        // passing requests down to LLVM. This means that all in-language
+        // features also work on the command line instead of having two
+        // different names when the LLVM name and the Rust name differ.
+        let Some(llvm_feature) = to_llvm_features(sess, feature) else { return };
+
+        features.extend(
+            std::iter::once(format!("{}{}", enable_disable, llvm_feature.llvm_feature_name)).chain(
+                llvm_feature.dependencies.into_iter().filter_map(move |feat| {
+                    match (enable, feat) {
+                        (_, TargetFeatureFoldStrength::Both(f))
+                        | (true, TargetFeatureFoldStrength::EnableOnly(f)) => {
+                            Some(format!("{enable_disable}{f}"))
+                        }
+                        _ => None,
+                    }
+                }),
+            ),
+        );
+    };
 
-    if wants_wasm_eh(sess) && sess.panic_strategy() == PanicStrategy::Unwind {
-        features.push("+exception-handling".into());
-    }
+    // Features implied by an implicit or explicit `--target`.
+    target_features::target_spec_to_backend_features(sess, &mut extend_backend_features);
 
     // -Ctarget-features
     if !only_base_features {
-        target_features::flag_to_backend_features(sess, |feature, enable| {
-            let enable_disable = if enable { '+' } else { '-' };
-            // We run through `to_llvm_features` when
-            // passing requests down to LLVM. This means that all in-language
-            // features also work on the command line instead of having two
-            // different names when the LLVM name and the Rust name differ.
-            let Some(llvm_feature) = to_llvm_features(sess, feature) else { return };
-
-            features.extend(
-                std::iter::once(format!("{}{}", enable_disable, llvm_feature.llvm_feature_name))
-                    .chain(llvm_feature.dependencies.into_iter().filter_map(move |feat| {
-                        match (enable, feat) {
-                            (_, TargetFeatureFoldStrength::Both(f))
-                            | (true, TargetFeatureFoldStrength::EnableOnly(f)) => {
-                                Some(format!("{enable_disable}{f}"))
-                            }
-                            _ => None,
-                        }
-                    })),
-            )
-        });
+        target_features::flag_to_backend_features(sess, extend_backend_features);
     }
 
     // We add this in the "base target" so that these show up in `sess.unstable_target_features`.

compiler/rustc_codegen_ssa/src/target_features.rs

@@ -139,11 +139,12 @@ pub(crate) fn check_target_feature_trait_unsafe(tcx: TyCtxt<'_>, id: LocalDefId,
     }
 }
 
-/// Parse the value of `-Ctarget-feature`, also expanding implied features,
-/// and call the closure for each (expanded) Rust feature. If the list contains
+/// Parse the value of the target spec `features` field or `-Ctarget-feature`, also expanding
+/// implied features, and call the closure for each (expanded) Rust feature. If the list contains
 /// a syntactically invalid item (not starting with `+`/`-`), the error callback is invoked.
-fn parse_rust_feature_flag<'a>(
+fn parse_rust_feature_list<'a>(
     sess: &'a Session,
+    features: &'a str,
     err_callback: impl Fn(&'a str),
     mut callback: impl FnMut(
         /* base_feature */ &'a str,
@@ -154,7 +155,7 @@ fn parse_rust_feature_flag<'a>(
     // A cache for the backwards implication map.
     let mut inverse_implied_features: Option<FxHashMap<&str, FxHashSet<&str>>> = None;
 
-    for feature in sess.opts.cg.target_feature.split(',') {
+    for feature in features.split(',') {
         if let Some(base_feature) = feature.strip_prefix('+') {
             // Skip features that are not target features, but rustc features.
             if RUSTC_SPECIFIC_FEATURES.contains(&base_feature) {
@@ -244,8 +245,9 @@ pub fn cfg_target_feature<'a, const N: usize>(
     let mut enabled_disabled_features = FxHashMap::default();
 
     // Add enabled and remove disabled features.
-    parse_rust_feature_flag(
+    parse_rust_feature_list(
         sess,
+        &sess.opts.cg.target_feature,
         /* err_callback */
         |feature| {
             sess.dcx().emit_warn(errors::UnknownCTargetFeaturePrefix { feature });
@@ -366,6 +368,37 @@ pub fn check_tied_features(
     None
 }
 
+/// Translates the target spec `features` field into a backend target feature list.
+///
+/// `extend_backend_features` extends the set of backend features (assumed to be in mutable state
+/// accessible by that closure) to enable/disable the given Rust feature name.
+pub fn target_spec_to_backend_features<'a>(
+    sess: &'a Session,
+    mut extend_backend_features: impl FnMut(&'a str, /* enable */ bool),
+) {
+    // Compute implied features
+    let mut rust_features = vec![];
+    parse_rust_feature_list(
+        sess,
+        &sess.target.features,
+        /* err_callback */
+        |feature| {
+            panic!("Target spec contains invalid feature {feature}");
+        },
+        |_base_feature, new_features, enable| {
+            // FIXME emit an error for unknown features like cfg_target_feature would for -Ctarget-feature
+            rust_features.extend(
+                UnordSet::from(new_features).to_sorted_stable_ord().iter().map(|&&s| (enable, s)),
+            );
+        },
+    );
+
+    // Add this to the backend features.
+    for (enable, feature) in rust_features {
+        extend_backend_features(feature, enable);
+    }
+}
+
 /// Translates the `-Ctarget-feature` flag into a backend target feature list.
 ///
 /// `extend_backend_features` extends the set of backend features (assumed to be in mutable state
@@ -376,8 +409,9 @@ pub fn flag_to_backend_features<'a>(
 ) {
     // Compute implied features
     let mut rust_features = vec![];
-    parse_rust_feature_flag(
+    parse_rust_feature_list(
         sess,
+        &sess.opts.cg.target_feature,
         /* err_callback */
         |_feature| {
             // Errors are already emitted in `cfg_target_feature`; avoid duplicates.

compiler/rustc_interface/src/util.rs

@@ -364,15 +364,16 @@ impl CodegenBackend for DummyCodegenBackend {
     }
 
     fn target_config(&self, sess: &Session) -> TargetConfig {
+        let abi_required_features = sess.target.abi_required_features();
         let (target_features, unstable_target_features) = cfg_target_feature::<0>(
             sess,
             |_feature| Default::default(),
             |feature| {
                 // This is a standin for the list of features a backend is expected to enable.
                 // It would be better to parse target.features instead and handle implied features,
-                // but target.features is a list of LLVM target features, not Rust target features.
-                // The dummy backend doesn't know the mapping between LLVM and Rust target features.
-                sess.target.abi_required_features().required.contains(&feature)
+                // but target.features doesn't contain features that are enabled by default for an
+                // architecture or target cpu.
+                abi_required_features.required.contains(&feature)
             },
         );
 

compiler/rustc_target/src/spec/mod.rs

@@ -2246,10 +2246,10 @@ pub struct TargetOptions {
     /// Whether a cpu needs to be explicitly set.
     /// Set to true if there is no default cpu. Defaults to false.
     pub need_explicit_cpu: bool,
-    /// Default target features to pass to LLVM. These features overwrite
-    /// `-Ctarget-cpu` but can be overwritten with `-Ctarget-features`.
-    /// Corresponds to `llc -mattr=$features`.
-    /// Note that these are LLVM feature names, not Rust feature names!
+    /// Default (Rust) target features to enable for this target. These features
+    /// overwrite `-Ctarget-cpu` but can be overwritten with `-Ctarget-features`.
+    /// Corresponds to `llc -mattr=$llvm_features` where `$llvm_features` is the
+    /// result of mapping the Rust features in this field to LLVM features.
     ///
     /// Generally it is a bad idea to use negative target features because they often interact very
     /// poorly with how `-Ctarget-cpu` works. Instead, try to use a lower "base CPU" and enable the

compiler/rustc_target/src/spec/targets/aarch64_apple_ios.rs

@@ -16,7 +16,7 @@ pub(crate) fn target() -> Target {
             .into(),
         arch,
         options: TargetOptions {
-            features: "+neon,+fp-armv8,+apple-a7".into(),
+            features: "+neon,+apple-a7".into(),
             max_atomic_width: Some(128),
             supported_sanitizers: SanitizerSet::ADDRESS
                 | SanitizerSet::THREAD

compiler/rustc_target/src/spec/targets/aarch64_apple_ios_macabi.rs

@@ -16,7 +16,7 @@ pub(crate) fn target() -> Target {
             .into(),
         arch,
         options: TargetOptions {
-            features: "+neon,+fp-armv8,+apple-a12".into(),
+            features: "+neon,+apple-a12".into(),
             max_atomic_width: Some(128),
             supported_sanitizers: SanitizerSet::ADDRESS | SanitizerSet::LEAK | SanitizerSet::THREAD,
             ..opts

compiler/rustc_target/src/spec/targets/aarch64_apple_ios_sim.rs

@@ -16,7 +16,7 @@ pub(crate) fn target() -> Target {
             .into(),
         arch,
         options: TargetOptions {
-            features: "+neon,+fp-armv8,+apple-a7".into(),
+            features: "+neon,+apple-a7".into(),
             max_atomic_width: Some(128),
             supported_sanitizers: SanitizerSet::ADDRESS
                 | SanitizerSet::THREAD

compiler/rustc_target/src/spec/targets/aarch64_apple_tvos.rs

@@ -16,7 +16,7 @@ pub(crate) fn target() -> Target {
             .into(),
         arch,
         options: TargetOptions {
-            features: "+neon,+fp-armv8,+apple-a7".into(),
+            features: "+neon,+apple-a7".into(),
             max_atomic_width: Some(128),
             ..opts
         },

compiler/rustc_target/src/spec/targets/aarch64_apple_tvos_sim.rs

@@ -16,7 +16,7 @@ pub(crate) fn target() -> Target {
             .into(),
         arch,
         options: TargetOptions {
-            features: "+neon,+fp-armv8,+apple-a7".into(),
+            features: "+neon,+apple-a7".into(),
             max_atomic_width: Some(128),
             ..opts
         },