Skip to content

Commit

Permalink
Rollup merge of rust-lang#130308 - davidtwco:tied-target-consolidatio…
Browse files Browse the repository at this point in the history
…n, r=wesleywiser

codegen_ssa: consolidate tied target checks

Fixes rust-lang#105110.
Fixes rust-lang#105111.

`rustc_codegen_llvm` and `rustc_codegen_gcc` duplicated logic for checking if tied target features were partially enabled. This PR consolidates these checks into `rustc_codegen_ssa` in the `codegen_fn_attrs` query, which also is run pre-monomorphisation for each function, which ensures that this check is run for unused functions, as would be expected.

Also adds a test confirming that enabling one tied feature doesn't imply another - the appropriate error for this was already being emitted. I did a bisect and narrowed it down to two patches it was likely to be - something in rust-lang#128796, probably rust-lang#128221 or rust-lang#128679.
  • Loading branch information
matthiaskrgr authored Oct 10, 2024
2 parents 4cc494b + 207bc77 commit b38afa0
Show file tree
Hide file tree
Showing 19 changed files with 175 additions and 163 deletions.
3 changes: 0 additions & 3 deletions compiler/rustc_codegen_gcc/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,6 @@ codegen_gcc_invalid_minimum_alignment =
codegen_gcc_lto_not_supported =
LTO is not supported. You may get a linker error.
codegen_gcc_tied_target_features = the target features {$features} must all be either enabled or disabled together
.help = add the missing features in a `target_feature` attribute
codegen_gcc_unwinding_inline_asm =
GCC backend does not support unwinding from inline asm
Expand Down
22 changes: 2 additions & 20 deletions compiler/rustc_codegen_gcc/src/attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,9 @@ use rustc_attr::InstructionSetAttr;
#[cfg(feature = "master")]
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
use rustc_middle::ty;
use rustc_span::symbol::sym;

use crate::context::CodegenCx;
use crate::errors::TiedTargetFeatures;
use crate::gcc_util::{check_tied_features, to_gcc_features};
use crate::gcc_util::to_gcc_features;

/// Get GCC attribute for the provided inline heuristic.
#[cfg(feature = "master")]
Expand Down Expand Up @@ -72,26 +70,10 @@ pub fn from_fn_attrs<'gcc, 'tcx>(
}
}

let function_features = codegen_fn_attrs
let mut function_features = codegen_fn_attrs
.target_features
.iter()
.map(|features| features.name.as_str())
.collect::<Vec<&str>>();

if let Some(features) = check_tied_features(
cx.tcx.sess,
&function_features.iter().map(|features| (*features, true)).collect(),
) {
let span = cx
.tcx
.get_attr(instance.def_id(), sym::target_feature)
.map_or_else(|| cx.tcx.def_span(instance.def_id()), |a| a.span);
cx.tcx.dcx().create_err(TiedTargetFeatures { features: features.join(", "), span }).emit();
return;
}

let mut function_features = function_features
.iter()
.flat_map(|feat| to_gcc_features(cx.tcx.sess, feat).into_iter())
.chain(codegen_fn_attrs.instruction_set.iter().map(|x| match *x {
InstructionSetAttr::ArmA32 => "-thumb-mode", // TODO(antoyo): support removing feature.
Expand Down
36 changes: 0 additions & 36 deletions compiler/rustc_codegen_gcc/src/errors.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
use rustc_errors::{Diag, DiagCtxtHandle, Diagnostic, EmissionGuarantee, Level};
use rustc_macros::{Diagnostic, Subdiagnostic};
use rustc_span::Span;

use crate::fluent_generated as fluent;

#[derive(Diagnostic)]
#[diag(codegen_gcc_unknown_ctarget_feature_prefix)]
#[note]
Expand Down Expand Up @@ -45,15 +42,6 @@ pub(crate) struct InvalidMinimumAlignment {
pub err: String,
}

#[derive(Diagnostic)]
#[diag(codegen_gcc_tied_target_features)]
#[help]
pub(crate) struct TiedTargetFeatures {
#[primary_span]
pub span: Span,
pub features: String,
}

#[derive(Diagnostic)]
#[diag(codegen_gcc_copy_bitcode)]
pub(crate) struct CopyBitcode {
Expand All @@ -78,27 +66,3 @@ pub(crate) struct LtoDylib;
pub(crate) struct LtoBitcodeFromRlib {
pub gcc_err: String,
}

pub(crate) struct TargetFeatureDisableOrEnable<'a> {
pub features: &'a [&'a str],
pub span: Option<Span>,
pub missing_features: Option<MissingFeatures>,
}

#[derive(Subdiagnostic)]
#[help(codegen_gcc_missing_features)]
pub(crate) struct MissingFeatures;

impl<G: EmissionGuarantee> Diagnostic<'_, G> for TargetFeatureDisableOrEnable<'_> {
fn into_diag(self, dcx: DiagCtxtHandle<'_>, level: Level) -> Diag<'_, G> {
let mut diag = Diag::new(dcx, level, fluent::codegen_gcc_target_feature_disable_or_enable);
if let Some(span) = self.span {
diag.span(span);
};
if let Some(missing_features) = self.missing_features {
diag.subdiagnostic(missing_features);
}
diag.arg("features", self.features.join(", "));
diag
}
}
24 changes: 3 additions & 21 deletions compiler/rustc_codegen_gcc/src/gcc_util.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
#[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;
use rustc_middle::bug;
use rustc_session::Session;
use rustc_target::target_features::RUSTC_SPECIFIC_FEATURES;
use smallvec::{SmallVec, smallvec};

use crate::errors::{
PossibleFeature, TargetFeatureDisableOrEnable, UnknownCTargetFeature,
UnknownCTargetFeaturePrefix,
};
use crate::errors::{PossibleFeature, UnknownCTargetFeature, UnknownCTargetFeaturePrefix};

/// The list of GCC features computed from CLI flags (`-Ctarget-cpu`, `-Ctarget-feature`,
/// `--target` and similar).
Expand Down Expand Up @@ -185,23 +184,6 @@ pub fn to_gcc_features<'a>(sess: &Session, s: &'a str) -> SmallVec<[&'a str; 2]>
}
}

// Given a map from target_features to whether they are enabled or disabled,
// ensure only valid combinations are allowed.
pub fn check_tied_features(
sess: &Session,
features: &FxHashMap<&str, bool>,
) -> Option<&'static [&'static str]> {
for tied in sess.target.tied_target_features() {
// Tied features must be set to the same value, or not set at all
let mut tied_iter = tied.iter();
let enabled = features.get(tied_iter.next().unwrap());
if tied_iter.any(|feature| enabled != features.get(feature)) {
return Some(tied);
}
}
None
}

fn arch_to_gcc(name: &str) -> &str {
match name {
"M68020" => "68020",
Expand Down
6 changes: 0 additions & 6 deletions compiler/rustc_codegen_llvm/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,6 @@ codegen_llvm_lto_proc_macro = lto cannot be used for `proc-macro` crate type wit
codegen_llvm_mismatch_data_layout =
data-layout for target `{$rustc_target}`, `{$rustc_layout}`, differs from LLVM target's `{$llvm_target}` default layout, `{$llvm_layout}`
codegen_llvm_missing_features =
add the missing features in a `target_feature` attribute
codegen_llvm_multiple_source_dicompileunit = multiple source DICompileUnits found
codegen_llvm_multiple_source_dicompileunit_with_llvm_err = multiple source DICompileUnits found: {$llvm_err}
Expand Down Expand Up @@ -63,9 +60,6 @@ codegen_llvm_serialize_module_with_llvm_err = failed to serialize module {$name}
codegen_llvm_symbol_already_defined =
symbol `{$symbol_name}` is already defined
codegen_llvm_target_feature_disable_or_enable =
the target features {$features} must all be either enabled or disabled together
codegen_llvm_target_machine = could not create LLVM TargetMachine for triple: {$triple}
codegen_llvm_target_machine_with_llvm_err = could not create LLVM TargetMachine for triple: {$triple}: {$llvm_err}
Expand Down
23 changes: 1 addition & 22 deletions compiler/rustc_codegen_llvm/src/attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,11 @@ use rustc_hir::def_id::DefId;
use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrFlags, PatchableFunctionEntry};
use rustc_middle::ty::{self, TyCtxt};
use rustc_session::config::{BranchProtection, FunctionReturn, OptLevel, PAuthKey, PacRet};
use rustc_span::symbol::sym;
use rustc_target::spec::{FramePointer, SanitizerSet, StackProbeType, StackProtector};
use smallvec::SmallVec;

use crate::context::CodegenCx;
use crate::errors::{MissingFeatures, SanitizerMemtagRequiresMte, TargetFeatureDisableOrEnable};
use crate::errors::SanitizerMemtagRequiresMte;
use crate::llvm::AttributePlace::Function;
use crate::llvm::{self, AllocKindFlags, Attribute, AttributeKind, AttributePlace, MemoryEffects};
use crate::value::Value;
Expand Down Expand Up @@ -502,26 +501,6 @@ pub(crate) fn llfn_attrs_from_instance<'ll, 'tcx>(
let function_features =
codegen_fn_attrs.target_features.iter().map(|f| f.name.as_str()).collect::<Vec<&str>>();

if let Some(f) = llvm_util::check_tied_features(
cx.tcx.sess,
&function_features.iter().map(|f| (*f, true)).collect(),
) {
let span = cx
.tcx
.get_attrs(instance.def_id(), sym::target_feature)
.next()
.map_or_else(|| cx.tcx.def_span(instance.def_id()), |a| a.span);
cx.tcx
.dcx()
.create_err(TargetFeatureDisableOrEnable {
features: f,
span: Some(span),
missing_features: Some(MissingFeatures),
})
.emit();
return;
}

let function_features = function_features
.iter()
// Convert to LLVMFeatures and filter out unavailable ones
Expand Down
24 changes: 0 additions & 24 deletions compiler/rustc_codegen_llvm/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,30 +80,6 @@ impl<G: EmissionGuarantee> Diagnostic<'_, G> for ParseTargetMachineConfig<'_> {
}
}

pub(crate) struct TargetFeatureDisableOrEnable<'a> {
pub features: &'a [&'a str],
pub span: Option<Span>,
pub missing_features: Option<MissingFeatures>,
}

#[derive(Subdiagnostic)]
#[help(codegen_llvm_missing_features)]
pub(crate) struct MissingFeatures;

impl<G: EmissionGuarantee> Diagnostic<'_, G> for TargetFeatureDisableOrEnable<'_> {
fn into_diag(self, dcx: DiagCtxtHandle<'_>, level: Level) -> Diag<'_, G> {
let mut diag = Diag::new(dcx, level, fluent::codegen_llvm_target_feature_disable_or_enable);
if let Some(span) = self.span {
diag.span(span);
};
if let Some(missing_features) = self.missing_features {
diag.subdiagnostic(missing_features);
}
diag.arg("features", self.features.join(", "));
diag
}
}

#[derive(Diagnostic)]
#[diag(codegen_llvm_lto_disallowed)]
pub(crate) struct LtoDisallowed;
Expand Down
26 changes: 4 additions & 22 deletions compiler/rustc_codegen_llvm/src/llvm_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use std::{ptr, slice, str};

use libc::c_int;
use rustc_codegen_ssa::base::wants_wasm_eh;
use rustc_codegen_ssa::codegen_attrs::check_tied_features;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_data_structures::small_c_str::SmallCStr;
use rustc_data_structures::unord::UnordSet;
Expand All @@ -19,8 +20,8 @@ use rustc_target::target_features::{RUSTC_SPECIAL_FEATURES, RUSTC_SPECIFIC_FEATU

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

Expand Down Expand Up @@ -276,25 +277,6 @@ pub(crate) fn to_llvm_features<'a>(sess: &Session, s: &'a str) -> Option<LLVMFea
}
}

/// Given a map from target_features to whether they are enabled or disabled,
/// ensure only valid combinations are allowed.
pub(crate) fn check_tied_features(
sess: &Session,
features: &FxHashMap<&str, bool>,
) -> Option<&'static [&'static str]> {
if !features.is_empty() {
for tied in sess.target.tied_target_features() {
// Tied features must be set to the same value, or not set at all
let mut tied_iter = tied.iter();
let enabled = features.get(tied_iter.next().unwrap());
if tied_iter.any(|f| enabled != features.get(f)) {
return Some(tied);
}
}
}
None
}

/// Used to generate cfg variables and apply features
/// Must express features in the way Rust understands them
pub fn target_features(sess: &Session, allow_unstable: bool) -> Vec<Symbol> {
Expand Down Expand Up @@ -685,7 +667,7 @@ pub(crate) fn global_llvm_features(
features.extend(feats);

if diagnostics && let Some(f) = check_tied_features(sess, &featsmap) {
sess.dcx().emit_err(TargetFeatureDisableOrEnable {
sess.dcx().emit_err(rustc_codegen_ssa::errors::TargetFeatureDisableOrEnable {
features: f,
span: None,
missing_features: None,
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_codegen_ssa/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,8 @@ codegen_ssa_metadata_object_file_write = error writing metadata object file: {$e
codegen_ssa_missing_cpp_build_tool_component = or a necessary component may be missing from the "C++ build tools" workload
codegen_ssa_missing_features = add the missing features in a `target_feature` attribute
codegen_ssa_missing_memory_ordering = Atomic intrinsic missing memory ordering
codegen_ssa_missing_query_depgraph =
Expand Down Expand Up @@ -238,6 +240,9 @@ codegen_ssa_stripping_debug_info_failed = stripping debug info with `{$util}` fa
codegen_ssa_symbol_file_write_failure = failed to write symbols file: {$error}
codegen_ssa_target_feature_disable_or_enable =
the target features {$features} must all be either enabled or disabled together
codegen_ssa_target_feature_safe_trait = `#[target_feature(..)]` cannot be applied to safe trait method
.label = cannot be applied to safe trait method
.label_def = not an `unsafe` function
Expand Down
45 changes: 43 additions & 2 deletions compiler/rustc_codegen_ssa/src/codegen_attrs.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use rustc_ast::{MetaItemInner, MetaItemKind, ast, attr};
use rustc_attr::{InlineAttr, InstructionSetAttr, OptimizeAttr, list_contains_name};
use rustc_data_structures::fx::FxHashMap;
use rustc_errors::codes::*;
use rustc_errors::{DiagMessage, SubdiagMessage, struct_span_code_err};
use rustc_hir as hir;
Expand All @@ -13,13 +14,13 @@ use rustc_middle::middle::codegen_fn_attrs::{
use rustc_middle::mir::mono::Linkage;
use rustc_middle::query::Providers;
use rustc_middle::ty::{self as ty, TyCtxt};
use rustc_session::lint;
use rustc_session::parse::feature_err;
use rustc_session::{Session, lint};
use rustc_span::symbol::Ident;
use rustc_span::{Span, sym};
use rustc_target::spec::{SanitizerSet, abi};

use crate::errors;
use crate::errors::{self, MissingFeatures, TargetFeatureDisableOrEnable};
use crate::target_features::{check_target_feature_trait_unsafe, from_target_feature};

fn linkage_by_name(tcx: TyCtxt<'_>, def_id: LocalDefId, name: &str) -> Linkage {
Expand Down Expand Up @@ -662,9 +663,49 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
}
}

if let Some(features) = check_tied_features(
tcx.sess,
&codegen_fn_attrs
.target_features
.iter()
.map(|features| (features.name.as_str(), true))
.collect(),
) {
let span = tcx
.get_attrs(did, sym::target_feature)
.next()
.map_or_else(|| tcx.def_span(did), |a| a.span);
tcx.dcx()
.create_err(TargetFeatureDisableOrEnable {
features,
span: Some(span),
missing_features: Some(MissingFeatures),
})
.emit();
}

codegen_fn_attrs
}

/// Given a map from target_features to whether they are enabled or disabled, ensure only valid
/// combinations are allowed.
pub fn check_tied_features(
sess: &Session,
features: &FxHashMap<&str, bool>,
) -> Option<&'static [&'static str]> {
if !features.is_empty() {
for tied in sess.target.tied_target_features() {
// Tied features must be set to the same value, or not set at all
let mut tied_iter = tied.iter();
let enabled = features.get(tied_iter.next().unwrap());
if tied_iter.any(|f| enabled != features.get(f)) {
return Some(tied);
}
}
}
None
}

/// Checks if the provided DefId is a method in a trait impl for a trait which has track_caller
/// applied to the method prototype.
fn should_inherit_track_caller(tcx: TyCtxt<'_>, def_id: DefId) -> bool {
Expand Down
Loading

0 comments on commit b38afa0

Please sign in to comment.