Auto merge of #134794 - RalfJung:abi-required-target-features, r=workingjubilee

Add a notion of "some ABIs require certain target features"

I think I finally found the right shape for the data and checks that I recently added in https://github.com/rust-lang/rust/pull/133099, https://github.com/rust-lang/rust/pull/133417, https://github.com/rust-lang/rust/pull/134337: we have a notion of "this ABI requires the following list of target features, and it is incompatible with the following list of target features". Both `-Ctarget-feature` and `#[target_feature]` are updated to ensure we follow the rules of the ABI.  This removes all the "toggleability" stuff introduced before, though we do keep the notion of a fully "forbidden" target feature -- this is needed to deal with target features that are actual ABI switches, and hence are needed to even compute the list of required target features.

We always explicitly (un)set all required and in-conflict features, just to avoid potential trouble caused by the default features of whatever the base CPU is. We do this *before* applying `-Ctarget-feature` to maintain backward compatibility; this poses a slight risk of missing some implicit feature dependencies in LLVM but has the advantage of not breaking users that deliberately toggle ABI-relevant target features. They get a warning but the feature does get toggled the way they requested.

For now, our logic supports x86, ARM, and RISC-V (just like the previous logic did). Unsurprisingly, RISC-V is the nicest. ;)

As a side-effect this also (unstably) allows *enabling* `x87` when that is harmless. I used the opportunity to mark SSE2 as required on x86-64, to better match the actual logic in LLVM and because all x86-64 chips do have SSE2. This infrastructure also prepares us for requiring SSE on x86-32 when we want to use that for our ABI (and for float semantics sanity), see https://github.com/rust-lang/rust/issues/133611, but no such change is happening in this PR.

r? `@workingjubilee`

compiler/rustc_codegen_gcc/src/errors.rs

@@ -28,6 +28,7 @@ pub(crate) struct UnstableCTargetFeature<'a> {
 #[diag(codegen_gcc_forbidden_ctarget_feature)]
 pub(crate) struct ForbiddenCTargetFeature<'a> {
     pub feature: &'a str,
+    pub enabled: &'a str,
     pub reason: &'a str,
 }
 

compiler/rustc_codegen_gcc/src/gcc_util.rs

@@ -1,9 +1,11 @@
+use std::iter::FromIterator;
+
 #[cfg(feature = "master")]
 use gccjit::Context;
 use rustc_codegen_ssa::codegen_attrs::check_tied_features;
 use rustc_codegen_ssa::errors::TargetFeatureDisableOrEnable;
-use rustc_data_structures::fx::FxHashMap;
+use rustc_data_structures::fx::{FxHashMap, FxHashSet};
-use rustc_middle::bug;
+use rustc_data_structures::unord::UnordSet;
 use rustc_session::Session;
 use rustc_target::target_features::RUSTC_SPECIFIC_FEATURES;
 use smallvec::{SmallVec, smallvec};
@@ -37,47 +39,54 @@ pub(crate) fn global_gcc_features(sess: &Session, diagnostics: bool) -> Vec<Stri
     let mut features = vec![];
 
     // Features implied by an implicit or explicit `--target`.
-    features.extend(
+    features.extend(sess.target.features.split(',').filter(|v| !v.is_empty()).map(String::from));
-        sess.target
-            .features
-            .split(',')
-            .filter(|v| !v.is_empty() && backend_feature_name(v).is_some())
-            .map(String::from),
-    );
 
     // -Ctarget-features
     let known_features = sess.target.rust_target_features();
     let mut featsmap = FxHashMap::default();
-    let feats = sess
-        .opts
-        .cg
-        .target_feature
-        .split(',')
-        .filter_map(|s| {
-            let enable_disable = match s.chars().next() {
-                None => return None,
-                Some(c @ ('+' | '-')) => c,
-                Some(_) => {
-                    if diagnostics {
-                        sess.dcx().emit_warn(UnknownCTargetFeaturePrefix { feature: s });
-                    }
-                    return None;
-                }
-            };
 
-            // Get the backend feature name, if any.
+    // Ensure that all ABI-required features are enabled, and the ABI-forbidden ones
-            // This excludes rustc-specific features, that do not get passed down to GCC.
+    // are disabled.
-            let feature = backend_feature_name(s)?;
+    let abi_feature_constraints = sess.target.abi_required_features();
-            // Warn against use of GCC specific feature names on the CLI.
+    let abi_incompatible_set =
+        FxHashSet::from_iter(abi_feature_constraints.incompatible.iter().copied());
+
+    // Compute implied features
+    let mut all_rust_features = vec![];
+    for feature in sess.opts.cg.target_feature.split(',') {
+        if let Some(feature) = feature.strip_prefix('+') {
+            all_rust_features.extend(
+                UnordSet::from(sess.target.implied_target_features(std::iter::once(feature)))
+                    .to_sorted_stable_ord()
+                    .iter()
+                    .map(|&&s| (true, s)),
+            )
+        } else if let Some(feature) = feature.strip_prefix('-') {
+            // FIXME: Why do we not remove implied features on "-" here?
+            // We do the equivalent above in `target_features_cfg`.
+            // See <https://github.com/rust-lang/rust/issues/134792>.
+            all_rust_features.push((false, feature));
+        } else if !feature.is_empty() {
             if diagnostics {
+                sess.dcx().emit_warn(UnknownCTargetFeaturePrefix { feature });
+            }
+        }
+    }
+    // Remove features that are meant for rustc, not codegen.
+    all_rust_features.retain(|(_, feature)| {
+        // Retain if it is not a rustc feature
+        !RUSTC_SPECIFIC_FEATURES.contains(feature)
+    });
+
+    // Check feature validity.
+    if diagnostics {
+        for &(enable, feature) in &all_rust_features {
             let feature_state = known_features.iter().find(|&&(v, _, _)| v == feature);
             match feature_state {
                 None => {
-                        let rust_feature =
+                    let rust_feature = known_features.iter().find_map(|&(rust_feature, _, _)| {
-                            known_features.iter().find_map(|&(rust_feature, _, _)| {
                         let gcc_features = to_gcc_features(sess, rust_feature);
-                                if gcc_features.contains(&feature)
+                        if gcc_features.contains(&feature) && !gcc_features.contains(&rust_feature)
-                                    && !gcc_features.contains(&rust_feature)
                         {
                             Some(rust_feature)
                         } else {
@@ -95,10 +104,12 @@ pub(crate) fn global_gcc_features(sess: &Session, diagnostics: bool) -> Vec<Stri
                     sess.dcx().emit_warn(unknown_feature);
                 }
                 Some((_, stability, _)) => {
-                        if let Err(reason) =
+                    if let Err(reason) = stability.toggle_allowed() {
-                            stability.toggle_allowed(&sess.target, enable_disable == '+')
+                        sess.dcx().emit_warn(ForbiddenCTargetFeature {
-                        {
+                            feature,
-                            sess.dcx().emit_warn(ForbiddenCTargetFeature { feature, reason });
+                            enabled: if enable { "enabled" } else { "disabled" },
+                            reason,
+                        });
                     } else if stability.requires_nightly().is_some() {
                         // An unstable feature. Warn about using it. (It makes little sense
                         // to hard-error here since we just warn about fully unknown
@@ -108,11 +119,57 @@ pub(crate) fn global_gcc_features(sess: &Session, diagnostics: bool) -> Vec<Stri
                 }
             }
 
-                // FIXME(nagisa): figure out how to not allocate a full hashset here.
+            // Ensure that the features we enable/disable are compatible with the ABI.
-                featsmap.insert(feature, enable_disable == '+');
+            if enable {
+                if abi_incompatible_set.contains(feature) {
+                    sess.dcx().emit_warn(ForbiddenCTargetFeature {
+                        feature,
+                        enabled: "enabled",
+                        reason: "this feature is incompatible with the target ABI",
+                    });
+                }
+            } else {
+                // FIXME: we have to request implied features here since
+                // negative features do not handle implied features above.
+                for &required in abi_feature_constraints.required.iter() {
+                    let implied = sess.target.implied_target_features(std::iter::once(required));
+                    if implied.contains(feature) {
+                        sess.dcx().emit_warn(ForbiddenCTargetFeature {
+                            feature,
+                            enabled: "disabled",
+                            reason: "this feature is required by the target ABI",
+                        });
+                    }
+                }
             }
 
-            // ... otherwise though we run through `to_gcc_features` when
+            // FIXME(nagisa): figure out how to not allocate a full hashset here.
+            featsmap.insert(feature, enable);
+        }
+    }
+
+    // To be sure the ABI-relevant features are all in the right state, we explicitly
+    // (un)set them here. This means if the target spec sets those features wrong,
+    // we will silently correct them rather than silently producing wrong code.
+    // (The target sanity check tries to catch this, but we can't know which features are
+    // enabled in GCC by default so we can't be fully sure about that check.)
+    // We add these at the beginning of the list so that `-Ctarget-features` can
+    // still override it... that's unsound, but more compatible with past behavior.
+    all_rust_features.splice(
+        0..0,
+        abi_feature_constraints
+            .required
+            .iter()
+            .map(|&f| (true, f))
+            .chain(abi_feature_constraints.incompatible.iter().map(|&f| (false, f))),
+    );
+
+    // Translate this into GCC features.
+    let feats = all_rust_features
+        .iter()
+        .filter_map(|&(enable, feature)| {
+            let enable_disable = if enable { '+' } else { '-' };
+            // 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
             // different names when the GCC name and the Rust name differ.
@@ -146,26 +203,12 @@ pub(crate) fn global_gcc_features(sess: &Session, diagnostics: bool) -> Vec<Stri
     features
 }
 
-/// Returns a feature name for the given `+feature` or `-feature` string.
-///
-/// Only allows features that are backend specific (i.e. not [`RUSTC_SPECIFIC_FEATURES`].)
-fn backend_feature_name(s: &str) -> Option<&str> {
-    // features must start with a `+` or `-`.
-    let feature = s.strip_prefix(&['+', '-'][..]).unwrap_or_else(|| {
-        bug!("target feature `{}` must begin with a `+` or `-`", s);
-    });
-    // Rustc-specific feature requests like `+crt-static` or `-crt-static`
-    // are not passed down to GCC.
-    if RUSTC_SPECIFIC_FEATURES.contains(&feature) {
-        return None;
-    }
-    Some(feature)
-}
-
 // To find a list of GCC's names, check https://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html
 pub fn to_gcc_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) {
+        // FIXME: seems like x87 does not exist?
+        ("x86", "x87") => smallvec![],
         ("x86", "sse4.2") => smallvec!["sse4.2", "crc32"],
         ("x86", "pclmulqdq") => smallvec!["pclmul"],
         ("x86", "rdrand") => smallvec!["rdrnd"],

compiler/rustc_codegen_llvm/messages.ftl

@@ -10,7 +10,7 @@ codegen_llvm_dynamic_linking_with_lto =
 codegen_llvm_fixed_x18_invalid_arch = the `-Zfixed-x18` flag is not supported on the `{$arch}` architecture
 
 codegen_llvm_forbidden_ctarget_feature =
-    target feature `{$feature}` cannot be toggled with `-Ctarget-feature`: {$reason}
+    target feature `{$feature}` cannot be {$enabled} with `-Ctarget-feature`: {$reason}
     .note = this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
 codegen_llvm_forbidden_ctarget_feature_issue = for more information, see issue #116344 <https://github.com/rust-lang/rust/issues/116344>
 
@@ -24,8 +24,6 @@ codegen_llvm_invalid_minimum_alignment_not_power_of_two =
 codegen_llvm_invalid_minimum_alignment_too_large =
     invalid minimum global alignment: {$align} is too large
 
-codegen_llvm_invalid_target_feature_prefix = target feature `{$feature}` must begin with a `+` or `-`"
-
 codegen_llvm_load_bitcode = failed to load bitcode of module "{$name}"
 codegen_llvm_load_bitcode_with_llvm_err = failed to load bitcode of module "{$name}": {$llvm_err}
 

compiler/rustc_codegen_llvm/src/errors.rs

@@ -37,6 +37,7 @@ pub(crate) struct UnstableCTargetFeature<'a> {
 #[note(codegen_llvm_forbidden_ctarget_feature_issue)]
 pub(crate) struct ForbiddenCTargetFeature<'a> {
     pub feature: &'a str,
+    pub enabled: &'a str,
     pub reason: &'a str,
 }
 
@@ -213,12 +214,6 @@ pub(crate) struct MismatchedDataLayout<'a> {
     pub llvm_layout: &'a str,
 }
 
-#[derive(Diagnostic)]
-#[diag(codegen_llvm_invalid_target_feature_prefix)]
-pub(crate) struct InvalidTargetFeaturePrefix<'a> {
-    pub feature: &'a str,
-}
-
 #[derive(Diagnostic)]
 #[diag(codegen_llvm_fixed_x18_invalid_arch)]
 pub(crate) struct FixedX18InvalidArch<'a> {

compiler/rustc_codegen_llvm/src/llvm_util.rs

@@ -21,8 +21,8 @@ use rustc_target::target_features::{RUSTC_SPECIAL_FEATURES, RUSTC_SPECIFIC_FEATU
 
 use crate::back::write::create_informational_target_machine;
 use crate::errors::{
-    FixedX18InvalidArch, ForbiddenCTargetFeature, InvalidTargetFeaturePrefix, PossibleFeature,
+    FixedX18InvalidArch, ForbiddenCTargetFeature, PossibleFeature, UnknownCTargetFeature,
-    UnknownCTargetFeature, UnknownCTargetFeaturePrefix, UnstableCTargetFeature,
+    UnknownCTargetFeaturePrefix, UnstableCTargetFeature,
 };
 use crate::llvm;
 
@@ -348,7 +348,16 @@ pub fn target_features_cfg(sess: &Session, allow_unstable: bool) -> Vec<Symbol>
     {
         if enabled {
             // Also add all transitively implied features.
-            features.extend(sess.target.implied_target_features(std::iter::once(feature)));
+
+            // We don't care about the order in `features` since the only thing we use it for is the
+            // `features.contains` below.
+            #[allow(rustc::potential_query_instability)]
+            features.extend(
+                sess.target
+                    .implied_target_features(std::iter::once(feature.as_str()))
+                    .iter()
+                    .map(|s| Symbol::intern(s)),
+            );
         } else {
             // Remove transitively reverse-implied features.
 
@@ -356,7 +365,11 @@ pub fn target_features_cfg(sess: &Session, allow_unstable: bool) -> Vec<Symbol>
             // `features.contains` below.
             #[allow(rustc::potential_query_instability)]
             features.retain(|f| {
-                if sess.target.implied_target_features(std::iter::once(*f)).contains(&feature) {
+                if sess
+                    .target
+                    .implied_target_features(std::iter::once(f.as_str()))
+                    .contains(&feature.as_str())
+                {
                     // If `f` if implies `feature`, then `!feature` implies `!f`, so we have to
                     // remove `f`. (This is the standard logical contraposition principle.)
                     false
@@ -638,7 +651,7 @@ pub(crate) fn global_llvm_features(
         sess.target
             .features
             .split(',')
-            .filter(|v| !v.is_empty() && backend_feature_name(sess, v).is_some())
+            .filter(|v| !v.is_empty())
             // Drop +v8plus feature introduced in LLVM 20.
             .filter(|v| *v != "+v8plus" || get_version() >= (20, 0, 0))
             .map(String::from),
@@ -651,44 +664,45 @@ pub(crate) fn global_llvm_features(
     // -Ctarget-features
     if !only_base_features {
         let known_features = sess.target.rust_target_features();
+        // Will only be filled when `diagnostics` is set!
         let mut featsmap = FxHashMap::default();
 
-        // insert implied features
+        // Ensure that all ABI-required features are enabled, and the ABI-forbidden ones
+        // are disabled.
+        let abi_feature_constraints = sess.target.abi_required_features();
+        let abi_incompatible_set =
+            FxHashSet::from_iter(abi_feature_constraints.incompatible.iter().copied());
+
+        // Compute implied features
         let mut all_rust_features = vec![];
         for feature in sess.opts.cg.target_feature.split(',') {
-            match feature.strip_prefix('+') {
+            if let Some(feature) = feature.strip_prefix('+') {
-                Some(feature) => all_rust_features.extend(
+                all_rust_features.extend(
-                    UnordSet::from(
+                    UnordSet::from(sess.target.implied_target_features(std::iter::once(feature)))
-                        sess.target
-                            .implied_target_features(std::iter::once(Symbol::intern(feature))),
-                    )
                         .to_sorted_stable_ord()
                         .iter()
-                    .map(|s| format!("+{}", s.as_str())),
+                        .map(|&&s| (true, s)),
-                ),
+                )
-                _ => all_rust_features.push(feature.to_string()),
+            } else if let Some(feature) = feature.strip_prefix('-') {
-            }
+                // FIXME: Why do we not remove implied features on "-" here?
-        }
+                // We do the equivalent above in `target_features_cfg`.
-
+                // See <https://github.com/rust-lang/rust/issues/134792>.
-        let feats = all_rust_features
+                all_rust_features.push((false, feature));
-            .iter()
+            } else if !feature.is_empty() {
-            .filter_map(|s| {
-                let enable_disable = match s.chars().next() {
-                    None => return None,
-                    Some(c @ ('+' | '-')) => c,
-                    Some(_) => {
                 if diagnostics {
-                            sess.dcx().emit_warn(UnknownCTargetFeaturePrefix { feature: s });
+                    sess.dcx().emit_warn(UnknownCTargetFeaturePrefix { feature });
                 }
-                        return None;
             }
-                };
+        }
+        // Remove features that are meant for rustc, not LLVM.
+        all_rust_features.retain(|(_, feature)| {
+            // Retain if it is not a rustc feature
+            !RUSTC_SPECIFIC_FEATURES.contains(feature)
+        });
 
-                // Get the backend feature name, if any.
+        // Check feature validity.
-                // This excludes rustc-specific features, which do not get passed to LLVM.
-                let feature = backend_feature_name(sess, s)?;
-                // Warn against use of LLVM specific feature names and unstable features on the CLI.
         if diagnostics {
+            for &(enable, feature) in &all_rust_features {
                 let feature_state = known_features.iter().find(|&&(v, _, _)| v == feature);
                 match feature_state {
                     None => {
@@ -709,18 +723,17 @@ pub(crate) fn global_llvm_features(
                                 rust_feature: PossibleFeature::Some { rust_feature },
                             }
                         } else {
-                                UnknownCTargetFeature {
+                            UnknownCTargetFeature { feature, rust_feature: PossibleFeature::None }
-                                    feature,
-                                    rust_feature: PossibleFeature::None,
-                                }
                         };
                         sess.dcx().emit_warn(unknown_feature);
                     }
                     Some((_, stability, _)) => {
-                            if let Err(reason) =
+                        if let Err(reason) = stability.toggle_allowed() {
-                                stability.toggle_allowed(&sess.target, enable_disable == '+')
+                            sess.dcx().emit_warn(ForbiddenCTargetFeature {
-                            {
+                                feature,
-                                sess.dcx().emit_warn(ForbiddenCTargetFeature { feature, reason });
+                                enabled: if enable { "enabled" } else { "disabled" },
+                                reason,
+                            });
                         } else if stability.requires_nightly().is_some() {
                             // An unstable feature. Warn about using it. It makes little sense
                             // to hard-error here since we just warn about fully unknown
@@ -730,10 +743,57 @@ pub(crate) fn global_llvm_features(
                     }
                 }
 
-                    // FIXME(nagisa): figure out how to not allocate a full hashset here.
+                // Ensure that the features we enable/disable are compatible with the ABI.
-                    featsmap.insert(feature, enable_disable == '+');
+                if enable {
+                    if abi_incompatible_set.contains(feature) {
+                        sess.dcx().emit_warn(ForbiddenCTargetFeature {
+                            feature,
+                            enabled: "enabled",
+                            reason: "this feature is incompatible with the target ABI",
+                        });
+                    }
+                } else {
+                    // FIXME: we have to request implied features here since
+                    // negative features do not handle implied features above.
+                    for &required in abi_feature_constraints.required.iter() {
+                        let implied =
+                            sess.target.implied_target_features(std::iter::once(required));
+                        if implied.contains(feature) {
+                            sess.dcx().emit_warn(ForbiddenCTargetFeature {
+                                feature,
+                                enabled: "disabled",
+                                reason: "this feature is required by the target ABI",
+                            });
+                        }
+                    }
                 }
 
+                // FIXME(nagisa): figure out how to not allocate a full hashset here.
+                featsmap.insert(feature, enable);
+            }
+        }
+
+        // To be sure the ABI-relevant features are all in the right state, we explicitly
+        // (un)set them here. This means if the target spec sets those features wrong,
+        // we will silently correct them rather than silently producing wrong code.
+        // (The target sanity check tries to catch this, but we can't know which features are
+        // enabled in LLVM by default so we can't be fully sure about that check.)
+        // We add these at the beginning of the list so that `-Ctarget-features` can
+        // still override it... that's unsound, but more compatible with past behavior.
+        all_rust_features.splice(
+            0..0,
+            abi_feature_constraints
+                .required
+                .iter()
+                .map(|&f| (true, f))
+                .chain(abi_feature_constraints.incompatible.iter().map(|&f| (false, f))),
+        );
+
+        // Translate this into LLVM features.
+        let feats = all_rust_features
+            .iter()
+            .filter_map(|&(enable, feature)| {
+                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
@@ -746,9 +806,9 @@ pub(crate) fn global_llvm_features(
                         enable_disable, llvm_feature.llvm_feature_name
                     ))
                     .chain(llvm_feature.dependency.into_iter().filter_map(
-                        move |feat| match (enable_disable, feat) {
+                        move |feat| match (enable, feat) {
-                            ('-' | '+', TargetFeatureFoldStrength::Both(f))
+                            (_, TargetFeatureFoldStrength::Both(f))
-                            | ('+', TargetFeatureFoldStrength::EnableOnly(f)) => {
+                            | (true, TargetFeatureFoldStrength::EnableOnly(f)) => {
                                 Some(format!("{enable_disable}{f}"))
                             }
                             _ => None,
@@ -780,22 +840,6 @@ pub(crate) fn global_llvm_features(
     features
 }
 
-/// Returns a feature name for the given `+feature` or `-feature` string.
-///
-/// Only allows features that are backend specific (i.e. not [`RUSTC_SPECIFIC_FEATURES`].)
-fn backend_feature_name<'a>(sess: &Session, s: &'a str) -> Option<&'a str> {
-    // features must start with a `+` or `-`.
-    let feature = s
-        .strip_prefix(&['+', '-'][..])
-        .unwrap_or_else(|| sess.dcx().emit_fatal(InvalidTargetFeaturePrefix { feature: s }));
-    // Rustc-specific feature requests like `+crt-static` or `-crt-static`
-    // are not passed down to LLVM.
-    if s.is_empty() || RUSTC_SPECIFIC_FEATURES.contains(&feature) {
-        return None;
-    }
-    Some(feature)
-}
-
 pub(crate) fn tune_cpu(sess: &Session) -> Option<&str> {
     let name = sess.opts.unstable_opts.tune_cpu.as_ref()?;
     Some(handle_native(name))

compiler/rustc_codegen_ssa/messages.ftl

@@ -67,7 +67,7 @@ codegen_ssa_failed_to_write = failed to write {$path}: {$error}
 codegen_ssa_field_associated_value_expected = associated value expected for `{$name}`
 
 codegen_ssa_forbidden_target_feature_attr =
-    target feature `{$feature}` cannot be toggled with `#[target_feature]`: {$reason}
+    target feature `{$feature}` cannot be enabled with `#[target_feature]`: {$reason}
 
 codegen_ssa_ignoring_emit_path = ignoring emit path because multiple .{$extension} files were produced
 

compiler/rustc_codegen_ssa/src/target_features.rs

@@ -19,7 +19,7 @@ use crate::errors;
 pub(crate) fn from_target_feature_attr(
     tcx: TyCtxt<'_>,
     attr: &hir::Attribute,
-    rust_target_features: &UnordMap<String, target_features::StabilityComputed>,
+    rust_target_features: &UnordMap<String, target_features::Stability>,
     target_features: &mut Vec<TargetFeature>,
 ) {
     let Some(list) = attr.meta_item_list() else { return };
@@ -32,7 +32,7 @@ pub(crate) fn from_target_feature_attr(
             .emit();
     };
     let rust_features = tcx.features();
-    let mut added_target_features = Vec::new();
+    let abi_feature_constraints = tcx.sess.target.abi_required_features();
     for item in list {
         // Only `enable = ...` is accepted in the meta-item list.
         if !item.has_name(sym::enable) {
@@ -47,7 +47,7 @@ pub(crate) fn from_target_feature_attr(
         };
 
         // We allow comma separation to enable multiple features.
-        added_target_features.extend(value.as_str().split(',').filter_map(|feature| {
+        for feature in value.as_str().split(',') {
             let Some(stability) = rust_target_features.get(feature) else {
                 let msg = format!("the feature named `{feature}` is not valid for this target");
                 let mut err = tcx.dcx().struct_span_err(item.span(), msg);
@@ -59,12 +59,12 @@ pub(crate) fn from_target_feature_attr(
                     }
                 }
                 err.emit();
-                return None;
+                continue;
             };
 
             // Only allow target features whose feature gates have been enabled
             // and which are permitted to be toggled.
-            if let Err(reason) = stability.toggle_allowed(/*enable*/ true) {
+            if let Err(reason) = stability.toggle_allowed() {
                 tcx.dcx().emit_err(errors::ForbiddenTargetFeatureAttr {
                     span: item.span(),
                     feature,
@@ -80,31 +80,25 @@ pub(crate) fn from_target_feature_attr(
                     format!("the target feature `{feature}` is currently unstable"),
                 )
                 .emit();
+            } else {
+                // Add this and the implied features.
+                let feature_sym = Symbol::intern(feature);
+                for &name in tcx.implied_target_features(feature_sym) {
+                    // But ensure the ABI does not forbid enabling this.
+                    // Here we do assume that LLVM doesn't add even more implied features
+                    // we don't know about, at least no features that would have ABI effects!
+                    if abi_feature_constraints.incompatible.contains(&name.as_str()) {
+                        tcx.dcx().emit_err(errors::ForbiddenTargetFeatureAttr {
+                            span: item.span(),
+                            feature: name.as_str(),
+                            reason: "this feature is incompatible with the target ABI",
+                        });
+                    }
+                    target_features.push(TargetFeature { name, implied: name != feature_sym })
                 }
-            Some(Symbol::intern(feature))
-        }));
             }
-
-    // Add explicit features
-    target_features.extend(
-        added_target_features.iter().copied().map(|name| TargetFeature { name, implied: false }),
-    );
-
-    // Add implied features
-    let mut implied_target_features = UnordSet::new();
-    for feature in added_target_features.iter() {
-        implied_target_features.extend(tcx.implied_target_features(*feature).clone());
         }
-    for feature in added_target_features.iter() {
-        implied_target_features.remove(feature);
     }
-    target_features.extend(
-        implied_target_features
-            .into_sorted_stable_ord()
-            .iter()
-            .copied()
-            .map(|name| TargetFeature { name, implied: true }),
-    )
 }
 
 /// Computes the set of target features used in a function for the purposes of
@@ -147,25 +141,28 @@ pub(crate) fn provide(providers: &mut Providers) {
     *providers = Providers {
         rust_target_features: |tcx, cnum| {
             assert_eq!(cnum, LOCAL_CRATE);
-            let target = &tcx.sess.target;
             if tcx.sess.opts.actually_rustdoc {
                 // rustdoc needs to be able to document functions that use all the features, so
                 // whitelist them all
                 rustc_target::target_features::all_rust_features()
-                    .map(|(a, b)| (a.to_string(), b.compute_toggleability(target)))
+                    .map(|(a, b)| (a.to_string(), b))
                     .collect()
             } else {
                 tcx.sess
                     .target
                     .rust_target_features()
                     .iter()
-                    .map(|(a, b, _)| (a.to_string(), b.compute_toggleability(target)))
+                    .map(|(a, b, _)| (a.to_string(), *b))
                     .collect()
             }
         },
-        implied_target_features: |tcx, feature| {
+        implied_target_features: |tcx, feature: Symbol| {
+            let feature = feature.as_str();
             UnordSet::from(tcx.sess.target.implied_target_features(std::iter::once(feature)))
                 .into_sorted_stable_ord()
+                .into_iter()
+                .map(|s| Symbol::intern(s))
+                .collect()
         },
         asm_target_features,
         ..*providers

compiler/rustc_middle/src/middle/codegen_fn_attrs.rs

@@ -28,6 +28,7 @@ pub struct CodegenFnAttrs {
     pub link_ordinal: Option<u16>,
     /// The `#[target_feature(enable = "...")]` attribute and the enabled
     /// features (only enabled features are supported right now).
+    /// Implied target features have already been applied.
     pub target_features: Vec<TargetFeature>,
     /// The `#[linkage = "..."]` attribute on Rust-defined items and the value we found.
     pub linkage: Option<Linkage>,

compiler/rustc_middle/src/query/mod.rs

@@ -2357,7 +2357,7 @@ rustc_queries! {
     }
 
     /// Returns the Rust target features for the current target. These are not always the same as LLVM target features!
-    query rust_target_features(_: CrateNum) -> &'tcx UnordMap<String, rustc_target::target_features::StabilityComputed> {
+    query rust_target_features(_: CrateNum) -> &'tcx UnordMap<String, rustc_target::target_features::Stability> {
         arena_cache
         eval_always
         desc { "looking up Rust target features" }

compiler/rustc_target/src/spec/mod.rs

@@ -2653,10 +2653,6 @@ impl TargetOptions {
     pub(crate) fn has_feature(&self, search_feature: &str) -> bool {
         self.features.split(',').any(|f| f.strip_prefix('+').is_some_and(|f| f == search_feature))
     }
-
-    pub(crate) fn has_neg_feature(&self, search_feature: &str) -> bool {
-        self.features.split(',').any(|f| f.strip_prefix('-').is_some_and(|f| f == search_feature))
-    }
 }
 
 impl Default for TargetOptions {
@@ -3203,7 +3199,8 @@ impl Target {
                 check_matches!(
                     &*self.llvm_abiname,
                     "ilp32" | "ilp32f" | "ilp32d" | "ilp32e",
-                    "invalid RISC-V ABI name"
+                    "invalid RISC-V ABI name: {}",
+                    self.llvm_abiname,
                 );
             }
             "riscv64" => {
@@ -3211,7 +3208,8 @@ impl Target {
                 check_matches!(
                     &*self.llvm_abiname,
                     "lp64" | "lp64f" | "lp64d" | "lp64e",
-                    "invalid RISC-V ABI name"
+                    "invalid RISC-V ABI name: {}",
+                    self.llvm_abiname,
                 );
             }
             "arm" => {
@@ -3245,6 +3243,26 @@ impl Target {
                     ));
                 }
             }
+            // Check that we don't mis-set any of the ABI-relevant features.
+            let abi_feature_constraints = self.abi_required_features();
+            for feat in abi_feature_constraints.required {
+                // The feature might be enabled by default so we can't *require* it to show up.
+                // But it must not be *disabled*.
+                if features_disabled.contains(feat) {
+                    return Err(format!(
+                        "target feature `{feat}` is required by the ABI but gets disabled in target spec"
+                    ));
+                }
+            }
+            for feat in abi_feature_constraints.incompatible {
+                // The feature might be disabled by default so we can't *require* it to show up.
+                // But it must not be *enabled*.
+                if features_enabled.contains(feat) {
+                    return Err(format!(
+                        "target feature `{feat}` is incompatible with the ABI but gets enabled in target spec"
+                    ));
+                }
+            }
         }
 
         Ok(())

tests/codegen/target-feature-overrides.rs

@@ -39,8 +39,8 @@ pub unsafe fn banana() -> u32 {
 }
 
 // CHECK: attributes [[APPLEATTRS]]
-// COMPAT-SAME: "target-features"="+avx,+avx2,{{.*}}"
+// COMPAT-SAME: "target-features"="+x87,+sse2,+avx,+avx2,{{.*}}"
-// INCOMPAT-SAME: "target-features"="-avx2,-avx,+avx,{{.*}}"
+// INCOMPAT-SAME: "target-features"="+x87,+sse2,-avx2,-avx,+avx,{{.*}}"
 // CHECK: attributes [[BANANAATTRS]]
-// COMPAT-SAME: "target-features"="+avx,+avx2,{{.*}}"
+// COMPAT-SAME: "target-features"="+x87,+sse2,+avx,+avx2,{{.*}}"
-// INCOMPAT-SAME: "target-features"="-avx2,-avx"
+// INCOMPAT-SAME: "target-features"="+x87,+sse2,-avx2,-avx"

tests/codegen/tied-features-strength.rs

@@ -11,10 +11,11 @@
 // ENABLE_SVE: attributes #0 = { {{.*}} "target-features"="{{((\+outline-atomics,?)|(\+v8a,?)|(\+fpmr,?)?|(\+sve,?)|(\+neon,?)|(\+fp-armv8,?))*}}" }
 
 //@ [DISABLE_SVE] compile-flags: -C target-feature=-sve -Copt-level=0
-// DISABLE_SVE: attributes #0 = { {{.*}} "target-features"="{{((\+outline-atomics,?)|(\+v8a,?)|(\+fpmr,?)?|(-sve,?)|(\+neon,?))*}}" }
+// DISABLE_SVE: attributes #0 = { {{.*}} "target-features"="{{((\+outline-atomics,?)|(\+v8a,?)|(\+fpmr,?)?|(-sve,?)|(\+neon,?)|(\+fp-armv8,?))*}}" }
 
 //@ [DISABLE_NEON] compile-flags: -C target-feature=-neon -Copt-level=0
-// DISABLE_NEON: attributes #0 = { {{.*}} "target-features"="{{((\+outline-atomics,?)|(\+v8a,?)|(\+fpmr,?)?|(-fp-armv8,?)|(-neon,?))*}}" }
+// `neon` and `fp-armv8` get enabled as target base features, but then disabled again at the end of the list.
+// DISABLE_NEON: attributes #0 = { {{.*}} "target-features"="{{((\+outline-atomics,?)|(\+v8a,?)|(\+fp-armv8,?)|(\+neon,?))*}},-neon,-fp-armv8{{(,\+fpmr)?}}" }
 
 //@ [ENABLE_NEON] compile-flags: -C target-feature=+neon -Copt-level=0
 // ENABLE_NEON: attributes #0 = { {{.*}} "target-features"="{{((\+outline-atomics,?)|(\+v8a,?)|(\+fpmr,?)?|(\+fp-armv8,?)|(\+neon,?))*}}" }

tests/run-make/simd-ffi/rmake.rs

@@ -56,7 +56,7 @@ fn main() {
             .target(&target)
             .emit("llvm-ir,asm")
             .input("simd.rs")
-            .arg("-Ctarget-feature=+neon,+sse")
+            .arg("-Ctarget-feature=-soft-float,+neon,+sse")
             .arg(&format!("-Cextra-filename=-{target}"))
             .run();
     }

tests/ui/target-feature/feature-hierarchy.aarch64-sve2.stderr

@@ -1,4 +1,4 @@
-warning: target feature `neon` cannot be toggled with `-Ctarget-feature`: unsound on hard-float targets because it changes float ABI
+warning: target feature `neon` cannot be disabled with `-Ctarget-feature`: this feature is required by the target ABI
    |
    = note: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
    = note: for more information, see issue #116344 <https://github.com/rust-lang/rust/issues/116344>

tests/ui/target-feature/forbidden-hardfloat-target-feature-attribute.rs

@@ -1,11 +1,11 @@
-//@ compile-flags: --target=x86_64-unknown-linux-gnu --crate-type=lib
+//@ compile-flags: --target=riscv32e-unknown-none-elf --crate-type=lib
-//@ needs-llvm-components: x86
+//@ needs-llvm-components: riscv
-#![feature(no_core, lang_items)]
+#![feature(no_core, lang_items, riscv_target_feature)]
 #![no_core]
 
 #[lang = "sized"]
 pub trait Sized {}
 
-#[target_feature(enable = "x87")]
+#[target_feature(enable = "d")]
-//~^ERROR: cannot be toggled with
+//~^ERROR: cannot be enabled with
 pub unsafe fn my_fun() {}

tests/ui/target-feature/forbidden-hardfloat-target-feature-attribute.stderr

@@ -1,8 +1,8 @@
-error: target feature `x87` cannot be toggled with `#[target_feature]`: unsound on hard-float targets because it changes float ABI
+error: target feature `d` cannot be enabled with `#[target_feature]`: this feature is incompatible with the target ABI
   --> $DIR/forbidden-hardfloat-target-feature-attribute.rs:9:18
    |
-LL | #[target_feature(enable = "x87")]
+LL | #[target_feature(enable = "d")]
-   |                  ^^^^^^^^^^^^^^
+   |                  ^^^^^^^^^^^^
 
 error: aborting due to 1 previous error
 

tests/ui/target-feature/forbidden-hardfloat-target-feature-flag-disable-implied.rs

@@ -0,0 +1,10 @@
+//@ compile-flags: --target=x86_64-unknown-linux-gnu --crate-type=lib
+//@ needs-llvm-components: x86
+//@ compile-flags: -Ctarget-feature=-sse
+// For now this is just a warning.
+//@ build-pass
+#![feature(no_core, lang_items)]
+#![no_core]
+
+#[lang = "sized"]
+pub trait Sized {}

tests/ui/target-feature/forbidden-hardfloat-target-feature-flag-disable-implied.stderr

@@ -0,0 +1,7 @@
+warning: target feature `sse` cannot be disabled with `-Ctarget-feature`: this feature is required by the target ABI
+   |
+   = note: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
+   = note: for more information, see issue #116344 <https://github.com/rust-lang/rust/issues/116344>
+
+warning: 1 warning emitted
+

tests/ui/target-feature/forbidden-hardfloat-target-feature-flag-disable-neon.stderr

@@ -1,4 +1,4 @@
-warning: target feature `neon` cannot be toggled with `-Ctarget-feature`: unsound on hard-float targets because it changes float ABI
+warning: target feature `neon` cannot be disabled with `-Ctarget-feature`: this feature is required by the target ABI
    |
    = note: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
    = note: for more information, see issue #116344 <https://github.com/rust-lang/rust/issues/116344>

tests/ui/target-feature/forbidden-hardfloat-target-feature-flag-disable.stderr

@@ -1,7 +1,11 @@
-warning: target feature `x87` cannot be toggled with `-Ctarget-feature`: unsound on hard-float targets because it changes float ABI
+warning: unstable feature specified for `-Ctarget-feature`: `x87`
+   |
+   = note: this feature is not stably supported; its behavior can change in the future
+
+warning: target feature `x87` cannot be disabled with `-Ctarget-feature`: this feature is required by the target ABI
    |
    = note: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
    = note: for more information, see issue #116344 <https://github.com/rust-lang/rust/issues/116344>
 
-warning: 1 warning emitted
+warning: 2 warnings emitted
 

tests/ui/target-feature/forbidden-target-feature-attribute.rs

@@ -7,5 +7,5 @@
 pub trait Sized {}
 
 #[target_feature(enable = "soft-float")]
-//~^ERROR: cannot be toggled with
+//~^ERROR: cannot be enabled with
 pub unsafe fn my_fun() {}

tests/ui/target-feature/forbidden-target-feature-attribute.stderr

@@ -1,4 +1,4 @@
-error: target feature `soft-float` cannot be toggled with `#[target_feature]`: unsound because it changes float ABI
+error: target feature `soft-float` cannot be enabled with `#[target_feature]`: unsound because it changes float ABI
   --> $DIR/forbidden-target-feature-attribute.rs:9:18
    |
 LL | #[target_feature(enable = "soft-float")]

tests/ui/target-feature/forbidden-target-feature-flag-disable.stderr

@@ -1,4 +1,4 @@
-warning: target feature `soft-float` cannot be toggled with `-Ctarget-feature`: unsound because it changes float ABI
+warning: target feature `soft-float` cannot be disabled with `-Ctarget-feature`: unsound because it changes float ABI
    |
    = note: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
    = note: for more information, see issue #116344 <https://github.com/rust-lang/rust/issues/116344>

tests/ui/target-feature/forbidden-target-feature-flag.stderr

@@ -1,4 +1,4 @@
-warning: target feature `soft-float` cannot be toggled with `-Ctarget-feature`: unsound because it changes float ABI
+warning: target feature `soft-float` cannot be enabled with `-Ctarget-feature`: unsound because it changes float ABI
    |
    = note: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
    = note: for more information, see issue #116344 <https://github.com/rust-lang/rust/issues/116344>