diff --git a/compiler/rustc_codegen_gcc/src/gcc_util.rs b/compiler/rustc_codegen_gcc/src/gcc_util.rs index 88e5eefd7a156..058a874501b26 100644 --- a/compiler/rustc_codegen_gcc/src/gcc_util.rs +++ b/compiler/rustc_codegen_gcc/src/gcc_util.rs @@ -96,7 +96,7 @@ pub(crate) fn global_gcc_features(sess: &Session, diagnostics: bool) -> Vec { if let Err(reason) = - stability.compute_toggleability(&sess.target).allow_toggle() + stability.toggle_allowed(&sess.target, enable_disable == '+') { sess.dcx().emit_warn(ForbiddenCTargetFeature { feature, reason }); } else if stability.requires_nightly().is_some() { diff --git a/compiler/rustc_codegen_gcc/src/lib.rs b/compiler/rustc_codegen_gcc/src/lib.rs index 764e84be1feb9..bb0f2fa5b014e 100644 --- a/compiler/rustc_codegen_gcc/src/lib.rs +++ b/compiler/rustc_codegen_gcc/src/lib.rs @@ -483,9 +483,9 @@ fn target_features_cfg( .rust_target_features() .iter() .filter(|(_, gate, _)| gate.in_cfg()) - .filter_map(|&(feature, gate, _)| { + .filter_map(|(feature, gate, _)| { if sess.is_nightly_build() || allow_unstable || gate.requires_nightly().is_none() { - Some(feature) + Some(*feature) } else { None } diff --git a/compiler/rustc_codegen_llvm/src/llvm_util.rs b/compiler/rustc_codegen_llvm/src/llvm_util.rs index bfec7d708cfbc..194438af88c26 100644 --- a/compiler/rustc_codegen_llvm/src/llvm_util.rs +++ b/compiler/rustc_codegen_llvm/src/llvm_util.rs @@ -373,9 +373,9 @@ pub fn target_features_cfg(sess: &Session, allow_unstable: bool) -> Vec .rust_target_features() .iter() .filter(|(_, gate, _)| gate.in_cfg()) - .filter_map(|&(feature, gate, _)| { + .filter_map(|(feature, gate, _)| { if sess.is_nightly_build() || allow_unstable || gate.requires_nightly().is_none() { - Some(feature) + Some(*feature) } else { None } @@ -718,7 +718,7 @@ pub(crate) fn global_llvm_features( } Some((_, stability, _)) => { if let Err(reason) = - stability.compute_toggleability(&sess.target).allow_toggle() + stability.toggle_allowed(&sess.target, enable_disable == '+') { sess.dcx().emit_warn(ForbiddenCTargetFeature { feature, reason }); } else if stability.requires_nightly().is_some() { diff --git a/compiler/rustc_codegen_ssa/src/target_features.rs b/compiler/rustc_codegen_ssa/src/target_features.rs index fa600ec7166ad..f4d4a9db1d898 100644 --- a/compiler/rustc_codegen_ssa/src/target_features.rs +++ b/compiler/rustc_codegen_ssa/src/target_features.rs @@ -65,7 +65,7 @@ pub(crate) fn from_target_feature_attr( // Only allow target features whose feature gates have been enabled // and which are permitted to be toggled. - if let Err(reason) = stability.allow_toggle() { + if let Err(reason) = stability.toggle_allowed(/*enable*/ true) { tcx.dcx().emit_err(errors::ForbiddenTargetFeatureAttr { span: item.span(), feature, @@ -160,7 +160,7 @@ pub(crate) fn provide(providers: &mut Providers) { .target .rust_target_features() .iter() - .map(|&(a, b, _)| (a.to_string(), b.compute_toggleability(target))) + .map(|(a, b, _)| (a.to_string(), b.compute_toggleability(target))) .collect() } }, diff --git a/compiler/rustc_target/src/spec/mod.rs b/compiler/rustc_target/src/spec/mod.rs index 7d308c6c662cd..a44d2af6b9086 100644 --- a/compiler/rustc_target/src/spec/mod.rs +++ b/compiler/rustc_target/src/spec/mod.rs @@ -2213,6 +2213,10 @@ pub struct TargetOptions { /// `-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! + /// + /// 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 + /// features you want to use. pub features: StaticCow, /// Direct or use GOT indirect to reference external data symbols pub direct_access_external_data: Option, @@ -2605,15 +2609,11 @@ impl TargetOptions { } pub(crate) fn has_feature(&self, search_feature: &str) -> bool { - self.features.split(',').any(|f| { - if let Some(f) = f.strip_prefix('+') - && f == search_feature - { - true - } else { - false - } - }) + 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)) } } diff --git a/compiler/rustc_target/src/target_features.rs b/compiler/rustc_target/src/target_features.rs index 29d3f826a1544..713b5dc70d7ad 100644 --- a/compiler/rustc_target/src/target_features.rs +++ b/compiler/rustc_target/src/target_features.rs @@ -20,7 +20,7 @@ pub const RUSTC_SPECIAL_FEATURES: &[&str] = &["backchain"]; /// `Toggleability` is the type storing whether (un)stable features can be toggled: /// this is initially a function since it can depend on `Target`, but for stable hashing /// it needs to be something hashable to we have to make the type generic. -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone)] pub enum Stability { /// This target feature is stable, it can be used in `#[target_feature]` and /// `#[cfg(target_feature)]`. @@ -44,11 +44,21 @@ pub enum Stability { Forbidden { reason: &'static str }, } -/// `Stability` where `allow_toggle` has not been computed yet. /// Returns `Ok` if the toggle is allowed, `Err` with an explanation of not. -pub type StabilityUncomputed = Stability Result<(), &'static str>>; +/// The `bool` indicates whether the feature is being enabled (`true`) or disabled. +pub type AllowToggleUncomputed = fn(&Target, bool) -> Result<(), &'static str>; + +/// The computed result of whether a feature can be enabled/disabled on the current target. +#[derive(Debug, Clone)] +pub struct AllowToggleComputed { + enable: Result<(), &'static str>, + disable: Result<(), &'static str>, +} + +/// `Stability` where `allow_toggle` has not been computed yet. +pub type StabilityUncomputed = Stability; /// `Stability` where `allow_toggle` has already been computed. -pub type StabilityComputed = Stability>; +pub type StabilityComputed = Stability; impl> HashStable for Stability { #[inline] @@ -69,11 +79,20 @@ impl> HashStable for Stability HashStable for AllowToggleComputed { + #[inline] + fn hash_stable(&self, hcx: &mut CTX, hasher: &mut StableHasher) { + let AllowToggleComputed { enable, disable } = self; + enable.hash_stable(hcx, hasher); + disable.hash_stable(hcx, hasher); + } +} + impl Stability { /// Returns whether the feature can be used in `cfg(target_feature)` ever. /// (It might still be nightly-only even if this returns `true`, so make sure to also check /// `requires_nightly`.) - pub fn in_cfg(self) -> bool { + pub fn in_cfg(&self) -> bool { !matches!(self, Stability::Forbidden { .. }) } @@ -85,8 +104,8 @@ impl Stability { /// Before calling this, ensure the feature is even permitted for this use: /// - for `#[target_feature]`/`-Ctarget-feature`, check `allow_toggle()` /// - for `cfg(target_feature)`, check `in_cfg` - pub fn requires_nightly(self) -> Option { - match self { + pub fn requires_nightly(&self) -> Option { + match *self { Stability::Unstable { nightly_feature, .. } => Some(nightly_feature), Stability::Stable { .. } => None, Stability::Forbidden { .. } => panic!("forbidden features should not reach this far"), @@ -95,35 +114,49 @@ impl Stability { } impl StabilityUncomputed { - pub fn compute_toggleability(self, target: &Target) -> StabilityComputed { + pub fn compute_toggleability(&self, target: &Target) -> StabilityComputed { use Stability::*; - match self { - Stable { allow_toggle } => Stable { allow_toggle: allow_toggle(target) }, + let compute = |f: AllowToggleUncomputed| AllowToggleComputed { + enable: f(target, true), + disable: f(target, false), + }; + match *self { + Stable { allow_toggle } => Stable { allow_toggle: compute(allow_toggle) }, Unstable { nightly_feature, allow_toggle } => { - Unstable { nightly_feature, allow_toggle: allow_toggle(target) } + Unstable { nightly_feature, allow_toggle: compute(allow_toggle) } } Forbidden { reason } => Forbidden { reason }, } } + + pub fn toggle_allowed(&self, target: &Target, enable: bool) -> Result<(), &'static str> { + use Stability::*; + match *self { + Stable { allow_toggle } => allow_toggle(target, enable), + Unstable { allow_toggle, .. } => allow_toggle(target, enable), + Forbidden { reason } => Err(reason), + } + } } impl StabilityComputed { /// Returns whether the feature may be toggled via `#[target_feature]` or `-Ctarget-feature`. /// (It might still be nightly-only even if this returns `true`, so make sure to also check /// `requires_nightly`.) - pub fn allow_toggle(self) -> Result<(), &'static str> { - match self { + pub fn toggle_allowed(&self, enable: bool) -> Result<(), &'static str> { + let allow_toggle = match self { Stability::Stable { allow_toggle } => allow_toggle, Stability::Unstable { allow_toggle, .. } => allow_toggle, - Stability::Forbidden { reason } => Err(reason), - } + Stability::Forbidden { reason } => return Err(reason), + }; + if enable { allow_toggle.enable } else { allow_toggle.disable } } } // Constructors for the list below, defaulting to "always allow toggle". -const STABLE: StabilityUncomputed = Stability::Stable { allow_toggle: |_target| Ok(()) }; +const STABLE: StabilityUncomputed = Stability::Stable { allow_toggle: |_target, _enable| Ok(()) }; const fn unstable(nightly_feature: Symbol) -> StabilityUncomputed { - Stability::Unstable { nightly_feature, allow_toggle: |_target| Ok(()) } + Stability::Unstable { nightly_feature, allow_toggle: |_target, _enable| Ok(()) } } // Here we list target features that rustc "understands": they can be used in `#[target_feature]` @@ -184,7 +217,7 @@ const ARM_FEATURES: &[(&str, StabilityUncomputed, ImpliedFeatures)] = &[ "fpregs", Stability::Unstable { nightly_feature: sym::arm_target_feature, - allow_toggle: |target: &Target| { + allow_toggle: |target: &Target, _enable| { // Only allow toggling this if the target has `soft-float` set. With `soft-float`, // `fpregs` isn't needed so changing it cannot affect the ABI. if target.has_feature("soft-float") { @@ -257,6 +290,7 @@ const AARCH64_FEATURES: &[(&str, StabilityUncomputed, ImpliedFeatures)] = &[ ("flagm", STABLE, &[]), // FEAT_FLAGM2 ("flagm2", unstable(sym::aarch64_unstable_target_feature), &[]), + ("fp-armv8", Stability::Forbidden { reason: "Rust ties `fp-armv8` to `neon`" }, &[]), // FEAT_FP16 // Rust ties FP and Neon: https://github.com/rust-lang/rust/pull/91608 ("fp16", STABLE, &["neon"]), @@ -292,7 +326,28 @@ const AARCH64_FEATURES: &[(&str, StabilityUncomputed, ImpliedFeatures)] = &[ // FEAT_MTE & FEAT_MTE2 ("mte", STABLE, &[]), // FEAT_AdvSimd & FEAT_FP - ("neon", STABLE, &[]), + ( + "neon", + Stability::Stable { + allow_toggle: |target, enable| { + if target.abi == "softfloat" { + // `neon` has no ABI implications for softfloat targets, we can allow this. + Ok(()) + } else if enable + && !target.has_neg_feature("fp-armv8") + && !target.has_neg_feature("neon") + { + // neon is enabled by default, and has not been disabled, so enabling it again + // is redundant and we can permit it. Forbidding this would be a breaking change + // since this feature is stable. + Ok(()) + } else { + Err("unsound on hard-float targets because it changes float ABI") + } + }, + }, + &[], + ), // FEAT_PAUTH (address authentication) ("paca", STABLE, &[]), // FEAT_PAUTH (generic authentication) @@ -481,7 +536,7 @@ const X86_FEATURES: &[(&str, StabilityUncomputed, ImpliedFeatures)] = &[ "x87", Stability::Unstable { nightly_feature: sym::x87_target_feature, - allow_toggle: |target: &Target| { + allow_toggle: |target: &Target, _enable| { // Only allow toggling this if the target has `soft-float` set. With `soft-float`, // `fpregs` isn't needed so changing it cannot affect the ABI. if target.has_feature("soft-float") { diff --git a/tests/ui/target-feature/feature-hierarchy.aarch64-sve2.stderr b/tests/ui/target-feature/feature-hierarchy.aarch64-sve2.stderr new file mode 100644 index 0000000000000..d5d7d7aa627ba --- /dev/null +++ b/tests/ui/target-feature/feature-hierarchy.aarch64-sve2.stderr @@ -0,0 +1,7 @@ +warning: target feature `neon` cannot be toggled with `-Ctarget-feature`: unsound on hard-float targets 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 + +warning: 1 warning emitted + diff --git a/tests/ui/target-feature/forbidden-hardfloat-target-feature-flag-disable-neon.rs b/tests/ui/target-feature/forbidden-hardfloat-target-feature-flag-disable-neon.rs new file mode 100644 index 0000000000000..95c366bb3f5be --- /dev/null +++ b/tests/ui/target-feature/forbidden-hardfloat-target-feature-flag-disable-neon.rs @@ -0,0 +1,10 @@ +//@ compile-flags: --target=aarch64-unknown-linux-gnu --crate-type=lib +//@ needs-llvm-components: aarch64 +//@ compile-flags: -Ctarget-feature=-neon +// For now this is just a warning. +//@ build-pass +#![feature(no_core, lang_items)] +#![no_core] + +#[lang = "sized"] +pub trait Sized {} diff --git a/tests/ui/target-feature/forbidden-hardfloat-target-feature-flag-disable-neon.stderr b/tests/ui/target-feature/forbidden-hardfloat-target-feature-flag-disable-neon.stderr new file mode 100644 index 0000000000000..d5d7d7aa627ba --- /dev/null +++ b/tests/ui/target-feature/forbidden-hardfloat-target-feature-flag-disable-neon.stderr @@ -0,0 +1,7 @@ +warning: target feature `neon` cannot be toggled with `-Ctarget-feature`: unsound on hard-float targets 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 + +warning: 1 warning emitted +