ABI-required target features: warn when they are missing in base CPU (rather than silently enabling them)

This commit is contained in:
Ralf Jung 2025-01-27 18:30:17 +01:00
parent f753850659
commit 93ee180cfa
15 changed files with 80 additions and 127 deletions

View file

@ -1,10 +1,8 @@
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, FxHashSet};
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::unord::UnordSet;
use rustc_session::Session;
use rustc_target::target_features::RUSTC_SPECIFIC_FEATURES;
@ -45,12 +43,6 @@ pub(crate) fn global_gcc_features(sess: &Session, diagnostics: bool) -> Vec<Stri
let known_features = sess.target.rust_target_features();
let mut featsmap = FxHashMap::default();
// 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(',') {
@ -117,51 +109,11 @@ pub(crate) fn global_gcc_features(sess: &Session, diagnostics: bool) -> Vec<Stri
}
}
// Ensure that the features we enable/disable are compatible with the ABI.
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 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().flat_map(|&(enable, feature)| {

View file

@ -493,9 +493,10 @@ fn target_features_cfg(
sess.target
.rust_target_features()
.iter()
.filter(|&&(_, gate, _)| gate.in_cfg())
.filter_map(|&(feature, gate, _)| {
if sess.is_nightly_build() || allow_unstable || gate.requires_nightly().is_none() {
if allow_unstable
|| (gate.in_cfg() && (sess.is_nightly_build() || gate.requires_nightly().is_none()))
{
Some(feature)
} else {
None

View file

@ -319,7 +319,6 @@ pub fn target_features_cfg(sess: &Session, allow_unstable: bool) -> Vec<Symbol>
sess.target
.rust_target_features()
.iter()
.filter(|(_, gate, _)| gate.in_cfg())
.filter(|(feature, _, _)| {
// skip checking special features, as LLVM may not understand them
if RUSTC_SPECIAL_FEATURES.contains(feature) {
@ -388,9 +387,13 @@ pub fn target_features_cfg(sess: &Session, allow_unstable: bool) -> Vec<Symbol>
sess.target
.rust_target_features()
.iter()
.filter(|(_, gate, _)| gate.in_cfg())
.filter_map(|(feature, gate, _)| {
if sess.is_nightly_build() || allow_unstable || gate.requires_nightly().is_none() {
// The `allow_unstable` set is used by rustc internally to determined which target
// features are truly available, so we want to return even perma-unstable "forbidden"
// features.
if allow_unstable
|| (gate.in_cfg() && (sess.is_nightly_build() || gate.requires_nightly().is_none()))
{
Some(*feature)
} else {
None
@ -670,12 +673,6 @@ pub(crate) fn global_llvm_features(
// Will only be filled when `diagnostics` is set!
let mut featsmap = FxHashMap::default();
// 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(',') {
@ -746,52 +743,11 @@ pub(crate) fn global_llvm_features(
}
}
// Ensure that the features we enable/disable are compatible with the ABI.
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()

View file

@ -1,3 +1,8 @@
interface_abi_required_feature =
target feature `{$feature}` must be {$enabled} to ensure that the ABI of the current target can be implemented correctly
.note = this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
interface_abi_required_feature_issue = for more information, see issue #116344 <https://github.com/rust-lang/rust/issues/116344>
interface_cant_emit_mir =
could not emit MIR: {$error}

View file

@ -103,3 +103,12 @@ pub struct IgnoringOutDir;
#[derive(Diagnostic)]
#[diag(interface_multiple_output_types_to_stdout)]
pub struct MultipleOutputTypesToStdout;
#[derive(Diagnostic)]
#[diag(interface_abi_required_feature)]
#[note]
#[note(interface_abi_required_feature_issue)]
pub(crate) struct AbiRequiredTargetFeature<'a> {
pub feature: &'a str,
pub enabled: &'a str,
}

View file

@ -492,6 +492,8 @@ pub fn run_compiler<R: Send>(config: Config, f: impl FnOnce(&Compiler) -> R + Se
}
sess.lint_store = Some(Lrc::new(lint_store));
util::check_abi_required_features(&sess);
let compiler = Compiler {
sess,
codegen_backend,

View file

@ -18,21 +18,25 @@ use rustc_session::{EarlyDiagCtxt, Session, filesearch};
use rustc_span::edit_distance::find_best_match_for_name;
use rustc_span::edition::Edition;
use rustc_span::source_map::SourceMapInputs;
use rustc_span::sym;
use rustc_span::{Symbol, sym};
use rustc_target::spec::Target;
use tracing::info;
use crate::errors;
/// Function pointer type that constructs a new CodegenBackend.
pub type MakeBackendFn = fn() -> Box<dyn CodegenBackend>;
type MakeBackendFn = fn() -> Box<dyn CodegenBackend>;
/// Adds `target_feature = "..."` cfgs for a variety of platform
/// specific features (SSE, NEON etc.).
///
/// This is performed by checking whether a set of permitted features
/// is available on the target machine, by querying the codegen backend.
pub fn add_configuration(cfg: &mut Cfg, sess: &mut Session, codegen_backend: &dyn CodegenBackend) {
pub(crate) 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_cfg(sess, true);
@ -48,6 +52,34 @@ pub fn add_configuration(cfg: &mut Cfg, sess: &mut Session, codegen_backend: &dy
}
}
/// Ensures that all target features required by the ABI are present.
/// Must be called after `unstable_target_features` has been populated!
pub(crate) fn check_abi_required_features(sess: &Session) {
let abi_feature_constraints = sess.target.abi_required_features();
// We check this against `unstable_target_features` as that is conveniently already
// back-translated to rustc feature names, taking into account `-Ctarget-cpu` and `-Ctarget-feature`.
// Just double-check that the features we care about are actually on our list.
for feature in
abi_feature_constraints.required.iter().chain(abi_feature_constraints.incompatible.iter())
{
assert!(
sess.target.rust_target_features().iter().any(|(name, ..)| feature == name),
"target feature {feature} is required/incompatible for the current ABI but not a recognized feature for this target"
);
}
for feature in abi_feature_constraints.required {
if !sess.unstable_target_features.contains(&Symbol::intern(feature)) {
sess.dcx().emit_warn(errors::AbiRequiredTargetFeature { feature, enabled: "enabled" });
}
}
for feature in abi_feature_constraints.incompatible {
if sess.unstable_target_features.contains(&Symbol::intern(feature)) {
sess.dcx().emit_warn(errors::AbiRequiredTargetFeature { feature, enabled: "disabled" });
}
}
}
pub static STACK_SIZE: OnceLock<usize> = OnceLock::new();
pub const DEFAULT_STACK_SIZE: usize = 8 * 1024 * 1024;

View file

@ -1,5 +1,5 @@
// We're testing x86 target specific features
//@only-target: x86_64 i686
// We're testing x86-32 target specific features. SSE always exists on x86-64.
//@only-target: i686
//@compile-flags: -C target-feature=-sse2
#[cfg(target_arch = "x86")]

View file

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

View file

@ -11,11 +11,10 @@
// 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,?)|(\+fp-armv8,?))*}}" }
// DISABLE_SVE: attributes #0 = { {{.*}} "target-features"="{{((\+outline-atomics,?)|(\+v8a,?)|(\+fpmr,?)?|(-sve,?)|(\+neon,?))*}}" }
//@ [DISABLE_NEON] compile-flags: -C target-feature=-neon -Copt-level=0
// `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)?}}" }
// DISABLE_NEON: attributes #0 = { {{.*}} "target-features"="{{((\+outline-atomics,?)|(\+v8a,?)|(\+fpmr,?)?|(-fp-armv8,?)|(-neon,?))*}}" }
//@ [ENABLE_NEON] compile-flags: -C target-feature=+neon -Copt-level=0
// ENABLE_NEON: attributes #0 = { {{.*}} "target-features"="{{((\+outline-atomics,?)|(\+v8a,?)|(\+fpmr,?)?|(\+fp-armv8,?)|(\+neon,?))*}}" }

View file

@ -6,6 +6,10 @@
//@ normalize-stdout: "libthe_backend.dylib" -> "libthe_backend.so"
//@ normalize-stdout: "the_backend.dll" -> "libthe_backend.so"
// Pick a target that requires no target features, so that no warning is shown
// about missing target features.
//@ compile-flags: --target arm-unknown-linux-gnueabi
//@ needs-llvm-components: arm
//@ revisions: normal dep bindep
//@ compile-flags: --crate-type=lib
//@ [normal] compile-flags: --emit=link=-

View file

@ -1,7 +0,0 @@
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>
warning: 1 warning emitted

View file

@ -1,4 +1,4 @@
warning: target feature `sse` cannot be disabled with `-Ctarget-feature`: this feature is required by the target ABI
warning: target feature `sse2` must be enabled to ensure that the ABI of the current target can be implemented correctly
|
= 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>

View file

@ -1,4 +1,4 @@
warning: target feature `neon` cannot be disabled with `-Ctarget-feature`: this feature is required by the target ABI
warning: target feature `neon` must be enabled to ensure that the ABI of the current target can be implemented correctly
|
= 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>

View file

@ -1,11 +1,11 @@
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
warning: target feature `x87` must be enabled to ensure that the ABI of the current target can be implemented correctly
|
= 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: unstable feature specified for `-Ctarget-feature`: `x87`
|
= note: this feature is not stably supported; its behavior can change in the future
warning: 2 warnings emitted