Skip to content

Commit 2b928b4

Browse files
RalfJungRalfJung
committed
explicitly model that certain ABIs require/forbid certain target features
1 parent 97a56fb commit 2b928b4

18 files changed

+300
-269
lines changed

‎compiler/rustc_codegen_llvm/messages.ftl

+1-3
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ codegen_llvm_dynamic_linking_with_lto =
88
codegen_llvm_fixed_x18_invalid_arch = the `-Zfixed-x18` flag is not supported on the `{$arch}` architecture
99
1010
codegen_llvm_forbidden_ctarget_feature =
11-
target feature `{$feature}` cannot be toggled with `-Ctarget-feature`: {$reason}
11+
target feature `{$feature}` cannot be {$enabled} with `-Ctarget-feature`: {$reason}
1212
.note = this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
1313
codegen_llvm_forbidden_ctarget_feature_issue = for more information, see issue #116344 <https://github.com/rust-lang/rust/issues/116344>
1414
@@ -22,8 +22,6 @@ codegen_llvm_invalid_minimum_alignment_not_power_of_two =
2222
codegen_llvm_invalid_minimum_alignment_too_large =
2323
invalid minimum global alignment: {$align} is too large
2424
25-
codegen_llvm_invalid_target_feature_prefix = target feature `{$feature}` must begin with a `+` or `-`"
26-
2725
codegen_llvm_load_bitcode = failed to load bitcode of module "{$name}"
2826
codegen_llvm_load_bitcode_with_llvm_err = failed to load bitcode of module "{$name}": {$llvm_err}
2927

‎compiler/rustc_codegen_llvm/src/errors.rs

+1-6
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ pub(crate) struct UnstableCTargetFeature<'a> {
3737
#[note(codegen_llvm_forbidden_ctarget_feature_issue)]
3838
pub(crate) struct ForbiddenCTargetFeature<'a> {
3939
pub feature: &'a str,
40+
pub enabled: &'a str,
4041
pub reason: &'a str,
4142
}
4243

@@ -205,12 +206,6 @@ pub(crate) struct MismatchedDataLayout<'a> {
205206
pub llvm_layout: &'a str,
206207
}
207208

208-
#[derive(Diagnostic)]
209-
#[diag(codegen_llvm_invalid_target_feature_prefix)]
210-
pub(crate) struct InvalidTargetFeaturePrefix<'a> {
211-
pub feature: &'a str,
212-
}
213-
214209
#[derive(Diagnostic)]
215210
#[diag(codegen_llvm_fixed_x18_invalid_arch)]
216211
pub(crate) struct FixedX18InvalidArch<'a> {

‎compiler/rustc_codegen_llvm/src/llvm_util.rs

+127-93
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,8 @@ use rustc_target::target_features::{RUSTC_SPECIAL_FEATURES, RUSTC_SPECIFIC_FEATU
2121

2222
use crate::back::write::create_informational_target_machine;
2323
use crate::errors::{
24-
FixedX18InvalidArch, ForbiddenCTargetFeature, InvalidTargetFeaturePrefix, PossibleFeature,
25-
UnknownCTargetFeature, UnknownCTargetFeaturePrefix, UnstableCTargetFeature,
24+
FixedX18InvalidArch, ForbiddenCTargetFeature, PossibleFeature, UnknownCTargetFeature,
25+
UnknownCTargetFeaturePrefix, UnstableCTargetFeature,
2626
};
2727
use crate::llvm;
2828

@@ -348,15 +348,28 @@ pub fn target_features_cfg(sess: &Session, allow_unstable: bool) -> Vec<Symbol>
348348
{
349349
if enabled {
350350
// Also add all transitively implied features.
351-
features.extend(sess.target.implied_target_features(std::iter::once(feature)));
351+
352+
// We don't care about the order in `features` since the only thing we use it for is the
353+
// `features.contains` below.
354+
#[allow(rustc::potential_query_instability)]
355+
features.extend(
356+
sess.target
357+
.implied_target_features(std::iter::once(feature.as_str()))
358+
.iter()
359+
.map(|s| Symbol::intern(s)),
360+
);
352361
} else {
353362
// Remove transitively reverse-implied features.
354363

355364
// We don't care about the order in `features` since the only thing we use it for is the
356365
// `features.contains` below.
357366
#[allow(rustc::potential_query_instability)]
358367
features.retain(|f| {
359-
if sess.target.implied_target_features(std::iter::once(*f)).contains(&feature) {
368+
if sess
369+
.target
370+
.implied_target_features(std::iter::once(f.as_str()))
371+
.contains(&feature.as_str())
372+
{
360373
// If `f` if implies `feature`, then `!feature` implies `!f`, so we have to
361374
// remove `f`. (This is the standard logical contraposition principle.)
362375
false
@@ -638,7 +651,7 @@ pub(crate) fn global_llvm_features(
638651
sess.target
639652
.features
640653
.split(',')
641-
.filter(|v| !v.is_empty() && backend_feature_name(sess, v).is_some())
654+
.filter(|v| !v.is_empty())
642655
// Drop +v8plus feature introduced in LLVM 20.
643656
.filter(|v| *v != "+v8plus" || get_version() >= (20, 0, 0))
644657
.map(String::from),
@@ -651,89 +664,126 @@ pub(crate) fn global_llvm_features(
651664
// -Ctarget-features
652665
if !only_base_features {
653666
let known_features = sess.target.rust_target_features();
667+
// Will only be filled when `diagnostics` is set!
654668
let mut featsmap = FxHashMap::default();
655669

656-
// insert implied features
670+
// Ensure that all ABI-required features are enabled, and the ABI-forbidden ones
671+
// are disabled.
672+
let (abi_enable, abi_disable) = sess.target.abi_required_features();
673+
let abi_enable_set = FxHashSet::from_iter(abi_enable.iter().copied());
674+
let abi_disable_set = FxHashSet::from_iter(abi_disable.iter().copied());
675+
676+
// Compute implied features
657677
let mut all_rust_features = vec![];
658678
for feature in sess.opts.cg.target_feature.split(',') {
659-
match feature.strip_prefix('+') {
660-
Some(feature) => all_rust_features.extend(
661-
UnordSet::from(
662-
sess.target
663-
.implied_target_features(std::iter::once(Symbol::intern(feature))),
664-
)
665-
.to_sorted_stable_ord()
666-
.iter()
667-
.map(|s| format!("+{}", s.as_str())),
668-
),
669-
_ => all_rust_features.push(feature.to_string()),
679+
if let Some(feature) = feature.strip_prefix('+') {
680+
all_rust_features.extend(
681+
UnordSet::from(sess.target.implied_target_features(std::iter::once(feature)))
682+
.to_sorted_stable_ord()
683+
.iter()
684+
.map(|&&s| (true, s)),
685+
)
686+
} else if let Some(feature) = feature.strip_prefix('-') {
687+
// FIXME: Why do we not remove implied features on "-" here?
688+
// We do the equivalent above in `target_features_cfg`.
689+
// See <https://github.com/rust-lang/rust/issues/134792>.
690+
all_rust_features.push((false, feature));
691+
} else if !feature.is_empty() {
692+
if diagnostics {
693+
sess.dcx().emit_warn(UnknownCTargetFeaturePrefix { feature });
694+
}
670695
}
671696
}
697+
// Remove features that are meant for rustc, not LLVM.
698+
all_rust_features.retain(|(_, feature)| {
699+
// Retain if it is not a rustc feature
700+
!RUSTC_SPECIFIC_FEATURES.contains(feature)
701+
});
672702

673-
let feats = all_rust_features
674-
.iter()
675-
.filter_map(|s| {
676-
let enable_disable = match s.chars().next() {
677-
None => return None,
678-
Some(c @ ('+' | '-')) => c,
679-
Some(_) => {
680-
if diagnostics {
681-
sess.dcx().emit_warn(UnknownCTargetFeaturePrefix { feature: s });
682-
}
683-
return None;
684-
}
685-
};
686-
687-
// Get the backend feature name, if any.
688-
// This excludes rustc-specific features, which do not get passed to LLVM.
689-
let feature = backend_feature_name(sess, s)?;
690-
// Warn against use of LLVM specific feature names and unstable features on the CLI.
691-
if diagnostics {
692-
let feature_state = known_features.iter().find(|&&(v, _, _)| v == feature);
693-
match feature_state {
694-
None => {
695-
let rust_feature =
696-
known_features.iter().find_map(|&(rust_feature, _, _)| {
697-
let llvm_features = to_llvm_features(sess, rust_feature)?;
698-
if llvm_features.contains(feature)
699-
&& !llvm_features.contains(rust_feature)
700-
{
701-
Some(rust_feature)
702-
} else {
703-
None
704-
}
705-
});
706-
let unknown_feature = if let Some(rust_feature) = rust_feature {
707-
UnknownCTargetFeature {
708-
feature,
709-
rust_feature: PossibleFeature::Some { rust_feature },
710-
}
711-
} else {
712-
UnknownCTargetFeature {
713-
feature,
714-
rust_feature: PossibleFeature::None,
703+
// Check feature validity.
704+
if diagnostics {
705+
for &(enable, feature) in &all_rust_features {
706+
let feature_state = known_features.iter().find(|&&(v, _, _)| v == feature);
707+
match feature_state {
708+
None => {
709+
let rust_feature =
710+
known_features.iter().find_map(|&(rust_feature, _, _)| {
711+
let llvm_features = to_llvm_features(sess, rust_feature)?;
712+
if llvm_features.contains(feature)
713+
&& !llvm_features.contains(rust_feature)
714+
{
715+
Some(rust_feature)
716+
} else {
717+
None
715718
}
716-
};
717-
sess.dcx().emit_warn(unknown_feature);
718-
}
719-
Some((_, stability, _)) => {
720-
if let Err(reason) =
721-
stability.toggle_allowed(&sess.target, enable_disable == '+')
722-
{
723-
sess.dcx().emit_warn(ForbiddenCTargetFeature { feature, reason });
724-
} else if stability.requires_nightly().is_some() {
725-
// An unstable feature. Warn about using it. It makes little sense
726-
// to hard-error here since we just warn about fully unknown
727-
// features above.
728-
sess.dcx().emit_warn(UnstableCTargetFeature { feature });
719+
});
720+
let unknown_feature = if let Some(rust_feature) = rust_feature {
721+
UnknownCTargetFeature {
722+
feature,
723+
rust_feature: PossibleFeature::Some { rust_feature },
729724
}
725+
} else {
726+
UnknownCTargetFeature { feature, rust_feature: PossibleFeature::None }
727+
};
728+
sess.dcx().emit_warn(unknown_feature);
729+
}
730+
Some((_, stability, _)) => {
731+
if let Err(reason) = stability.toggle_allowed(&sess.target, enable) {
732+
sess.dcx().emit_warn(ForbiddenCTargetFeature {
733+
feature,
734+
enabled: if enable { "enabled" } else { "disabled" },
735+
reason,
736+
});
737+
} else if stability.requires_nightly().is_some() {
738+
// An unstable feature. Warn about using it. It makes little sense
739+
// to hard-error here since we just warn about fully unknown
740+
// features above.
741+
sess.dcx().emit_warn(UnstableCTargetFeature { feature });
730742
}
731743
}
744+
}
732745

733-
// FIXME(nagisa): figure out how to not allocate a full hashset here.
734-
featsmap.insert(feature, enable_disable == '+');
746+
// Ensure that the features we enable/disable are compatible with the ABI.
747+
if enable {
748+
if abi_disable_set.contains(feature) {
749+
sess.dcx().emit_warn(ForbiddenCTargetFeature {
750+
feature,
751+
enabled: "enabled",
752+
reason: "this feature is incompatible with the target ABI",
753+
});
754+
}
755+
} else {
756+
if abi_enable_set.contains(feature) {
757+
sess.dcx().emit_warn(ForbiddenCTargetFeature {
758+
feature,
759+
enabled: "disabled",
760+
reason: "this feature is required by the target ABI",
761+
});
762+
}
735763
}
736764

765+
// FIXME(nagisa): figure out how to not allocate a full hashset here.
766+
featsmap.insert(feature, enable);
767+
}
768+
}
769+
770+
// To be sure the ABI-relevant features are all in the right state, we explicitly
771+
// (un)set them here. This means if the target spec sets those features wrong,
772+
// we will silently correct them rather than silently producing wrong code.
773+
// (The target sanity check tries to catch this, but we can't know which features are
774+
// enabled in LLVM by default so we can't be fully sure about that check.)
775+
for feature in abi_enable {
776+
all_rust_features.push((true, feature));
777+
}
778+
for feature in abi_disable {
779+
all_rust_features.push((false, feature));
780+
}
781+
782+
// Translate this into LLVM features.
783+
let feats = all_rust_features
784+
.iter()
785+
.filter_map(|&(enable, feature)| {
786+
let enable_disable = if enable { '+' } else { '-' };
737787
// We run through `to_llvm_features` when
738788
// passing requests down to LLVM. This means that all in-language
739789
// features also work on the command line instead of having two
@@ -746,9 +796,9 @@ pub(crate) fn global_llvm_features(
746796
enable_disable, llvm_feature.llvm_feature_name
747797
))
748798
.chain(llvm_feature.dependency.into_iter().filter_map(
749-
move |feat| match (enable_disable, feat) {
750-
('-' | '+', TargetFeatureFoldStrength::Both(f))
751-
| ('+', TargetFeatureFoldStrength::EnableOnly(f)) => {
799+
move |feat| match (enable, feat) {
800+
(_, TargetFeatureFoldStrength::Both(f))
801+
| (true, TargetFeatureFoldStrength::EnableOnly(f)) => {
752802
Some(format!("{enable_disable}{f}"))
753803
}
754804
_ => None,
@@ -780,22 +830,6 @@ pub(crate) fn global_llvm_features(
780830
features
781831
}
782832

783-
/// Returns a feature name for the given `+feature` or `-feature` string.
784-
///
785-
/// Only allows features that are backend specific (i.e. not [`RUSTC_SPECIFIC_FEATURES`].)
786-
fn backend_feature_name<'a>(sess: &Session, s: &'a str) -> Option<&'a str> {
787-
// features must start with a `+` or `-`.
788-
let feature = s
789-
.strip_prefix(&['+', '-'][..])
790-
.unwrap_or_else(|| sess.dcx().emit_fatal(InvalidTargetFeaturePrefix { feature: s }));
791-
// Rustc-specific feature requests like `+crt-static` or `-crt-static`
792-
// are not passed down to LLVM.
793-
if s.is_empty() || RUSTC_SPECIFIC_FEATURES.contains(&feature) {
794-
return None;
795-
}
796-
Some(feature)
797-
}
798-
799833
pub(crate) fn tune_cpu(sess: &Session) -> Option<&str> {
800834
let name = sess.opts.unstable_opts.tune_cpu.as_ref()?;
801835
Some(handle_native(name))

‎compiler/rustc_codegen_ssa/messages.ftl

+1-1
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ codegen_ssa_failed_to_write = failed to write {$path}: {$error}
6767
codegen_ssa_field_associated_value_expected = associated value expected for `{$name}`
6868
6969
codegen_ssa_forbidden_target_feature_attr =
70-
target feature `{$feature}` cannot be toggled with `#[target_feature]`: {$reason}
70+
target feature `{$feature}` cannot be enabled with `#[target_feature]`: {$reason}
7171
7272
codegen_ssa_ignoring_emit_path = ignoring emit path because multiple .{$extension} files were produced
7373

0 commit comments

Comments
 (0)