Skip to content

Commit

Permalink
Auto merge of #129881 - veluca93:struct_tf, r=<try>
Browse files Browse the repository at this point in the history
Implement struct_target_features.

This PR implements a first version of RFC 3525.

This is a roll-up of #129764, #129783 and #129764, which will hopefully result in a PR that does not introduce perf regressions in the first place.

This PR also includes code to handle generics, unlike the original PR, since doing so influenced the design of the original PR significantly.

r? Kobzol

Tracking issue: #129107
  • Loading branch information
bors committed Oct 5, 2024
2 parents 2b21f90 + d752ae8 commit 446b363
Show file tree
Hide file tree
Showing 37 changed files with 751 additions and 66 deletions.
11 changes: 5 additions & 6 deletions compiler/rustc_codegen_gcc/src/attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use rustc_attr::InlineAttr;
use rustc_attr::InstructionSetAttr;
#[cfg(feature = "master")]
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
use rustc_middle::ty;
use rustc_middle::ty::{self, ParamEnv};
use rustc_span::symbol::sym;

use crate::context::CodegenCx;
Expand Down Expand Up @@ -72,11 +72,10 @@ pub fn from_fn_attrs<'gcc, 'tcx>(
}
}

let function_features = codegen_fn_attrs
.target_features
.iter()
.map(|features| features.name.as_str())
.collect::<Vec<&str>>();
let function_features =
codegen_fn_attrs.target_features_for_instance(cx.tcx, ParamEnv::reveal_all(), instance);
let function_features =
function_features.iter().map(|features| features.name.as_str()).collect::<Vec<&str>>();

if let Some(features) = check_tied_features(
cx.tcx.sess,
Expand Down
6 changes: 4 additions & 2 deletions compiler/rustc_codegen_llvm/src/attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use rustc_attr::{InlineAttr, InstructionSetAttr, OptimizeAttr};
use rustc_codegen_ssa::traits::*;
use rustc_hir::def_id::DefId;
use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrFlags, PatchableFunctionEntry};
use rustc_middle::ty::{self, TyCtxt};
use rustc_middle::ty::{self, ParamEnv, TyCtxt};
use rustc_session::config::{BranchProtection, FunctionReturn, OptLevel, PAuthKey, PacRet};
use rustc_span::symbol::sym;
use rustc_target::spec::{FramePointer, SanitizerSet, StackProbeType, StackProtector};
Expand Down Expand Up @@ -500,7 +500,9 @@ pub(crate) fn llfn_attrs_from_instance<'ll, 'tcx>(
to_add.extend(tune_cpu_attr(cx));

let function_features =
codegen_fn_attrs.target_features.iter().map(|f| f.name.as_str()).collect::<Vec<&str>>();
codegen_fn_attrs.target_features_for_instance(cx.tcx, ParamEnv::reveal_all(), instance);
let function_features =
function_features.iter().map(|f| f.name.as_str()).collect::<Vec<&str>>();

if let Some(f) = llvm_util::check_tied_features(
cx.tcx.sess,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ 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_safe_trait = `#[target_feature(..)]` cannot be applied to safe trait method
codegen_ssa_target_feature_safe_trait = `#[target_feature(enable = ..)]` cannot be applied to safe trait method
.label = cannot be applied to safe trait method
.label_def = not an `unsafe` function
Expand Down
77 changes: 62 additions & 15 deletions compiler/rustc_codegen_ssa/src/codegen_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ use rustc_hir::def_id::{DefId, LOCAL_CRATE, LocalDefId};
use rustc_hir::weak_lang_items::WEAK_LANG_ITEMS;
use rustc_hir::{LangItem, lang_items};
use rustc_middle::middle::codegen_fn_attrs::{
CodegenFnAttrFlags, CodegenFnAttrs, PatchableFunctionEntry,
CodegenFnAttrFlags, CodegenFnAttrs, PatchableFunctionEntry, TargetFeature,
extend_with_struct_target_features,
};
use rustc_middle::mir::mono::Linkage;
use rustc_middle::query::Providers;
Expand Down Expand Up @@ -78,23 +79,26 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
let mut link_ordinal_span = None;
let mut no_sanitize_span = None;

let fn_sig_outer = || {
use DefKind::*;

let def_kind = tcx.def_kind(did);
if let Fn | AssocFn | Variant | Ctor(..) = def_kind { Some(tcx.fn_sig(did)) } else { None }
};

for attr in attrs.iter() {
// In some cases, attribute are only valid on functions, but it's the `check_attr`
// pass that check that they aren't used anywhere else, rather this module.
// In these cases, we bail from performing further checks that are only meaningful for
// functions (such as calling `fn_sig`, which ICEs if given a non-function). We also
// report a delayed bug, just in case `check_attr` isn't doing its job.
let fn_sig = || {
use DefKind::*;

let def_kind = tcx.def_kind(did);
if let Fn | AssocFn | Variant | Ctor(..) = def_kind {
Some(tcx.fn_sig(did))
} else {
let sig = fn_sig_outer();
if sig.is_none() {
tcx.dcx()
.span_delayed_bug(attr.span, "this attribute can only be applied to functions");
None
}
sig
};

let Some(Ident { name, .. }) = attr.ident() else {
Expand Down Expand Up @@ -246,7 +250,12 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
&& let Some(fn_sig) = fn_sig()
&& fn_sig.skip_binder().safety() == hir::Safety::Safe
{
if tcx.sess.target.is_like_wasm || tcx.sess.opts.actually_rustdoc {
if attr.meta_item_list().is_some_and(|list| {
list.len() == 1 && list[0].ident().is_some_and(|x| x.name == sym::from_args)
}) {
// #[target_feature(from_args)] can be applied to safe functions and safe
// trait methods.
} else if tcx.sess.target.is_like_wasm || tcx.sess.opts.actually_rustdoc {
// The `#[target_feature]` attribute is allowed on
// WebAssembly targets on all functions, including safe
// ones. Other targets require that `#[target_feature]` is
Expand Down Expand Up @@ -284,7 +293,8 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
tcx,
attr,
supported_target_features,
&mut codegen_fn_attrs.target_features,
&mut codegen_fn_attrs.def_target_features,
Some(&mut codegen_fn_attrs.target_features_from_args),
);
}
sym::linkage => {
Expand Down Expand Up @@ -590,16 +600,39 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
let owner_id = tcx.parent(did.to_def_id());
if tcx.def_kind(owner_id).has_codegen_attrs() {
codegen_fn_attrs
.target_features
.extend(tcx.codegen_fn_attrs(owner_id).target_features.iter().copied());
.def_target_features
.extend(tcx.codegen_fn_attrs(owner_id).def_target_features.iter().copied());
}
}

// If a function uses #[target_feature] it can't be inlined into general
if let Some(sig) = fn_sig_outer()
&& codegen_fn_attrs.target_features_from_args
{
let mut additional_tf = vec![];
for ty in sig.skip_binder().inputs().skip_binder() {
extend_with_struct_target_features(
tcx,
tcx.param_env(did.to_def_id()).and(*ty),
&mut additional_tf,
)
}
if !additional_tf.is_empty() && codegen_fn_attrs.inline == InlineAttr::Always {
tcx.dcx().span_err(
tcx.hir().span(tcx.local_def_id_to_hir_id(did)),
"cannot use a struct with target features in a #[inline(always)] function",
);
}
codegen_fn_attrs
.def_target_features
.extend(additional_tf.iter().map(|tf| TargetFeature { implied: true, ..*tf }));
}

// If a function uses non-default target_features it can't be inlined into general
// purpose functions as they wouldn't have the right target features
// enabled. For that reason we also forbid #[inline(always)] as it can't be
// respected.
if !codegen_fn_attrs.target_features.is_empty() && codegen_fn_attrs.inline == InlineAttr::Always
if !codegen_fn_attrs.def_target_features.is_empty()
&& codegen_fn_attrs.inline == InlineAttr::Always
{
if let Some(span) = inline_span {
tcx.dcx().span_err(
Expand Down Expand Up @@ -738,6 +771,20 @@ fn check_link_name_xor_ordinal(
}
}

fn struct_target_features(tcx: TyCtxt<'_>, def_id: LocalDefId) -> &[TargetFeature] {
let mut features = vec![];
let supported_features = tcx.supported_target_features(LOCAL_CRATE);
for attr in tcx.get_attrs(def_id, sym::target_feature) {
from_target_feature(tcx, attr, supported_features, &mut features, None);
}
tcx.arena.alloc_slice(&features)
}

pub(crate) fn provide(providers: &mut Providers) {
*providers = Providers { codegen_fn_attrs, should_inherit_track_caller, ..*providers };
*providers = Providers {
codegen_fn_attrs,
should_inherit_track_caller,
struct_target_features,
..*providers
};
}
13 changes: 11 additions & 2 deletions compiler/rustc_codegen_ssa/src/target_features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ pub(crate) fn from_target_feature(
attr: &ast::Attribute,
supported_target_features: &UnordMap<String, Option<Symbol>>,
target_features: &mut Vec<TargetFeature>,
mut features_from_args: Option<&mut bool>,
) {
let Some(list) = attr.meta_item_list() else { return };
let bad_item = |span| {
Expand All @@ -33,6 +34,14 @@ pub(crate) fn from_target_feature(
let rust_features = tcx.features();
let mut added_target_features = Vec::new();
for item in list {
if let Some(ref mut from_args) = features_from_args
&& item.ident().is_some_and(|x| x.name == sym::from_args)
&& tcx.features().struct_target_features
{
**from_args = true;
continue;
}

// Only `enable = ...` is accepted in the meta-item list.
if !item.has_name(sym::enable) {
bad_item(item.span());
Expand Down Expand Up @@ -128,7 +137,7 @@ fn asm_target_features(tcx: TyCtxt<'_>, did: DefId) -> &FxIndexSet<Symbol> {
let mut target_features = tcx.sess.unstable_target_features.clone();
if tcx.def_kind(did).has_codegen_attrs() {
let attrs = tcx.codegen_fn_attrs(did);
target_features.extend(attrs.target_features.iter().map(|feature| feature.name));
target_features.extend(attrs.def_target_features.iter().map(|feature| feature.name));
match attrs.instruction_set {
None => {}
Some(InstructionSetAttr::ArmA32) => {
Expand All @@ -144,7 +153,7 @@ fn asm_target_features(tcx: TyCtxt<'_>, did: DefId) -> &FxIndexSet<Symbol> {
tcx.arena.alloc(target_features)
}

/// Checks the function annotated with `#[target_feature]` is not a safe
/// Checks the function annotated with `#[target_feature(enable = ...)]` is not a safe
/// trait method implementation, reporting an error if it is.
pub(crate) fn check_target_feature_trait_unsafe(tcx: TyCtxt<'_>, id: LocalDefId, attr_span: Span) {
if let DefKind::AssocFn = tcx.def_kind(id) {
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_feature/src/unstable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -598,6 +598,8 @@ declare_features! (
(unstable, strict_provenance, "1.61.0", Some(95228)),
/// Allows string patterns to dereference values to match them.
(unstable, string_deref_patterns, "1.67.0", Some(87121)),
/// Allows structs to carry target_feature information.
(incomplete, struct_target_features, "CURRENT_RUSTC_VERSION", Some(129107)),
/// Allows the use of `#[target_feature]` on safe functions.
(unstable, target_feature_11, "1.45.0", Some(69098)),
/// Allows using `#[thread_local]` on `static` items.
Expand Down
37 changes: 37 additions & 0 deletions compiler/rustc_hir/src/def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,43 @@ impl DefKind {
| DefKind::ExternCrate => false,
}
}

/// Whether `query struct_target_features` should be used with this definition.
pub fn has_struct_target_features(self) -> bool {
match self {
DefKind::Struct => true,
DefKind::Fn
| DefKind::Union
| DefKind::Enum
| DefKind::AssocFn
| DefKind::Ctor(..)
| DefKind::Closure
| DefKind::Static { .. }
| DefKind::Mod
| DefKind::Variant
| DefKind::Trait
| DefKind::TyAlias
| DefKind::ForeignTy
| DefKind::TraitAlias
| DefKind::AssocTy
| DefKind::Const
| DefKind::AssocConst
| DefKind::Macro(..)
| DefKind::Use
| DefKind::ForeignMod
| DefKind::OpaqueTy
| DefKind::Impl { .. }
| DefKind::Field
| DefKind::TyParam
| DefKind::ConstParam
| DefKind::LifetimeParam
| DefKind::AnonConst
| DefKind::InlineConst
| DefKind::SyntheticCoroutineBody
| DefKind::GlobalAsm
| DefKind::ExternCrate => false,
}
}
}

/// The resolution of a path or export.
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/check/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ fn check_main_fn_ty(tcx: TyCtxt<'_>, main_def_id: DefId) {
error = true;
}

if !tcx.codegen_fn_attrs(main_def_id).target_features.is_empty()
if !tcx.codegen_fn_attrs(main_def_id).def_target_features.is_empty()
// Calling functions with `#[target_feature]` is not unsafe on WASM, see #84988
&& !tcx.sess.target.is_like_wasm
&& !tcx.sess.opts.actually_rustdoc
Expand Down
11 changes: 8 additions & 3 deletions compiler/rustc_hir_typeck/src/coercion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -909,10 +909,15 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
return Err(TypeError::IntrinsicCast);
}

// Safe `#[target_feature]` functions are not assignable to safe fn pointers (RFC 2396).

// Safe functions with explicit `#[target_feature]` attributes are not
// assignable to safe fn pointers (RFC 2396).
if b_hdr.safety == hir::Safety::Safe
&& !self.tcx.codegen_fn_attrs(def_id).target_features.is_empty()
&& self
.tcx
.codegen_fn_attrs(def_id)
.def_target_features
.iter()
.any(|x| !x.implied)
{
return Err(TypeError::TargetFeatureCast(def_id));
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,7 @@ provide! { tcx, def_id, other, cdata,
variances_of => { table }
fn_sig => { table }
codegen_fn_attrs => { table }
struct_target_features => { table_defaulted_array }
impl_trait_header => { table }
const_param_default => { table }
object_lifetime_default => { table }
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_metadata/src/rmeta/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1398,6 +1398,9 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
if def_kind.has_codegen_attrs() {
record!(self.tables.codegen_fn_attrs[def_id] <- self.tcx.codegen_fn_attrs(def_id));
}
if def_kind.has_struct_target_features() {
record_defaulted_array!(self.tables.struct_target_features[def_id] <- self.tcx.struct_target_features(def_id));
}
if should_encode_visibility(def_kind) {
let vis =
self.tcx.local_visibility(local_id).map_id(|def_id| def_id.local_def_index);
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_metadata/src/rmeta/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use rustc_macros::{
Decodable, Encodable, MetadataDecodable, MetadataEncodable, TyDecodable, TyEncodable,
};
use rustc_middle::metadata::ModChild;
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrs;
use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrs, TargetFeature};
use rustc_middle::middle::debugger_visualizer::DebuggerVisualizerFile;
use rustc_middle::middle::exported_symbols::{ExportedSymbol, SymbolExportInfo};
use rustc_middle::middle::lib_features::FeatureStability;
Expand Down Expand Up @@ -404,6 +404,7 @@ define_tables! {
// individually instead of `DefId`s.
module_children_reexports: Table<DefIndex, LazyArray<ModChild>>,
cross_crate_inlinable: Table<DefIndex, bool>,
struct_target_features: Table<DefIndex, LazyArray<TargetFeature>>,

- optional:
attributes: Table<DefIndex, LazyArray<ast::Attribute>>,
Expand Down
Loading

0 comments on commit 446b363

Please sign in to comment.