about summary refs log tree commit diff
path: root/compiler/rustc_codegen_ssa/src/codegen_attrs.rs
diff options
context:
space:
mode:
authorTrevor Gross <t.gross35@gmail.com>2025-06-20 02:50:38 -0400
committerGitHub <noreply@github.com>2025-06-20 02:50:38 -0400
commitc117ebefd23c47fa71252291f7fb597555c74daa (patch)
tree1d95919b7442fa150f2f4e51e802f112a54a7ad1 /compiler/rustc_codegen_ssa/src/codegen_attrs.rs
parentbab4ca914ea58b5ced4903225fe31e90bfffed5f (diff)
parenta50a3b8e318594c41783294e440d864763e412ef (diff)
downloadrust-c117ebefd23c47fa71252291f7fb597555c74daa.tar.gz
rust-c117ebefd23c47fa71252291f7fb597555c74daa.zip
Rollup merge of #140920 - RalfJung:target-feature-unification, r=nnethercote,WaffleLapkin
Extract some shared code from codegen backend target feature handling

There's a bunch of code duplication between the GCC and LLVM backends in target feature handling. This moves that into new shared helper functions in `rustc_codegen_ssa`.

The first two commits should be purely refactoring. I am fairly sure the LLVM-side behavior stays the same; if the GCC side deliberately diverges from this then I may have missed that. I did account for one divergence, which I do not know is deliberate or not: GCC does not seem to use the `-Ctarget-feature` flag to populate `cfg(target_feature)`. That seems odd, since the `-Ctarget-feature` flag is used to populate the return value of `global_gcc_features` which controls the target features actually used by GCC. ``@GuillaumeGomez`` ``@antoyo`` is there a reason `target_config` ignores `-Ctarget-feature` but `global_gcc_features`  does not? The second commit also cleans up a bunch of unneeded complexity added in https://github.com/rust-lang/rust/pull/135927.

The third commit extracts some shared logic out of the functions that populate `cfg(target_feature)` and the backend target feature set, respectively. This one actually has some slight functional changes:
- Before, with `-Ctarget-feature=-feat`, if there is some other feature `x` that implies `feat` we would *not* add `-x` to the backend target feature set. Now, we do. This fixes rust-lang/rust#134792.
- The logic that removes `x` from `cfg(target_feature)` in this case also changed a bit, avoiding a large number of calls to the (uncached) `sess.target.implied_target_features` (if there were a large number of positive features listed before a negative feature) but instead constructing a full inverse implication map when encountering the first negative feature. Ideally this would be done with queries but the backend target feature logic runs before `tcx` so we can't use that...
- Previously, if feature "a" implied "b" and "b" was unstable, then using `-Ctarget-feature=+a` would also emit a warning about `b`. I had to remove this since when accounting for negative implications, this emits a ton of warnings in a bunch of existing tests... I assume this was unintentional anyway.

The fourth commit increases consistency of the GCC backend with the LLVM backend.

The last commit does some further cleanup:
- Get rid of RUSTC_SPECIAL_FEATURES. It was only needed for s390x "backchain", but since LLVM 19 that is always a regular target feature so we don't need this hack any more. The hack also has various unintended side-effects so we don't want to keep it. Fixes https://github.com/rust-lang/rust/issues/142412.
- Move RUSTC_SPECIFIC_FEATURES handling into the shared parse_rust_feature_flag helper so all consumers of `-Ctarget-feature` that only care about actual target features (and not "crt-static") have it. Previously, we actually set `cfg(target_feature = "crt-static")` twice: once in the backend target feature logic, and once specifically for that one feature. IIUC, some targets are meant to ignore `-Ctarget-feature=+crt-static`, it seems like before this PR that flag still incorrectly enabled `cfg(target_feature = "crt-static")` (but I didn't test this).
- Move fixed_x18 handling together with retpoline handling.
- Forbid setting fixed_x18 as a regular target feature, even unstably. It must be set via the `-Z` flag.

``@bjorn3`` I did not touch the cranelift backend here, since AFAIK it doesn't really support target features. But if you ever do, please use the new helpers. :)

Cc ``@workingjubilee``
Diffstat (limited to 'compiler/rustc_codegen_ssa/src/codegen_attrs.rs')
-rw-r--r--compiler/rustc_codegen_ssa/src/codegen_attrs.rs26
1 files changed, 4 insertions, 22 deletions
diff --git a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs
index cd1fa6a06bb..e855f1d3558 100644
--- a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs
+++ b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs
@@ -6,7 +6,6 @@ use rustc_ast::{LitKind, MetaItem, MetaItemInner, attr};
 use rustc_attr_data_structures::{
     AttributeKind, InlineAttr, InstructionSetAttr, OptimizeAttr, find_attr,
 };
-use rustc_data_structures::fx::FxHashMap;
 use rustc_hir::def::DefKind;
 use rustc_hir::def_id::{DefId, LOCAL_CRATE, LocalDefId};
 use rustc_hir::weak_lang_items::WEAK_LANG_ITEMS;
@@ -18,13 +17,15 @@ use rustc_middle::mir::mono::Linkage;
 use rustc_middle::query::Providers;
 use rustc_middle::span_bug;
 use rustc_middle::ty::{self as ty, TyCtxt};
+use rustc_session::lint;
 use rustc_session::parse::feature_err;
-use rustc_session::{Session, lint};
 use rustc_span::{Ident, Span, sym};
 use rustc_target::spec::SanitizerSet;
 
 use crate::errors;
-use crate::target_features::{check_target_feature_trait_unsafe, from_target_feature_attr};
+use crate::target_features::{
+    check_target_feature_trait_unsafe, check_tied_features, from_target_feature_attr,
+};
 
 fn linkage_by_name(tcx: TyCtxt<'_>, def_id: LocalDefId, name: &str) -> Linkage {
     use rustc_middle::mir::mono::Linkage::*;
@@ -590,25 +591,6 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
     codegen_fn_attrs
 }
 
-/// Given a map from target_features to whether they are enabled or disabled, ensure only valid
-/// combinations are allowed.
-pub fn check_tied_features(
-    sess: &Session,
-    features: &FxHashMap<&str, bool>,
-) -> Option<&'static [&'static str]> {
-    if !features.is_empty() {
-        for tied in sess.target.tied_target_features() {
-            // Tied features must be set to the same value, or not set at all
-            let mut tied_iter = tied.iter();
-            let enabled = features.get(tied_iter.next().unwrap());
-            if tied_iter.any(|f| enabled != features.get(f)) {
-                return Some(tied);
-            }
-        }
-    }
-    None
-}
-
 /// Checks if the provided DefId is a method in a trait impl for a trait which has track_caller
 /// applied to the method prototype.
 fn should_inherit_track_caller(tcx: TyCtxt<'_>, def_id: DefId) -> bool {