Skip to content

Commit

Permalink
Auto merge of #133099 - RalfJung:forbidden-hardfloat-features, r=work…
Browse files Browse the repository at this point in the history
…ingjubilee

forbid toggling x87 and fpregs on hard-float targets

Part of #116344, follow-up to #129884:

The `x87`  target feature on x86 and the `fpregs` target feature on ARM must not be disabled on a hardfloat target, as that would change the float ABI. However, *enabling* `fpregs` on ARM is [explicitly requested](#130988) as it seems to be useful. Therefore, we need to refine the distinction of "forbidden" target features and "allowed" target features: all (un)stable target features can determine on a per-target basis whether they should be allowed to be toggled or not. `fpregs` then checks whether the current target has the `soft-float` feature, and if yes, `fpregs` is permitted -- otherwise, it is not. (Same for `x87` on x86).

Also fixes #132351. Since `fpregs` and `x87` can be enabled on some builds and disabled on others, it would make sense that one can query it via `cfg`. Therefore, I made them behave in `cfg` like any other unstable target feature.

The first commit prepares the infrastructure, but does not change behavior. The second commit then wires up `fpregs` and `x87` with that new infrastructure.

r? `@workingjubilee`
  • Loading branch information
bors committed Dec 13, 2024
2 parents e217f94 + 60eca2c commit 327c7ee
Show file tree
Hide file tree
Showing 32 changed files with 611 additions and 409 deletions.
6 changes: 5 additions & 1 deletion compiler/rustc_codegen_cranelift/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,11 @@ impl CodegenBackend for CraneliftCodegenBackend {
}
}

fn target_features(&self, sess: &Session, _allow_unstable: bool) -> Vec<rustc_span::Symbol> {
fn target_features_cfg(
&self,
sess: &Session,
_allow_unstable: bool,
) -> Vec<rustc_span::Symbol> {
// FIXME return the actually used target features. this is necessary for #[cfg(target_feature)]
if sess.target.arch == "x86_64" && sess.target.os != "none" {
// x86_64 mandates SSE2 support
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_gcc/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ codegen_gcc_lto_not_supported =
LTO is not supported. You may get a linker error.
codegen_gcc_forbidden_ctarget_feature =
target feature `{$feature}` cannot be toggled with `-Ctarget-feature`
target feature `{$feature}` cannot be toggled with `-Ctarget-feature`: {$reason}
codegen_gcc_unwinding_inline_asm =
GCC backend does not support unwinding from inline asm
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_codegen_gcc/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 reason: &'a str,
}

#[derive(Subdiagnostic)]
Expand Down
20 changes: 12 additions & 8 deletions compiler/rustc_codegen_gcc/src/gcc_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use rustc_codegen_ssa::errors::TargetFeatureDisableOrEnable;
use rustc_data_structures::fx::FxHashMap;
use rustc_middle::bug;
use rustc_session::Session;
use rustc_target::target_features::{RUSTC_SPECIFIC_FEATURES, Stability};
use rustc_target::target_features::RUSTC_SPECIFIC_FEATURES;
use smallvec::{SmallVec, smallvec};

use crate::errors::{
Expand Down Expand Up @@ -94,13 +94,17 @@ pub(crate) fn global_gcc_features(sess: &Session, diagnostics: bool) -> Vec<Stri
};
sess.dcx().emit_warn(unknown_feature);
}
Some((_, Stability::Stable, _)) => {}
Some((_, Stability::Unstable(_), _)) => {
// An unstable feature. Warn about using it.
sess.dcx().emit_warn(UnstableCTargetFeature { feature });
}
Some((_, Stability::Forbidden { .. }, _)) => {
sess.dcx().emit_err(ForbiddenCTargetFeature { feature });
Some((_, stability, _)) => {
if let Err(reason) =
stability.compute_toggleability(&sess.target).allow_toggle()
{
sess.dcx().emit_warn(ForbiddenCTargetFeature { feature, 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
// features above).
sess.dcx().emit_warn(UnstableCTargetFeature { feature });
}
}
}

Expand Down
11 changes: 6 additions & 5 deletions compiler/rustc_codegen_gcc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,8 +260,8 @@ impl CodegenBackend for GccCodegenBackend {
.join(sess)
}

fn target_features(&self, sess: &Session, allow_unstable: bool) -> Vec<Symbol> {
target_features(sess, allow_unstable, &self.target_info)
fn target_features_cfg(&self, sess: &Session, allow_unstable: bool) -> Vec<Symbol> {
target_features_cfg(sess, allow_unstable, &self.target_info)
}
}

Expand Down Expand Up @@ -472,7 +472,8 @@ fn to_gcc_opt_level(optlevel: Option<OptLevel>) -> OptimizationLevel {
}
}

pub fn target_features(
/// Returns the features that should be set in `cfg(target_feature)`.
fn target_features_cfg(
sess: &Session,
allow_unstable: bool,
target_info: &LockedTargetInfo,
Expand All @@ -481,9 +482,9 @@ pub fn target_features(
sess.target
.rust_target_features()
.iter()
.filter(|(_, gate, _)| gate.is_supported())
.filter(|(_, gate, _)| gate.in_cfg())
.filter_map(|&(feature, gate, _)| {
if sess.is_nightly_build() || allow_unstable || gate.is_stable() {
if sess.is_nightly_build() || allow_unstable || gate.requires_nightly().is_none() {
Some(feature)
} else {
None
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_codegen_llvm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use std::mem::ManuallyDrop;
use back::owned_target_machine::OwnedTargetMachine;
use back::write::{create_informational_target_machine, create_target_machine};
use errors::ParseTargetMachineConfig;
pub use llvm_util::target_features;
pub use llvm_util::target_features_cfg;
use rustc_ast::expand::allocator::AllocatorKind;
use rustc_codegen_ssa::back::lto::{LtoModuleCodegen, SerializedModule, ThinModule};
use rustc_codegen_ssa::back::write::{
Expand Down Expand Up @@ -330,8 +330,8 @@ impl CodegenBackend for LlvmCodegenBackend {
llvm_util::print_version();
}

fn target_features(&self, sess: &Session, allow_unstable: bool) -> Vec<Symbol> {
target_features(sess, allow_unstable)
fn target_features_cfg(&self, sess: &Session, allow_unstable: bool) -> Vec<Symbol> {
target_features_cfg(sess, allow_unstable)
}

fn codegen_crate<'tcx>(
Expand Down
30 changes: 17 additions & 13 deletions compiler/rustc_codegen_llvm/src/llvm_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use rustc_session::Session;
use rustc_session::config::{PrintKind, PrintRequest};
use rustc_span::symbol::Symbol;
use rustc_target::spec::{MergeFunctions, PanicStrategy, SmallDataThresholdSupport};
use rustc_target::target_features::{RUSTC_SPECIAL_FEATURES, RUSTC_SPECIFIC_FEATURES, Stability};
use rustc_target::target_features::{RUSTC_SPECIAL_FEATURES, RUSTC_SPECIFIC_FEATURES};

use crate::back::write::create_informational_target_machine;
use crate::errors::{
Expand Down Expand Up @@ -300,7 +300,7 @@ pub(crate) fn to_llvm_features<'a>(sess: &Session, s: &'a str) -> Option<LLVMFea
/// Must express features in the way Rust understands them.
///
/// We do not have to worry about RUSTC_SPECIFIC_FEATURES here, those are handled outside codegen.
pub fn target_features(sess: &Session, allow_unstable: bool) -> Vec<Symbol> {
pub fn target_features_cfg(sess: &Session, allow_unstable: bool) -> Vec<Symbol> {
let mut features: FxHashSet<Symbol> = Default::default();

// Add base features for the target.
Expand All @@ -316,7 +316,7 @@ pub fn target_features(sess: &Session, allow_unstable: bool) -> Vec<Symbol> {
sess.target
.rust_target_features()
.iter()
.filter(|(_, gate, _)| gate.is_supported())
.filter(|(_, gate, _)| gate.in_cfg())
.filter(|(feature, _, _)| {
// skip checking special features, as LLVM may not understand them
if RUSTC_SPECIAL_FEATURES.contains(feature) {
Expand Down Expand Up @@ -372,9 +372,9 @@ pub fn target_features(sess: &Session, allow_unstable: bool) -> Vec<Symbol> {
sess.target
.rust_target_features()
.iter()
.filter(|(_, gate, _)| gate.is_supported())
.filter(|(_, gate, _)| gate.in_cfg())
.filter_map(|&(feature, gate, _)| {
if sess.is_nightly_build() || allow_unstable || gate.is_stable() {
if sess.is_nightly_build() || allow_unstable || gate.requires_nightly().is_none() {
Some(feature)
} else {
None
Expand Down Expand Up @@ -493,7 +493,7 @@ fn print_target_features(sess: &Session, tm: &llvm::TargetMachine, out: &mut Str
.rust_target_features()
.iter()
.filter_map(|(feature, gate, _implied)| {
if !gate.is_supported() {
if !gate.in_cfg() {
// Only list (experimentally) supported features.
return None;
}
Expand Down Expand Up @@ -716,13 +716,17 @@ pub(crate) fn global_llvm_features(
};
sess.dcx().emit_warn(unknown_feature);
}
Some((_, Stability::Stable, _)) => {}
Some((_, Stability::Unstable(_), _)) => {
// An unstable feature. Warn about using it.
sess.dcx().emit_warn(UnstableCTargetFeature { feature });
}
Some((_, Stability::Forbidden { reason }, _)) => {
sess.dcx().emit_warn(ForbiddenCTargetFeature { feature, reason });
Some((_, stability, _)) => {
if let Err(reason) =
stability.compute_toggleability(&sess.target).allow_toggle()
{
sess.dcx().emit_warn(ForbiddenCTargetFeature { feature, 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
// features above.
sess.dcx().emit_warn(UnstableCTargetFeature { feature });
}
}
}

Expand Down
53 changes: 23 additions & 30 deletions compiler/rustc_codegen_ssa/src/target_features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use rustc_middle::ty::TyCtxt;
use rustc_session::parse::feature_err;
use rustc_span::Span;
use rustc_span::symbol::{Symbol, sym};
use rustc_target::target_features::{self, Stability};
use rustc_target::target_features;

use crate::errors;

Expand All @@ -20,7 +20,7 @@ use crate::errors;
pub(crate) fn from_target_feature_attr(
tcx: TyCtxt<'_>,
attr: &ast::Attribute,
rust_target_features: &UnordMap<String, target_features::Stability>,
rust_target_features: &UnordMap<String, target_features::StabilityComputed>,
target_features: &mut Vec<TargetFeature>,
) {
let Some(list) = attr.meta_item_list() else { return };
Expand Down Expand Up @@ -63,32 +63,24 @@ pub(crate) fn from_target_feature_attr(
return None;
};

// Only allow target features whose feature gates have been enabled.
let allowed = match stability {
Stability::Forbidden { .. } => false,
Stability::Stable => true,
Stability::Unstable(name) => rust_features.enabled(*name),
};
if !allowed {
match stability {
Stability::Stable => unreachable!(),
&Stability::Unstable(lang_feature_name) => {
feature_err(
&tcx.sess,
lang_feature_name,
item.span(),
format!("the target feature `{feature}` is currently unstable"),
)
.emit();
}
Stability::Forbidden { reason } => {
tcx.dcx().emit_err(errors::ForbiddenTargetFeatureAttr {
span: item.span(),
feature,
reason,
});
}
}
// Only allow target features whose feature gates have been enabled
// and which are permitted to be toggled.
if let Err(reason) = stability.allow_toggle() {
tcx.dcx().emit_err(errors::ForbiddenTargetFeatureAttr {
span: item.span(),
feature,
reason,
});
} else if let Some(nightly_feature) = stability.requires_nightly()
&& !rust_features.enabled(nightly_feature)
{
feature_err(
&tcx.sess,
nightly_feature,
item.span(),
format!("the target feature `{feature}` is currently unstable"),
)
.emit();
}
Some(Symbol::intern(feature))
}));
Expand Down Expand Up @@ -156,18 +148,19 @@ 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))
.map(|(a, b)| (a.to_string(), b.compute_toggleability(target)))
.collect()
} else {
tcx.sess
.target
.rust_target_features()
.iter()
.map(|&(a, b, _)| (a.to_string(), b))
.map(|&(a, b, _)| (a.to_string(), b.compute_toggleability(target)))
.collect()
}
},
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_codegen_ssa/src/traits/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,9 @@ pub trait CodegenBackend {

fn print(&self, _req: &PrintRequest, _out: &mut String, _sess: &Session) {}

fn target_features(&self, _sess: &Session, _allow_unstable: bool) -> Vec<Symbol> {
/// Returns the features that should be set in `cfg(target_features)`.
/// RUSTC_SPECIFIC_FEATURES should be skipped here, those are handled outside codegen.
fn target_features_cfg(&self, _sess: &Session, _allow_unstable: bool) -> Vec<Symbol> {
vec![]
}

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_feature/src/unstable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,7 @@ declare_features! (
(unstable, sse4a_target_feature, "1.27.0", Some(44839)),
(unstable, tbm_target_feature, "1.27.0", Some(44839)),
(unstable, wasm_target_feature, "1.30.0", Some(44839)),
(unstable, x87_target_feature, "CURRENT_RUSTC_VERSION", Some(44839)),
// !!!! !!!! !!!! !!!! !!!! !!!! !!!! !!!! !!!! !!!! !!!!
// Features are listed in alphabetical order. Tidy will fail if you don't keep it this way.
// !!!! !!!! !!!! !!!! !!!! !!!! !!!! !!!! !!!! !!!! !!!!
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_interface/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,10 @@ pub type MakeBackendFn = fn() -> Box<dyn CodegenBackend>;
pub fn add_configuration(cfg: &mut Cfg, sess: &mut Session, codegen_backend: &dyn CodegenBackend) {
let tf = sym::target_feature;

let unstable_target_features = codegen_backend.target_features(sess, true);
let unstable_target_features = codegen_backend.target_features_cfg(sess, true);
sess.unstable_target_features.extend(unstable_target_features.iter().cloned());

let target_features = codegen_backend.target_features(sess, false);
let target_features = codegen_backend.target_features_cfg(sess, false);
sess.target_features.extend(target_features.iter().cloned());

cfg.extend(target_features.into_iter().map(|feat| (tf, Some(feat))));
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2233,7 +2233,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::Stability> {
query rust_target_features(_: CrateNum) -> &'tcx UnordMap<String, rustc_target::target_features::StabilityComputed> {
arena_cache
eval_always
desc { "looking up Rust target features" }
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_session/src/config/cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ impl CheckCfg {

ins!(sym::target_feature, empty_values).extend(
rustc_target::target_features::all_rust_features()
.filter(|(_, s)| s.is_supported())
.filter(|(_, s)| s.in_cfg())
.map(|(f, _s)| f)
.chain(rustc_target::target_features::RUSTC_SPECIFIC_FEATURES.iter().cloned())
.map(Symbol::intern),
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2201,6 +2201,7 @@ symbols! {
writeln_macro,
x86_amx_intrinsics,
x87_reg,
x87_target_feature,
xer,
xmm_reg,
xop_target_feature,
Expand Down
12 changes: 12 additions & 0 deletions compiler/rustc_target/src/spec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2603,6 +2603,18 @@ impl TargetOptions {
.collect();
}
}

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
}
})
}
}

impl Default for TargetOptions {
Expand Down
Loading

0 comments on commit 327c7ee

Please sign in to comment.