Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mark some target features as 'forbidden' so they cannot be (un)set with -Ctarget-feature #129884

Merged
merged 1 commit into from
Nov 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions compiler/rustc_codegen_gcc/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ codegen_gcc_invalid_minimum_alignment =
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`

codegen_gcc_unwinding_inline_asm =
GCC backend does not support unwinding from inline asm

Expand All @@ -24,11 +27,15 @@ codegen_gcc_lto_dylib = lto cannot be used for `dylib` crate type without `-Zdyl
codegen_gcc_lto_bitcode_from_rlib = failed to get bitcode from object file for LTO ({$gcc_err})

codegen_gcc_unknown_ctarget_feature =
unknown feature specified for `-Ctarget-feature`: `{$feature}`
.note = it is still passed through to the codegen backend
unknown and unstable feature specified for `-Ctarget-feature`: `{$feature}`
.note = it is still passed through to the codegen backend, but use of this feature might be unsound and the behavior of this feature can change in the future
.possible_feature = you might have meant: `{$rust_feature}`
.consider_filing_feature_request = consider filing a feature request

codegen_gcc_unstable_ctarget_feature =
unstable feature specified for `-Ctarget-feature`: `{$feature}`
.note = this feature is not stably supported; its behavior can change in the future

codegen_gcc_missing_features =
add the missing features in a `target_feature` attribute

Expand Down
13 changes: 13 additions & 0 deletions compiler/rustc_codegen_gcc/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,19 @@ pub(crate) struct UnknownCTargetFeature<'a> {
pub rust_feature: PossibleFeature<'a>,
}

#[derive(Diagnostic)]
#[diag(codegen_gcc_unstable_ctarget_feature)]
#[note]
pub(crate) struct UnstableCTargetFeature<'a> {
pub feature: &'a str,
}

#[derive(Diagnostic)]
#[diag(codegen_gcc_forbidden_ctarget_feature)]
pub(crate) struct ForbiddenCTargetFeature<'a> {
pub feature: &'a str,
}

#[derive(Subdiagnostic)]
pub(crate) enum PossibleFeature<'a> {
#[help(codegen_gcc_possible_feature)]
Expand Down
65 changes: 40 additions & 25 deletions compiler/rustc_codegen_gcc/src/gcc_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,13 @@ 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;
use rustc_target::target_features::{RUSTC_SPECIFIC_FEATURES, Stability};
use smallvec::{SmallVec, smallvec};

use crate::errors::{PossibleFeature, UnknownCTargetFeature, UnknownCTargetFeaturePrefix};
use crate::errors::{
ForbiddenCTargetFeature, PossibleFeature, UnknownCTargetFeature, UnknownCTargetFeaturePrefix,
UnstableCTargetFeature,
};

/// The list of GCC features computed from CLI flags (`-Ctarget-cpu`, `-Ctarget-feature`,
/// `--target` and similar).
Expand Down Expand Up @@ -43,7 +46,7 @@ pub(crate) fn global_gcc_features(sess: &Session, diagnostics: bool) -> Vec<Stri
);

// -Ctarget-features
let supported_features = sess.target.supported_target_features();
let known_features = sess.target.rust_target_features();
let mut featsmap = FxHashMap::default();
let feats = sess
.opts
Expand All @@ -62,37 +65,49 @@ pub(crate) fn global_gcc_features(sess: &Session, diagnostics: bool) -> Vec<Stri
}
};

// Get the backend feature name, if any.
// This excludes rustc-specific features, that do not get passed down to GCC.
let feature = backend_feature_name(s)?;
// Warn against use of GCC specific feature names on the CLI.
if diagnostics && !supported_features.iter().any(|&(v, _, _)| v == feature) {
let rust_feature = supported_features.iter().find_map(|&(rust_feature, _, _)| {
let gcc_features = to_gcc_features(sess, rust_feature);
if gcc_features.contains(&feature) && !gcc_features.contains(&rust_feature) {
Some(rust_feature)
} else {
None
if diagnostics {
let feature_state = known_features.iter().find(|&&(v, _, _)| v == feature);
match feature_state {
None => {
let rust_feature =
known_features.iter().find_map(|&(rust_feature, _, _)| {
let gcc_features = to_gcc_features(sess, rust_feature);
if gcc_features.contains(&feature)
&& !gcc_features.contains(&rust_feature)
{
Some(rust_feature)
} else {
None
}
});
let unknown_feature = if let Some(rust_feature) = rust_feature {
UnknownCTargetFeature {
feature,
rust_feature: PossibleFeature::Some { rust_feature },
}
} else {
UnknownCTargetFeature { feature, rust_feature: PossibleFeature::None }
};
sess.dcx().emit_warn(unknown_feature);
Comment on lines +72 to +95
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very preliminary remark: I know that you did not introduce it but I am Unexcited about seeing this kind of code duplication between cg_llvm and cg_gcc. Did you attempt to lift this into cg_ssa and fail?

To be clear, you're not required to do so, I'm just fishing for some preliminary information before I Actually Review this.

}
});
let unknown_feature = if let Some(rust_feature) = rust_feature {
UnknownCTargetFeature {
feature,
rust_feature: PossibleFeature::Some { rust_feature },
Some((_, Stability::Stable, _)) => {}
Some((_, Stability::Unstable(_), _)) => {
// An unstable feature. Warn about using it.
sess.dcx().emit_warn(UnstableCTargetFeature { feature });
}
} else {
UnknownCTargetFeature { feature, rust_feature: PossibleFeature::None }
};
sess.dcx().emit_warn(unknown_feature);
}
Some((_, Stability::Forbidden { .. }, _)) => {
sess.dcx().emit_err(ForbiddenCTargetFeature { feature });
}
}

if diagnostics {
// FIXME(nagisa): figure out how to not allocate a full hashset here.
featsmap.insert(feature, enable_disable == '+');
}

// rustc-specific features do not get passed down to GCC…
if RUSTC_SPECIFIC_FEATURES.contains(&feature) {
return None;
}
// ... otherwise though we run through `to_gcc_features` when
// passing requests down to GCC. This means that all in-language
// features also work on the command line instead of having two
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_codegen_gcc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -491,8 +491,9 @@ pub fn target_features(
) -> Vec<Symbol> {
// TODO(antoyo): use global_gcc_features.
sess.target
.supported_target_features()
.rust_target_features()
.iter()
.filter(|(_, gate, _)| gate.is_supported())
.filter_map(|&(feature, gate, _)| {
if sess.is_nightly_build() || allow_unstable || gate.is_stable() {
Some(feature)
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_codegen_llvm/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ codegen_llvm_dynamic_linking_with_lto =

codegen_llvm_fixed_x18_invalid_arch = the `-Zfixed-x18` flag is not supported on the `{$arch}` architecture

codegen_llvm_forbidden_ctarget_feature =
target feature `{$feature}` cannot be toggled with `-Ctarget-feature`: {$reason}
.note = this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
codegen_llvm_forbidden_ctarget_feature_issue = for more information, see issue #116344 <https://github.com/rust-lang/rust/issues/116344>

codegen_llvm_from_llvm_diag = {$message}

codegen_llvm_from_llvm_optimization_diag = {$filename}:{$line}:{$column} {$pass_name} ({$kind}): {$message}
Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_codegen_llvm/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,15 @@ pub(crate) struct UnstableCTargetFeature<'a> {
pub feature: &'a str,
}

#[derive(Diagnostic)]
#[diag(codegen_llvm_forbidden_ctarget_feature)]
#[note]
#[note(codegen_llvm_forbidden_ctarget_feature_issue)]
pub(crate) struct ForbiddenCTargetFeature<'a> {
pub feature: &'a str,
pub reason: &'a str,
}

#[derive(Subdiagnostic)]
pub(crate) enum PossibleFeature<'a> {
#[help(codegen_llvm_possible_feature)]
Expand Down
119 changes: 71 additions & 48 deletions compiler/rustc_codegen_llvm/src/llvm_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@ 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};
use rustc_target::target_features::{RUSTC_SPECIAL_FEATURES, RUSTC_SPECIFIC_FEATURES, Stability};

use crate::back::write::create_informational_target_machine;
use crate::errors::{
FixedX18InvalidArch, InvalidTargetFeaturePrefix, PossibleFeature, UnknownCTargetFeature,
UnknownCTargetFeaturePrefix, UnstableCTargetFeature,
FixedX18InvalidArch, ForbiddenCTargetFeature, InvalidTargetFeaturePrefix, PossibleFeature,
UnknownCTargetFeature, UnknownCTargetFeaturePrefix, UnstableCTargetFeature,
};
use crate::llvm;

Expand Down Expand Up @@ -280,19 +280,29 @@ pub(crate) fn to_llvm_features<'a>(sess: &Session, s: &'a str) -> Option<LLVMFea
}
}

/// Used to generate cfg variables and apply features
/// Must express features in the way Rust understands them
/// Used to generate cfg variables and apply features.
/// 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> {
let mut features = vec![];

// Add base features for the target
let mut features: FxHashSet<Symbol> = Default::default();

// Add base features for the target.
// We do *not* add the -Ctarget-features there, and instead duplicate the logic for that below.
// The reason is that if LLVM considers a feature implied but we do not, we don't want that to
// show up in `cfg`. That way, `cfg` is entirely under our control -- except for the handling of
// the target CPU, that is still expanded to target features (with all their implied features) by
// LLVM.
let target_machine = create_informational_target_machine(sess, true);
// Compute which of the known target features are enabled in the 'base' target machine.
// We only consider "supported" features; "forbidden" features are not reflected in `cfg` as of now.
features.extend(
sess.target
.supported_target_features()
.rust_target_features()
.iter()
.filter(|(_, gate, _)| gate.is_supported())
.filter(|(feature, _, _)| {
// skip checking special features, as LLVM may not understands them
// skip checking special features, as LLVM may not understand them
if RUSTC_SPECIAL_FEATURES.contains(feature) {
return true;
}
Expand Down Expand Up @@ -323,16 +333,22 @@ pub fn target_features(sess: &Session, allow_unstable: bool) -> Vec<Symbol> {
if enabled {
features.extend(sess.target.implied_target_features(std::iter::once(feature)));
} else {
// We don't care about the order in `features` since the only thing we use it for is the
// `features.contains` below.
#[allow(rustc::potential_query_instability)]
features.retain(|f| {
// Keep a feature if it does not imply `feature`. Or, equivalently,
// remove the reverse-dependencies of `feature`.
!sess.target.implied_target_features(std::iter::once(*f)).contains(&feature)
});
}
}

// Filter enabled features based on feature gates
sess.target
.supported_target_features()
.rust_target_features()
.iter()
.filter(|(_, gate, _)| gate.is_supported())
.filter_map(|&(feature, gate, _)| {
if sess.is_nightly_build() || allow_unstable || gate.is_stable() {
Some(feature)
Expand Down Expand Up @@ -392,9 +408,13 @@ fn print_target_features(out: &mut String, sess: &Session, tm: &llvm::TargetMach
let mut known_llvm_target_features = FxHashSet::<&'static str>::default();
let mut rustc_target_features = sess
.target
.supported_target_features()
.rust_target_features()
.iter()
.filter_map(|(feature, _gate, _implied)| {
.filter_map(|(feature, gate, _implied)| {
if !gate.is_supported() {
// Only list (experimentally) supported features.
return None;
}
// LLVM asserts that these are sorted. LLVM and Rust both use byte comparison for these
// strings.
let llvm_feature = to_llvm_features(sess, *feature)?.llvm_feature_name;
Expand Down Expand Up @@ -567,7 +587,7 @@ pub(crate) fn global_llvm_features(

// -Ctarget-features
if !only_base_features {
let supported_features = sess.target.supported_target_features();
let known_features = sess.target.rust_target_features();
let mut featsmap = FxHashMap::default();

// insert implied features
Expand Down Expand Up @@ -601,50 +621,53 @@ pub(crate) fn global_llvm_features(
}
};

// Get the backend feature name, if any.
// This excludes rustc-specific features, which do not get passed to LLVM.
let feature = backend_feature_name(sess, s)?;
// Warn against use of LLVM specific feature names and unstable features on the CLI.
if diagnostics {
let feature_state = supported_features.iter().find(|&&(v, _, _)| v == feature);
if feature_state.is_none() {
let rust_feature =
supported_features.iter().find_map(|&(rust_feature, _, _)| {
let llvm_features = to_llvm_features(sess, rust_feature)?;
if llvm_features.contains(feature)
&& !llvm_features.contains(rust_feature)
{
Some(rust_feature)
} else {
None
let feature_state = known_features.iter().find(|&&(v, _, _)| v == feature);
match feature_state {
None => {
let rust_feature =
known_features.iter().find_map(|&(rust_feature, _, _)| {
let llvm_features = to_llvm_features(sess, rust_feature)?;
if llvm_features.contains(feature)
&& !llvm_features.contains(rust_feature)
{
Some(rust_feature)
} else {
None
}
});
let unknown_feature = if let Some(rust_feature) = rust_feature {
UnknownCTargetFeature {
feature,
rust_feature: PossibleFeature::Some { rust_feature },
}
});
let unknown_feature = if let Some(rust_feature) = rust_feature {
UnknownCTargetFeature {
feature,
rust_feature: PossibleFeature::Some { rust_feature },
}
} else {
UnknownCTargetFeature { feature, rust_feature: PossibleFeature::None }
};
sess.dcx().emit_warn(unknown_feature);
} else if feature_state
.is_some_and(|(_name, feature_gate, _implied)| !feature_gate.is_stable())
{
// An unstable feature. Warn about using it.
sess.dcx().emit_warn(UnstableCTargetFeature { feature });
} else {
UnknownCTargetFeature {
feature,
rust_feature: PossibleFeature::None,
}
};
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 });
}
}
}

if diagnostics {
// FIXME(nagisa): figure out how to not allocate a full hashset here.
featsmap.insert(feature, enable_disable == '+');
}

// rustc-specific features do not get passed down to LLVM…
if RUSTC_SPECIFIC_FEATURES.contains(&feature) {
return None;
}

// ... otherwise though we run through `to_llvm_features` when
// We run through `to_llvm_features` when
// passing requests down to LLVM. This means that all in-language
// features also work on the command line instead of having two
// different names when the LLVM name and the Rust name differ.
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_codegen_ssa/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ codegen_ssa_failed_to_write = failed to write {$path}: {$error}

codegen_ssa_field_associated_value_expected = associated value expected for `{$name}`

codegen_ssa_forbidden_target_feature_attr =
target feature `{$feature}` cannot be toggled with `#[target_feature]`: {$reason}

codegen_ssa_ignoring_emit_path = ignoring emit path because multiple .{$extension} files were produced

codegen_ssa_ignoring_output = ignoring -o because multiple .{$extension} files were produced
Expand Down
Loading
Loading