Skip to content

Commit 3e52956

Browse files
RalfJungRalfJung
committed
show warning when required target feature is missing, rather than implicitly enabling it
1 parent 9d53e37 commit 3e52956

16 files changed

+85
-120
lines changed

‎compiler/rustc_codegen_gcc/src/gcc_util.rs‎

Lines changed: 1 addition & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,8 @@
1-
use std::iter::FromIterator;
2-
31
#[cfg(feature = "master")]
42
use gccjit::Context;
53
use rustc_codegen_ssa::codegen_attrs::check_tied_features;
64
use rustc_codegen_ssa::errors::TargetFeatureDisableOrEnable;
7-
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
5+
use rustc_data_structures::fx::FxHashMap;
86
use rustc_data_structures::unord::UnordSet;
97
use rustc_session::Session;
108
use rustc_target::target_features::RUSTC_SPECIFIC_FEATURES;
@@ -45,12 +43,6 @@ pub(crate) fn global_gcc_features(sess: &Session, diagnostics: bool) -> Vec<Stri
4543
let known_features = sess.target.rust_target_features();
4644
let mut featsmap = FxHashMap::default();
4745

48-
// Ensure that all ABI-required features are enabled, and the ABI-forbidden ones
49-
// are disabled.
50-
let abi_feature_constraints = sess.target.abi_required_features();
51-
let abi_incompatible_set =
52-
FxHashSet::from_iter(abi_feature_constraints.incompatible.iter().copied());
53-
5446
// Compute implied features
5547
let mut all_rust_features = vec![];
5648
for feature in sess.opts.cg.target_feature.split(',') {
@@ -119,51 +111,11 @@ pub(crate) fn global_gcc_features(sess: &Session, diagnostics: bool) -> Vec<Stri
119111
}
120112
}
121113

122-
// Ensure that the features we enable/disable are compatible with the ABI.
123-
if enable {
124-
if abi_incompatible_set.contains(feature) {
125-
sess.dcx().emit_warn(ForbiddenCTargetFeature {
126-
feature,
127-
enabled: "enabled",
128-
reason: "this feature is incompatible with the target ABI",
129-
});
130-
}
131-
} else {
132-
// FIXME: we have to request implied features here since
133-
// negative features do not handle implied features above.
134-
for &required in abi_feature_constraints.required.iter() {
135-
let implied = sess.target.implied_target_features(std::iter::once(required));
136-
if implied.contains(feature) {
137-
sess.dcx().emit_warn(ForbiddenCTargetFeature {
138-
feature,
139-
enabled: "disabled",
140-
reason: "this feature is required by the target ABI",
141-
});
142-
}
143-
}
144-
}
145-
146114
// FIXME(nagisa): figure out how to not allocate a full hashset here.
147115
featsmap.insert(feature, enable);
148116
}
149117
}
150118

151-
// To be sure the ABI-relevant features are all in the right state, we explicitly
152-
// (un)set them here. This means if the target spec sets those features wrong,
153-
// we will silently correct them rather than silently producing wrong code.
154-
// (The target sanity check tries to catch this, but we can't know which features are
155-
// enabled in GCC by default so we can't be fully sure about that check.)
156-
// We add these at the beginning of the list so that `-Ctarget-features` can
157-
// still override it... that's unsound, but more compatible with past behavior.
158-
all_rust_features.splice(
159-
0..0,
160-
abi_feature_constraints
161-
.required
162-
.iter()
163-
.map(|&f| (true, f))
164-
.chain(abi_feature_constraints.incompatible.iter().map(|&f| (false, f))),
165-
);
166-
167119
// Translate this into GCC features.
168120
let feats = all_rust_features
169121
.iter()

‎compiler/rustc_codegen_gcc/src/lib.rs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -492,9 +492,10 @@ fn target_features_cfg(
492492
sess.target
493493
.rust_target_features()
494494
.iter()
495-
.filter(|(_, gate, _)| gate.in_cfg())
496495
.filter_map(|(feature, gate, _)| {
497-
if sess.is_nightly_build() || allow_unstable || gate.requires_nightly().is_none() {
496+
if allow_unstable
497+
|| (gate.in_cfg() && (sess.is_nightly_build() || gate.requires_nightly().is_none()))
498+
{
498499
Some(*feature)
499500
} else {
500501
None

‎compiler/rustc_codegen_llvm/src/llvm_util.rs‎

Lines changed: 6 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,6 @@ pub fn target_features_cfg(sess: &Session, allow_unstable: bool) -> Vec<Symbol>
319319
sess.target
320320
.rust_target_features()
321321
.iter()
322-
.filter(|(_, gate, _)| gate.in_cfg())
323322
.filter(|(feature, _, _)| {
324323
// skip checking special features, as LLVM may not understand them
325324
if RUSTC_SPECIAL_FEATURES.contains(feature) {
@@ -388,9 +387,13 @@ pub fn target_features_cfg(sess: &Session, allow_unstable: bool) -> Vec<Symbol>
388387
sess.target
389388
.rust_target_features()
390389
.iter()
391-
.filter(|(_, gate, _)| gate.in_cfg())
392390
.filter_map(|(feature, gate, _)| {
393-
if sess.is_nightly_build() || allow_unstable || gate.requires_nightly().is_none() {
391+
// The `allow_unstable` set is used by rustc internally to determined which target
392+
// features are truly available, so we want to return even perma-unstable "forbidden"
393+
// features.
394+
if allow_unstable
395+
|| (gate.in_cfg() && (sess.is_nightly_build() || gate.requires_nightly().is_none()))
396+
{
394397
Some(*feature)
395398
} else {
396399
None
@@ -670,12 +673,6 @@ pub(crate) fn global_llvm_features(
670673
// Will only be filled when `diagnostics` is set!
671674
let mut featsmap = FxHashMap::default();
672675

673-
// Ensure that all ABI-required features are enabled, and the ABI-forbidden ones
674-
// are disabled.
675-
let abi_feature_constraints = sess.target.abi_required_features();
676-
let abi_incompatible_set =
677-
FxHashSet::from_iter(abi_feature_constraints.incompatible.iter().copied());
678-
679676
// Compute implied features
680677
let mut all_rust_features = vec![];
681678
for feature in sess.opts.cg.target_feature.split(',') {
@@ -746,52 +743,11 @@ pub(crate) fn global_llvm_features(
746743
}
747744
}
748745

749-
// Ensure that the features we enable/disable are compatible with the ABI.
750-
if enable {
751-
if abi_incompatible_set.contains(feature) {
752-
sess.dcx().emit_warn(ForbiddenCTargetFeature {
753-
feature,
754-
enabled: "enabled",
755-
reason: "this feature is incompatible with the target ABI",
756-
});
757-
}
758-
} else {
759-
// FIXME: we have to request implied features here since
760-
// negative features do not handle implied features above.
761-
for &required in abi_feature_constraints.required.iter() {
762-
let implied =
763-
sess.target.implied_target_features(std::iter::once(required));
764-
if implied.contains(feature) {
765-
sess.dcx().emit_warn(ForbiddenCTargetFeature {
766-
feature,
767-
enabled: "disabled",
768-
reason: "this feature is required by the target ABI",
769-
});
770-
}
771-
}
772-
}
773-
774746
// FIXME(nagisa): figure out how to not allocate a full hashset here.
775747
featsmap.insert(feature, enable);
776748
}
777749
}
778750

779-
// To be sure the ABI-relevant features are all in the right state, we explicitly
780-
// (un)set them here. This means if the target spec sets those features wrong,
781-
// we will silently correct them rather than silently producing wrong code.
782-
// (The target sanity check tries to catch this, but we can't know which features are
783-
// enabled in LLVM by default so we can't be fully sure about that check.)
784-
// We add these at the beginning of the list so that `-Ctarget-features` can
785-
// still override it... that's unsound, but more compatible with past behavior.
786-
all_rust_features.splice(
787-
0..0,
788-
abi_feature_constraints
789-
.required
790-
.iter()
791-
.map(|&f| (true, f))
792-
.chain(abi_feature_constraints.incompatible.iter().map(|&f| (false, f))),
793-
);
794-
795751
// Translate this into LLVM features.
796752
let feats = all_rust_features
797753
.iter()

‎compiler/rustc_interface/messages.ftl‎

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

‎compiler/rustc_interface/src/errors.rs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,3 +103,12 @@ pub struct IgnoringOutDir;
103103
#[derive(Diagnostic)]
104104
#[diag(interface_multiple_output_types_to_stdout)]
105105
pub struct MultipleOutputTypesToStdout;
106+
107+
#[derive(Diagnostic)]
108+
#[diag(interface_abi_required_feature)]
109+
#[note]
110+
#[note(interface_abi_required_feature_issue)]
111+
pub(crate) struct AbiRequiredTargetFeature<'a> {
112+
pub feature: &'a str,
113+
pub enabled: &'a str,
114+
}

‎compiler/rustc_interface/src/interface.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -484,6 +484,8 @@ pub fn run_compiler<R: Send>(config: Config, f: impl FnOnce(&Compiler) -> R + Se
484484
}
485485
sess.lint_store = Some(Lrc::new(lint_store));
486486

487+
util::check_abi_required_features(&sess);
488+
487489
let compiler = Compiler {
488490
sess,
489491
codegen_backend,

‎compiler/rustc_interface/src/util.rs‎

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,21 +18,25 @@ use rustc_session::{EarlyDiagCtxt, Session, filesearch};
1818
use rustc_span::edit_distance::find_best_match_for_name;
1919
use rustc_span::edition::Edition;
2020
use rustc_span::source_map::SourceMapInputs;
21-
use rustc_span::sym;
21+
use rustc_span::{Symbol, sym};
2222
use rustc_target::spec::Target;
2323
use tracing::info;
2424

2525
use crate::errors;
2626

2727
/// Function pointer type that constructs a new CodegenBackend.
28-
pub type MakeBackendFn = fn() -> Box<dyn CodegenBackend>;
28+
type MakeBackendFn = fn() -> Box<dyn CodegenBackend>;
2929

3030
/// Adds `target_feature = "..."` cfgs for a variety of platform
3131
/// specific features (SSE, NEON etc.).
3232
///
3333
/// This is performed by checking whether a set of permitted features
3434
/// is available on the target machine, by querying the codegen backend.
35-
pub fn add_configuration(cfg: &mut Cfg, sess: &mut Session, codegen_backend: &dyn CodegenBackend) {
35+
pub(crate) fn add_configuration(
36+
cfg: &mut Cfg,
37+
sess: &mut Session,
38+
codegen_backend: &dyn CodegenBackend,
39+
) {
3640
let tf = sym::target_feature;
3741

3842
let unstable_target_features = codegen_backend.target_features_cfg(sess, true);
@@ -48,6 +52,25 @@ pub fn add_configuration(cfg: &mut Cfg, sess: &mut Session, codegen_backend: &dy
4852
}
4953
}
5054

55+
/// Ensures that all target features required by the ABI are present.
56+
/// Must be called after `unstable_target_features` has been populated!
57+
pub(crate) fn check_abi_required_features(sess: &Session) {
58+
let abi_feature_constraints = sess.target.abi_required_features();
59+
// We check this against `unstable_target_features` as that is conveniently already
60+
// back-translated to rustc feature names.
61+
62+
for feature in abi_feature_constraints.required {
63+
if !sess.unstable_target_features.contains(&Symbol::intern(feature)) {
64+
sess.dcx().emit_warn(errors::AbiRequiredTargetFeature { feature, enabled: "enabled" });
65+
}
66+
}
67+
for feature in abi_feature_constraints.incompatible {
68+
if sess.unstable_target_features.contains(&Symbol::intern(feature)) {
69+
sess.dcx().emit_warn(errors::AbiRequiredTargetFeature { feature, enabled: "disabled" });
70+
}
71+
}
72+
}
73+
5174
pub static STACK_SIZE: OnceLock<usize> = OnceLock::new();
5275
pub const DEFAULT_STACK_SIZE: usize = 8 * 1024 * 1024;
5376

‎tests/codegen/target-feature-overrides.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,8 @@ pub unsafe fn banana() -> u32 {
3939
}
4040

4141
// CHECK: attributes [[APPLEATTRS]]
42-
// COMPAT-SAME: "target-features"="+x87,+sse2,+avx,+avx2,{{.*}}"
43-
// INCOMPAT-SAME: "target-features"="+x87,+sse2,-avx2,-avx,+avx,{{.*}}"
42+
// COMPAT-SAME: "target-features"="+avx,+avx2,{{.*}}"
43+
// INCOMPAT-SAME: "target-features"="-avx2,-avx,+avx,{{.*}}"
4444
// CHECK: attributes [[BANANAATTRS]]
45-
// COMPAT-SAME: "target-features"="+x87,+sse2,+avx,+avx2,{{.*}}"
46-
// INCOMPAT-SAME: "target-features"="+x87,+sse2,-avx2,-avx"
45+
// COMPAT-SAME: "target-features"="+avx,+avx2,{{.*}}"
46+
// INCOMPAT-SAME: "target-features"="-avx2,-avx"

‎tests/codegen/tied-features-strength.rs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,10 @@
1111
// ENABLE_SVE: attributes #0 = { {{.*}} "target-features"="{{((\+outline-atomics,?)|(\+v8a,?)|(\+fpmr,?)?|(\+sve,?)|(\+neon,?)|(\+fp-armv8,?))*}}" }
1212

1313
//@ [DISABLE_SVE] compile-flags: -C target-feature=-sve -Copt-level=0
14-
// DISABLE_SVE: attributes #0 = { {{.*}} "target-features"="{{((\+outline-atomics,?)|(\+v8a,?)|(\+fpmr,?)?|(-sve,?)|(\+neon,?)|(\+fp-armv8,?))*}}" }
14+
// DISABLE_SVE: attributes #0 = { {{.*}} "target-features"="{{((\+outline-atomics,?)|(\+v8a,?)|(\+fpmr,?)?|(-sve,?)|(\+neon,?))*}}" }
1515

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

2019
//@ [ENABLE_NEON] compile-flags: -C target-feature=+neon -Copt-level=0
2120
// ENABLE_NEON: attributes #0 = { {{.*}} "target-features"="{{((\+outline-atomics,?)|(\+v8a,?)|(\+fpmr,?)?|(\+fp-armv8,?)|(\+neon,?))*}}" }

‎tests/ui-fulldeps/codegen-backend/hotplug.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@
66
//@ normalize-stdout: "libthe_backend.dylib" -> "libthe_backend.so"
77
//@ normalize-stdout: "the_backend.dll" -> "libthe_backend.so"
88

9+
// Pick a target that requires no target features, so that no warning is shown
10+
// about missing target features.
11+
//@ compile-flags: --target arm-unknown-linux-gnueabi
912
//@ revisions: normal dep bindep
1013
//@ compile-flags: --crate-type=lib
1114
//@ [normal] compile-flags: --emit=link=-

0 commit comments

Comments
 (0)