From 69a19f4728dfa9c529e7ab288a53a160a75d13d9 Mon Sep 17 00:00:00 2001 From: Luca Versari Date: Mon, 2 Sep 2024 09:42:28 +0200 Subject: [PATCH 1/5] Implement struct_target_features for non-generic functions. --- .../rustc_codegen_ssa/src/codegen_attrs.rs | 89 ++++++++++-- compiler/rustc_feature/src/unstable.rs | 2 + compiler/rustc_hir/src/def.rs | 37 +++++ compiler/rustc_hir_typeck/src/coercion.rs | 2 + .../src/rmeta/decoder/cstore_impl.rs | 1 + compiler/rustc_metadata/src/rmeta/encoder.rs | 3 + compiler/rustc_metadata/src/rmeta/mod.rs | 3 +- .../src/middle/codegen_fn_attrs.rs | 8 +- compiler/rustc_middle/src/query/mod.rs | 11 +- compiler/rustc_middle/src/ty/parameterized.rs | 1 + compiler/rustc_mir_build/messages.ftl | 46 ++++++ .../rustc_mir_build/src/check_unsafety.rs | 132 +++++++++++++++++- compiler/rustc_mir_build/src/errors.rs | 56 ++++++++ compiler/rustc_passes/messages.ftl | 4 + compiler/rustc_passes/src/check_attr.rs | 18 ++- compiler/rustc_passes/src/errors.rs | 9 ++ compiler/rustc_span/src/symbol.rs | 1 + .../traits/fulfillment_errors.rs | 2 + .../src/traits/select/candidate_assembly.rs | 2 + .../struct-target-features.md | 7 + tests/assembly/struct-target-features.rs | 31 ++++ .../feature-gate-struct-target-features.rs | 4 + ...feature-gate-struct-target-features.stderr | 10 ++ .../struct-target-features-crate-dep.rs | 6 + .../struct-target-features-crate.rs | 23 +++ .../target-feature/struct-target-features.rs | 100 +++++++++++++ .../struct-target-features.stderr | 38 +++++ 27 files changed, 618 insertions(+), 28 deletions(-) create mode 100644 src/doc/unstable-book/src/language-features/struct-target-features.md create mode 100644 tests/assembly/struct-target-features.rs create mode 100644 tests/ui/feature-gates/feature-gate-struct-target-features.rs create mode 100644 tests/ui/feature-gates/feature-gate-struct-target-features.stderr create mode 100644 tests/ui/target-feature/auxiliary/struct-target-features-crate-dep.rs create mode 100644 tests/ui/target-feature/struct-target-features-crate.rs create mode 100644 tests/ui/target-feature/struct-target-features.rs create mode 100644 tests/ui/target-feature/struct-target-features.stderr diff --git a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs index d536419ab3c20..8b492bf13eaca 100644 --- a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs +++ b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs @@ -9,11 +9,11 @@ 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, }; use rustc_middle::mir::mono::Linkage; use rustc_middle::query::Providers; -use rustc_middle::ty::{self as ty, TyCtxt}; +use rustc_middle::ty::{self as ty, Ty, TyCtxt}; use rustc_session::parse::feature_err; use rustc_session::{Session, lint}; use rustc_span::symbol::Ident; @@ -79,6 +79,13 @@ 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. @@ -86,16 +93,12 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs { // 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 { @@ -596,7 +599,30 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs { } } - // If a function uses #[target_feature] it can't be inlined into general + if let Some(sig) = fn_sig_outer() { + for ty in sig.skip_binder().inputs().skip_binder() { + let additional_tf = + tcx.struct_reachable_target_features(tcx.param_env(did.to_def_id()).and(*ty)); + // FIXME(struct_target_features): is this really necessary? + if !additional_tf.is_empty() && sig.skip_binder().abi() != abi::Abi::Rust { + tcx.dcx().span_err( + tcx.hir().span(tcx.local_def_id_to_hir_id(did)), + "cannot use a struct with target features in a function with non-Rust ABI", + ); + } + 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 + .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. @@ -779,6 +805,47 @@ 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); + } + tcx.arena.alloc_slice(&features) +} + +fn struct_reachable_target_features<'tcx>( + tcx: TyCtxt<'tcx>, + env: ty::ParamEnvAnd<'tcx, Ty<'tcx>>, +) -> &'tcx [TargetFeature] { + // Collect target features from types reachable from `env.value` by dereferencing a certain + // number of references and resolving aliases. + + let mut ty = env.value; + if matches!(ty.kind(), ty::Alias(..)) { + ty = match tcx.try_normalize_erasing_regions(env.param_env, ty) { + Ok(ty) => ty, + Err(_) => return tcx.arena.alloc_slice(&[]), + }; + } + while let ty::Ref(_, inner, _) = ty.kind() { + ty = *inner; + } + + let tf = if let ty::Adt(adt_def, ..) = ty.kind() { + tcx.struct_target_features(adt_def.did()) + } else { + &[] + }; + tcx.arena.alloc_slice(tf) +} + 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, + struct_reachable_target_features, + ..*providers + }; } diff --git a/compiler/rustc_feature/src/unstable.rs b/compiler/rustc_feature/src/unstable.rs index c0398db9c101f..32f83f5214a47 100644 --- a/compiler/rustc_feature/src/unstable.rs +++ b/compiler/rustc_feature/src/unstable.rs @@ -601,6 +601,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. diff --git a/compiler/rustc_hir/src/def.rs b/compiler/rustc_hir/src/def.rs index 3276f516a52a1..5c9dc69e00918 100644 --- a/compiler/rustc_hir/src/def.rs +++ b/compiler/rustc_hir/src/def.rs @@ -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. diff --git a/compiler/rustc_hir_typeck/src/coercion.rs b/compiler/rustc_hir_typeck/src/coercion.rs index bd0b98702983a..571fe50a088bd 100644 --- a/compiler/rustc_hir_typeck/src/coercion.rs +++ b/compiler/rustc_hir_typeck/src/coercion.rs @@ -921,6 +921,8 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> { } // Safe `#[target_feature]` functions are not assignable to safe fn pointers (RFC 2396). + // FIXME(struct_target_features): should this be true also for functions that inherit + // target features from structs? if b_hdr.safety == hir::Safety::Safe && !self.tcx.codegen_fn_attrs(def_id).target_features.is_empty() diff --git a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs index 69707fdbe8fa1..b2bfcd27e3545 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs @@ -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 } diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs index afe03531861c9..fe01bfda91ed5 100644 --- a/compiler/rustc_metadata/src/rmeta/encoder.rs +++ b/compiler/rustc_metadata/src/rmeta/encoder.rs @@ -1401,6 +1401,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); diff --git a/compiler/rustc_metadata/src/rmeta/mod.rs b/compiler/rustc_metadata/src/rmeta/mod.rs index 79bd1c13b1216..b7d6973cfd50e 100644 --- a/compiler/rustc_metadata/src/rmeta/mod.rs +++ b/compiler/rustc_metadata/src/rmeta/mod.rs @@ -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; @@ -404,6 +404,7 @@ define_tables! { // individually instead of `DefId`s. module_children_reexports: Table>, cross_crate_inlinable: Table, + struct_target_features: Table>, - optional: attributes: Table>, diff --git a/compiler/rustc_middle/src/middle/codegen_fn_attrs.rs b/compiler/rustc_middle/src/middle/codegen_fn_attrs.rs index 90dff0f5c7da8..71b17aaad2cf3 100644 --- a/compiler/rustc_middle/src/middle/codegen_fn_attrs.rs +++ b/compiler/rustc_middle/src/middle/codegen_fn_attrs.rs @@ -26,8 +26,8 @@ pub struct CodegenFnAttrs { /// be set when `link_name` is set. This is for foreign items with the /// "raw-dylib" kind. pub link_ordinal: Option, - /// The `#[target_feature(enable = "...")]` attribute and the enabled - /// features (only enabled features are supported right now). + /// All the target features that are enabled for this function. Some features might be enabled + /// implicitly. pub target_features: Vec, /// The `#[linkage = "..."]` attribute on Rust-defined items and the value we found. pub linkage: Option, @@ -55,8 +55,8 @@ pub struct CodegenFnAttrs { pub struct TargetFeature { /// The name of the target feature (e.g. "avx") pub name: Symbol, - /// The feature is implied by another feature, rather than explicitly added by the - /// `#[target_feature]` attribute + /// The feature is implied by another feature or by an argument, rather than explicitly + /// added by the `#[target_feature]` attribute pub implied: bool, } diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index f0be70e00dfca..6f1b64fc72c12 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -48,7 +48,7 @@ use {rustc_ast as ast, rustc_attr as attr, rustc_hir as hir}; use crate::infer::canonical::{self, Canonical}; use crate::lint::LintExpectation; use crate::metadata::ModChild; -use crate::middle::codegen_fn_attrs::CodegenFnAttrs; +use crate::middle::codegen_fn_attrs::{CodegenFnAttrs, TargetFeature}; use crate::middle::debugger_visualizer::DebuggerVisualizerFile; use crate::middle::exported_symbols::{ExportedSymbol, SymbolExportInfo}; use crate::middle::lib_features::LibFeatures; @@ -1256,6 +1256,15 @@ rustc_queries! { feedable } + query struct_target_features(def_id: DefId) -> &'tcx [TargetFeature] { + separate_provide_extern + desc { |tcx| "computing target features for struct `{}`", tcx.def_path_str(def_id) } + } + + query struct_reachable_target_features(env: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> &'tcx [TargetFeature] { + desc { |tcx| "computing target features reachable from {}", env.value } + } + query asm_target_features(def_id: DefId) -> &'tcx FxIndexSet { desc { |tcx| "computing target features for inline asm of `{}`", tcx.def_path_str(def_id) } } diff --git a/compiler/rustc_middle/src/ty/parameterized.rs b/compiler/rustc_middle/src/ty/parameterized.rs index 7e1255f606c35..be611e19b49a7 100644 --- a/compiler/rustc_middle/src/ty/parameterized.rs +++ b/compiler/rustc_middle/src/ty/parameterized.rs @@ -59,6 +59,7 @@ trivially_parameterized_over_tcx! { std::string::String, crate::metadata::ModChild, crate::middle::codegen_fn_attrs::CodegenFnAttrs, + crate::middle::codegen_fn_attrs::TargetFeature, crate::middle::debugger_visualizer::DebuggerVisualizerFile, crate::middle::exported_symbols::SymbolExportInfo, crate::middle::lib_features::FeatureStability, diff --git a/compiler/rustc_mir_build/messages.ftl b/compiler/rustc_mir_build/messages.ftl index 1c4e9fd11cbd6..2649c7cbd0515 100644 --- a/compiler/rustc_mir_build/messages.ftl +++ b/compiler/rustc_mir_build/messages.ftl @@ -125,6 +125,37 @@ mir_build_initializing_type_with_requires_unsafe_unsafe_op_in_unsafe_fn_allowed .note = initializing a layout restricted type's field with a value outside the valid range is undefined behavior .label = initializing type with `rustc_layout_scalar_valid_range` attr +mir_build_initializing_type_with_target_feature_requires_unsafe = + initializing type `{$adt}` with `#[target_feature]` is unsafe and requires unsafe block + .help = in order for the call to be safe, the context requires the following additional target {$missing_target_features_count -> + [1] feature + *[count] features + }: {$missing_target_features} + .note = the {$build_target_features} target {$build_target_features_count -> + [1] feature + *[count] features + } being enabled in the build configuration does not remove the requirement to list {$build_target_features_count -> + [1] it + *[count] them + } in `#[target_feature]` + .label = call to function with `#[target_feature]` + +mir_build_initializing_type_with_target_feature_requires_unsafe_unsafe_op_in_unsafe_fn_allowed = + initializing type `{$adt}` with `#[target_feature]` is unsafe and requires unsafe function or block + .help = in order for the call to be safe, the context requires the following additional target {$missing_target_features_count -> + [1] feature + *[count] features + }: {$missing_target_features} + .note = the {$build_target_features} target {$build_target_features_count -> + [1] feature + *[count] features + } being enabled in the build configuration does not remove the requirement to list {$build_target_features_count -> + [1] it + *[count] them + } in `#[target_feature]` + .label = call to function with `#[target_feature]` + + mir_build_inline_assembly_requires_unsafe = use of inline assembly is unsafe and requires unsafe block .note = inline assembly is entirely unchecked and can cause undefined behavior @@ -388,6 +419,21 @@ mir_build_unsafe_op_in_unsafe_fn_initializing_type_with_requires_unsafe = .note = initializing a layout restricted type's field with a value outside the valid range is undefined behavior .label = initializing type with `rustc_layout_scalar_valid_range` attr +mir_build_unsafe_op_in_unsafe_fn_initializing_type_with_target_feature_requires_unsafe = + initializing type `{$adt}` with `#[target_feature]` is unsafe and requires unsafe block + .help = in order for the call to be safe, the context requires the following additional target {$missing_target_features_count -> + [1] feature + *[count] features + }: {$missing_target_features} + .note = the {$build_target_features} target {$build_target_features_count -> + [1] feature + *[count] features + } being enabled in the build configuration does not remove the requirement to list {$build_target_features_count -> + [1] it + *[count] them + } in `#[target_feature]` + .label = call to function with `#[target_feature]` + mir_build_unsafe_op_in_unsafe_fn_inline_assembly_requires_unsafe = use of inline assembly is unsafe and requires unsafe block .note = inline assembly is entirely unchecked and can cause undefined behavior diff --git a/compiler/rustc_mir_build/src/check_unsafety.rs b/compiler/rustc_mir_build/src/check_unsafety.rs index 8512763a595d5..f393fabccbd26 100644 --- a/compiler/rustc_mir_build/src/check_unsafety.rs +++ b/compiler/rustc_mir_build/src/check_unsafety.rs @@ -469,14 +469,18 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> { }; self.requires_unsafe(expr.span, CallToUnsafeFunction(func_id)); } else if let &ty::FnDef(func_did, _) = self.thir[fun].ty.kind() { - // If the called function has target features the calling function hasn't, + // If the called function has explicit target features the calling function hasn't, // the call requires `unsafe`. Don't check this on wasm // targets, though. For more information on wasm see the // is_like_wasm check in hir_analysis/src/collect.rs + // Implicit target features are OK because they are either a consequence of some + // explicit target feature (which is checked to be present in the caller) or + // come from a witness argument. let callee_features = &self.tcx.codegen_fn_attrs(func_did).target_features; if !self.tcx.sess.target.options.is_like_wasm && !callee_features.iter().all(|feature| { - self.body_target_features.iter().any(|f| f.name == feature.name) + feature.implied + || self.body_target_features.iter().any(|f| f.name == feature.name) }) { let missing: Vec<_> = callee_features @@ -551,10 +555,52 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> { user_ty: _, fields: _, base: _, - }) => match self.tcx.layout_scalar_valid_range(adt_def.did()) { - (Bound::Unbounded, Bound::Unbounded) => {} - _ => self.requires_unsafe(expr.span, InitializingTypeWith), - }, + }) => { + match self.tcx.layout_scalar_valid_range(adt_def.did()) { + (Bound::Unbounded, Bound::Unbounded) => {} + _ => self.requires_unsafe(expr.span, InitializingTypeWith), + } + let struct_features = self.tcx.struct_target_features(adt_def.did()); + if !struct_features.is_empty() { + if !self.tcx.sess.target.options.is_like_wasm + && !struct_features.iter().all(|feature| { + feature.implied + || self.body_target_features.iter().any(|f| f.name == feature.name) + }) + { + // Matches the logic for calling non-unsafe `target_feature` functions. + let missing: Vec<_> = struct_features + .iter() + .copied() + .filter(|feature| { + !feature.implied + && !self + .body_target_features + .iter() + .any(|body_feature| body_feature.name == feature.name) + }) + .map(|feature| feature.name) + .collect(); + let build_enabled = self + .tcx + .sess + .target_features + .iter() + .copied() + .filter(|feature| missing.contains(feature)) + .collect(); + self.requires_unsafe( + expr.span, + ConstructingTargetFeaturesTypeWith { + adt: adt_def.did(), + missing, + build_enabled, + }, + ); + } + } + } + ExprKind::Closure(box ClosureExpr { closure_id, args: _, @@ -656,6 +702,15 @@ enum UnsafeOpKind { CallToUnsafeFunction(Option), UseOfInlineAssembly, InitializingTypeWith, + ConstructingTargetFeaturesTypeWith { + adt: DefId, + /// Target features enabled in callee's `#[target_feature]` but missing in + /// caller's `#[target_feature]`. + missing: Vec, + /// Target features in `missing` that are enabled at compile time + /// (e.g., with `-C target-feature`). + build_enabled: Vec, + }, UseOfMutableStatic, UseOfExternStatic, DerefOfRawPointer, @@ -737,6 +792,29 @@ impl UnsafeOpKind { unsafe_not_inherited_note, }, ), + ConstructingTargetFeaturesTypeWith { adt, missing, build_enabled } => tcx + .emit_node_span_lint( + UNSAFE_OP_IN_UNSAFE_FN, + hir_id, + span, + UnsafeOpInUnsafeFnInitializingTypeWithTargetFeatureRequiresUnsafe { + span, + adt: with_no_trimmed_paths!(tcx.def_path_str(*adt)), + missing_target_features: DiagArgValue::StrListSepByAnd( + missing.iter().map(|feature| Cow::from(feature.to_string())).collect(), + ), + missing_target_features_count: missing.len(), + note: !build_enabled.is_empty(), + build_target_features: DiagArgValue::StrListSepByAnd( + build_enabled + .iter() + .map(|feature| Cow::from(feature.to_string())) + .collect(), + ), + build_target_features_count: build_enabled.len(), + unsafe_not_inherited_note, + }, + ), UseOfMutableStatic => tcx.emit_node_span_lint( UNSAFE_OP_IN_UNSAFE_FN, hir_id, @@ -894,6 +972,48 @@ impl UnsafeOpKind { unsafe_not_inherited_note, }); } + ConstructingTargetFeaturesTypeWith { adt, missing, build_enabled } + if unsafe_op_in_unsafe_fn_allowed => + { + dcx.emit_err( + InitializingTypeWithTargetFeatureRequiresUnsafeUnsafeOpInUnsafeFnAllowed { + span, + adt: with_no_trimmed_paths!(tcx.def_path_str(*adt)), + missing_target_features: DiagArgValue::StrListSepByAnd( + missing.iter().map(|feature| Cow::from(feature.to_string())).collect(), + ), + missing_target_features_count: missing.len(), + note: !build_enabled.is_empty(), + build_target_features: DiagArgValue::StrListSepByAnd( + build_enabled + .iter() + .map(|feature| Cow::from(feature.to_string())) + .collect(), + ), + build_target_features_count: build_enabled.len(), + unsafe_not_inherited_note, + }, + ); + } + ConstructingTargetFeaturesTypeWith { adt, missing, build_enabled } => { + dcx.emit_err(InitializingTypeWithTargetFeatureRequiresUnsafe { + span, + adt: with_no_trimmed_paths!(tcx.def_path_str(*adt)), + missing_target_features: DiagArgValue::StrListSepByAnd( + missing.iter().map(|feature| Cow::from(feature.to_string())).collect(), + ), + missing_target_features_count: missing.len(), + note: !build_enabled.is_empty(), + build_target_features: DiagArgValue::StrListSepByAnd( + build_enabled + .iter() + .map(|feature| Cow::from(feature.to_string())) + .collect(), + ), + build_target_features_count: build_enabled.len(), + unsafe_not_inherited_note, + }); + } UseOfMutableStatic if unsafe_op_in_unsafe_fn_allowed => { dcx.emit_err(UseOfMutableStaticRequiresUnsafeUnsafeOpInUnsafeFnAllowed { span, diff --git a/compiler/rustc_mir_build/src/errors.rs b/compiler/rustc_mir_build/src/errors.rs index 42be7f9402ecc..f54f33733d2c8 100644 --- a/compiler/rustc_mir_build/src/errors.rs +++ b/compiler/rustc_mir_build/src/errors.rs @@ -86,6 +86,23 @@ pub(crate) struct UnsafeOpInUnsafeFnInitializingTypeWithRequiresUnsafe { pub(crate) unsafe_not_inherited_note: Option, } +#[derive(LintDiagnostic)] +#[diag(mir_build_unsafe_op_in_unsafe_fn_initializing_type_with_target_feature_requires_unsafe, code = E0133)] +#[note] +pub(crate) struct UnsafeOpInUnsafeFnInitializingTypeWithTargetFeatureRequiresUnsafe { + #[label] + pub(crate) span: Span, + pub(crate) adt: String, + pub(crate) missing_target_features: DiagArgValue, + pub(crate) missing_target_features_count: usize, + #[note] + pub(crate) note: bool, + pub(crate) build_target_features: DiagArgValue, + pub(crate) build_target_features_count: usize, + #[subdiagnostic] + pub(crate) unsafe_not_inherited_note: Option, +} + #[derive(LintDiagnostic)] #[diag(mir_build_unsafe_op_in_unsafe_fn_mutable_static_requires_unsafe, code = E0133)] #[note] @@ -250,6 +267,24 @@ pub(crate) struct InitializingTypeWithRequiresUnsafe { pub(crate) unsafe_not_inherited_note: Option, } +#[derive(Diagnostic)] +#[diag(mir_build_initializing_type_with_target_feature_requires_unsafe, code = E0133)] +#[note] +pub(crate) struct InitializingTypeWithTargetFeatureRequiresUnsafe { + #[primary_span] + #[label] + pub(crate) span: Span, + pub(crate) adt: String, + pub(crate) missing_target_features: DiagArgValue, + pub(crate) missing_target_features_count: usize, + #[note] + pub(crate) note: bool, + pub(crate) build_target_features: DiagArgValue, + pub(crate) build_target_features_count: usize, + #[subdiagnostic] + pub(crate) unsafe_not_inherited_note: Option, +} + #[derive(Diagnostic)] #[diag( mir_build_initializing_type_with_requires_unsafe_unsafe_op_in_unsafe_fn_allowed, @@ -264,6 +299,27 @@ pub(crate) struct InitializingTypeWithRequiresUnsafeUnsafeOpInUnsafeFnAllowed { pub(crate) unsafe_not_inherited_note: Option, } +#[derive(Diagnostic)] +#[diag( + mir_build_initializing_type_with_target_feature_requires_unsafe_unsafe_op_in_unsafe_fn_allowed, + code = E0133 +)] +#[note] +pub(crate) struct InitializingTypeWithTargetFeatureRequiresUnsafeUnsafeOpInUnsafeFnAllowed { + #[primary_span] + #[label] + pub(crate) span: Span, + pub(crate) adt: String, + pub(crate) missing_target_features: DiagArgValue, + pub(crate) missing_target_features_count: usize, + #[note] + pub(crate) note: bool, + pub(crate) build_target_features: DiagArgValue, + pub(crate) build_target_features_count: usize, + #[subdiagnostic] + pub(crate) unsafe_not_inherited_note: Option, +} + #[derive(Diagnostic)] #[diag(mir_build_mutable_static_requires_unsafe, code = E0133)] #[note] diff --git a/compiler/rustc_passes/messages.ftl b/compiler/rustc_passes/messages.ftl index c11c38500345a..fea5d7c42d066 100644 --- a/compiler/rustc_passes/messages.ftl +++ b/compiler/rustc_passes/messages.ftl @@ -669,6 +669,10 @@ passes_should_be_applied_to_fn = *[false] not a function definition } +passes_should_be_applied_to_fn_or_struct = + attribute should be applied to a function definition or struct + .label = not a function definition or a struct + passes_should_be_applied_to_static = attribute should be applied to a static .label = not a static diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs index 44a62383e6eed..eeda3a4e38efb 100644 --- a/compiler/rustc_passes/src/check_attr.rs +++ b/compiler/rustc_passes/src/check_attr.rs @@ -701,6 +701,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> { }); } } + Target::Struct if self.tcx.features().struct_target_features => {} Target::Method(MethodKind::Trait { body: true } | MethodKind::Inherent) => {} // FIXME: #[target_feature] was previously erroneously allowed on statements and some // crates used this, so only emit a warning. @@ -720,11 +721,18 @@ impl<'tcx> CheckAttrVisitor<'tcx> { self.inline_attr_str_error_with_macro_def(hir_id, attr, "target_feature"); } _ => { - self.dcx().emit_err(errors::AttrShouldBeAppliedToFn { - attr_span: attr.span, - defn_span: span, - on_crate: hir_id == CRATE_HIR_ID, - }); + if self.tcx.features().struct_target_features { + self.dcx().emit_err(errors::AttrShouldBeAppliedToFnOrStruct { + attr_span: attr.span, + defn_span: span, + }); + } else { + self.dcx().emit_err(errors::AttrShouldBeAppliedToFn { + attr_span: attr.span, + defn_span: span, + on_crate: hir_id == CRATE_HIR_ID, + }); + } } } } diff --git a/compiler/rustc_passes/src/errors.rs b/compiler/rustc_passes/src/errors.rs index f9186d3089ab3..20f5e526a9faf 100644 --- a/compiler/rustc_passes/src/errors.rs +++ b/compiler/rustc_passes/src/errors.rs @@ -82,6 +82,15 @@ pub(crate) struct AttrShouldBeAppliedToFn { pub on_crate: bool, } +#[derive(Diagnostic)] +#[diag(passes_should_be_applied_to_fn_or_struct)] +pub(crate) struct AttrShouldBeAppliedToFnOrStruct { + #[primary_span] + pub attr_span: Span, + #[label] + pub defn_span: Span, +} + #[derive(Diagnostic)] #[diag(passes_should_be_applied_to_fn, code = E0739)] pub(crate) struct TrackedCallerWrongLocation { diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index cc3bda99a117b..ee6619aca4fb2 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -1919,6 +1919,7 @@ symbols! { stringify, struct_field_attributes, struct_inherit, + struct_target_features, struct_variant, structural_match, structural_peq, diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs index 824c25db07d2e..0b8b306d63bfb 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs @@ -464,6 +464,8 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { let is_target_feature_fn = if let ty::FnDef(def_id, _) = *leaf_trait_ref.skip_binder().self_ty().kind() { + // FIXME(struct_target_features): should a function that inherits + // target_features through arguments implement Fn traits? !self.tcx.codegen_fn_attrs(def_id).target_features.is_empty() } else { false diff --git a/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs b/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs index 20adda6f0de29..30693c04874d4 100644 --- a/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs +++ b/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs @@ -549,6 +549,8 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { // Provide an impl for suitable functions, rejecting `#[target_feature]` functions (RFC 2396). ty::FnDef(def_id, args) => { let tcx = self.tcx(); + // FIXME(struct_target_features): should a function that inherits target_features + // through an argument implement Fn traits? if tcx.fn_sig(def_id).skip_binder().is_fn_trait_compatible() && tcx.codegen_fn_attrs(def_id).target_features.is_empty() { diff --git a/src/doc/unstable-book/src/language-features/struct-target-features.md b/src/doc/unstable-book/src/language-features/struct-target-features.md new file mode 100644 index 0000000000000..e814fe9007240 --- /dev/null +++ b/src/doc/unstable-book/src/language-features/struct-target-features.md @@ -0,0 +1,7 @@ +# `struct_target_features` + +The tracking issue for this feature is: [#129107] + +[#129107]: https://github.com/rust-lang/rust/issues/129107 + +------------------------ diff --git a/tests/assembly/struct-target-features.rs b/tests/assembly/struct-target-features.rs new file mode 100644 index 0000000000000..b446c22db854c --- /dev/null +++ b/tests/assembly/struct-target-features.rs @@ -0,0 +1,31 @@ +//@ compile-flags: -O +//@ assembly-output: emit-asm +//@ only-x86_64 + +#![crate_type = "lib"] +#![feature(struct_target_features)] + +// Check that a struct_target_features type causes the compiler to effectively inline intrinsics. + +use std::arch::x86_64::*; + +#[target_feature(enable = "avx")] +struct Avx {} + +#[target_feature(enable = "fma")] +struct Fma {} + +pub fn add_simple(_: Avx, v: __m256) -> __m256 { + // CHECK-NOT: call + // CHECK: vaddps + unsafe { _mm256_add_ps(v, v) } +} + +pub fn add_fma_combined(_: &Avx, _: &Fma, v: __m256) -> (__m256, __m256) { + // CHECK-NOT: call + // CHECK-DAG: vaddps + let r1 = unsafe { _mm256_add_ps(v, v) }; + // CHECK-DAG: vfmadd213ps + let r2 = unsafe { _mm256_fmadd_ps(v, v, v) }; + (r1, r2) +} diff --git a/tests/ui/feature-gates/feature-gate-struct-target-features.rs b/tests/ui/feature-gates/feature-gate-struct-target-features.rs new file mode 100644 index 0000000000000..854948811469b --- /dev/null +++ b/tests/ui/feature-gates/feature-gate-struct-target-features.rs @@ -0,0 +1,4 @@ +#[target_feature(enable = "avx")] //~ ERROR attribute should be applied to a function definition +struct Avx {} + +fn main() {} diff --git a/tests/ui/feature-gates/feature-gate-struct-target-features.stderr b/tests/ui/feature-gates/feature-gate-struct-target-features.stderr new file mode 100644 index 0000000000000..1e18d3ee1e18d --- /dev/null +++ b/tests/ui/feature-gates/feature-gate-struct-target-features.stderr @@ -0,0 +1,10 @@ +error: attribute should be applied to a function definition + --> $DIR/feature-gate-struct-target-features.rs:1:1 + | +LL | #[target_feature(enable = "avx")] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | struct Avx {} + | ------------- not a function definition + +error: aborting due to 1 previous error + diff --git a/tests/ui/target-feature/auxiliary/struct-target-features-crate-dep.rs b/tests/ui/target-feature/auxiliary/struct-target-features-crate-dep.rs new file mode 100644 index 0000000000000..2c5ab9503b0d7 --- /dev/null +++ b/tests/ui/target-feature/auxiliary/struct-target-features-crate-dep.rs @@ -0,0 +1,6 @@ +#![feature(struct_target_features)] + +#[target_feature(enable = "avx")] +pub struct Avx {} + +pub struct NoFeatures {} diff --git a/tests/ui/target-feature/struct-target-features-crate.rs b/tests/ui/target-feature/struct-target-features-crate.rs new file mode 100644 index 0000000000000..84bc02b927939 --- /dev/null +++ b/tests/ui/target-feature/struct-target-features-crate.rs @@ -0,0 +1,23 @@ +//@ only-x86_64 +//@ aux-build: struct-target-features-crate-dep.rs +//@ check-pass +#![feature(target_feature_11)] + +extern crate struct_target_features_crate_dep; + +#[target_feature(enable = "avx")] +fn avx() {} + +fn f(_: struct_target_features_crate_dep::Avx) { + avx(); +} + +fn g(_: struct_target_features_crate_dep::NoFeatures) {} + +fn main() { + if is_x86_feature_detected!("avx") { + let avx = unsafe { struct_target_features_crate_dep::Avx {} }; + f(avx); + } + g(struct_target_features_crate_dep::NoFeatures {}); +} diff --git a/tests/ui/target-feature/struct-target-features.rs b/tests/ui/target-feature/struct-target-features.rs new file mode 100644 index 0000000000000..c74d0edad58ca --- /dev/null +++ b/tests/ui/target-feature/struct-target-features.rs @@ -0,0 +1,100 @@ +//@ only-x86_64 +#![feature(struct_target_features)] +//~^ WARNING the feature `struct_target_features` is incomplete and may not be safe to use and/or cause compiler crashes +#![feature(target_feature_11)] + +use std::arch::x86_64::*; + +#[target_feature(enable = "avx")] +struct Invalid(u32); + +#[target_feature(enable = "avx")] +struct Avx {} + +#[target_feature(enable = "sse")] +struct Sse(); + +#[target_feature(enable = "avx")] +fn avx() {} + +trait TFAssociatedType { + type Assoc; +} + +impl TFAssociatedType for () { + type Assoc = Avx; +} + +fn avx_self(_: <() as TFAssociatedType>::Assoc) { + avx(); +} + +fn avx_avx(_: Avx) { + avx(); +} + +extern "C" fn bad_fun(_: Avx) {} +//~^ ERROR cannot use a struct with target features in a function with non-Rust ABI + +#[inline(always)] +//~^ ERROR cannot use `#[inline(always)]` with `#[target_feature]` +fn inline_fun(_: Avx) {} +//~^ ERROR cannot use a struct with target features in a #[inline(always)] function + +trait Simd { + fn do_something(&self); +} + +impl Simd for Avx { + fn do_something(&self) { + unsafe { + println!("{:?}", _mm256_setzero_ps()); + } + } +} + +impl Simd for Sse { + fn do_something(&self) { + unsafe { + println!("{:?}", _mm_setzero_ps()); + } + } +} + +struct WithAvx { + #[allow(dead_code)] + avx: Avx, +} + +impl Simd for WithAvx { + fn do_something(&self) { + unsafe { + println!("{:?}", _mm256_setzero_ps()); + } + } +} + +#[inline(never)] +fn dosomething(simd: &S) { + simd.do_something(); +} + +fn avxfn(_: &Avx) { + // This is not unsafe because we already have the feature at function-level. + let _ = Avx {}; +} + +fn main() { + Avx {}; + //~^ ERROR initializing type `Avx` with `#[target_feature]` is unsafe and requires unsafe function or block [E0133] + + if is_x86_feature_detected!("avx") { + let avx = unsafe { Avx {} }; + avxfn(&avx); + dosomething(&avx); + dosomething(&WithAvx { avx }); + } + if is_x86_feature_detected!("sse") { + dosomething(&unsafe { Sse {} }) + } +} diff --git a/tests/ui/target-feature/struct-target-features.stderr b/tests/ui/target-feature/struct-target-features.stderr new file mode 100644 index 0000000000000..b4fbd557ab3e0 --- /dev/null +++ b/tests/ui/target-feature/struct-target-features.stderr @@ -0,0 +1,38 @@ +warning: the feature `struct_target_features` is incomplete and may not be safe to use and/or cause compiler crashes + --> $DIR/struct-target-features.rs:2:12 + | +LL | #![feature(struct_target_features)] + | ^^^^^^^^^^^^^^^^^^^^^^ + | + = note: see issue #129107 for more information + = note: `#[warn(incomplete_features)]` on by default + +error: cannot use a struct with target features in a function with non-Rust ABI + --> $DIR/struct-target-features.rs:36:1 + | +LL | extern "C" fn bad_fun(_: Avx) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: cannot use a struct with target features in a #[inline(always)] function + --> $DIR/struct-target-features.rs:41:1 + | +LL | fn inline_fun(_: Avx) {} + | ^^^^^^^^^^^^^^^^^^^^^ + +error: cannot use `#[inline(always)]` with `#[target_feature]` + --> $DIR/struct-target-features.rs:39:1 + | +LL | #[inline(always)] + | ^^^^^^^^^^^^^^^^^ + +error[E0133]: initializing type `Avx` with `#[target_feature]` is unsafe and requires unsafe function or block + --> $DIR/struct-target-features.rs:88:5 + | +LL | Avx {}; + | ^^^^^^ call to function with `#[target_feature]` + | + = note: the target features being enabled in the build configuration does not remove the requirement to list them in `#[target_feature]` + +error: aborting due to 4 previous errors; 1 warning emitted + +For more information about this error, try `rustc --explain E0133`. From 605a9d93c06742c1c5bb4162d22aa4e1068f880b Mon Sep 17 00:00:00 2001 From: Luca Versari Date: Tue, 10 Sep 2024 16:01:26 +0200 Subject: [PATCH 2/5] Un-querify struct_reachable_target_features --- .../rustc_codegen_ssa/src/codegen_attrs.rs | 57 ++++++++++--------- compiler/rustc_middle/src/query/mod.rs | 4 -- 2 files changed, 29 insertions(+), 32 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs index 8b492bf13eaca..81a33015bdf1e 100644 --- a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs +++ b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs @@ -600,26 +600,30 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs { } if let Some(sig) = fn_sig_outer() { + let mut additional_tf = vec![]; for ty in sig.skip_binder().inputs().skip_binder() { - let additional_tf = - tcx.struct_reachable_target_features(tcx.param_env(did.to_def_id()).and(*ty)); - // FIXME(struct_target_features): is this really necessary? - if !additional_tf.is_empty() && sig.skip_binder().abi() != abi::Abi::Rust { - tcx.dcx().span_err( - tcx.hir().span(tcx.local_def_id_to_hir_id(did)), - "cannot use a struct with target features in a function with non-Rust ABI", - ); - } - 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 - .target_features - .extend(additional_tf.iter().map(|tf| TargetFeature { implied: true, ..*tf })); + extend_with_struct_target_features( + tcx, + tcx.param_env(did.to_def_id()).and(*ty), + &mut additional_tf, + ) + } + // FIXME(struct_target_features): is this really necessary? + if !additional_tf.is_empty() && sig.skip_binder().abi() != abi::Abi::Rust { + tcx.dcx().span_err( + tcx.hir().span(tcx.local_def_id_to_hir_id(did)), + "cannot use a struct with target features in a function with non-Rust ABI", + ); } + 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 + .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 @@ -814,10 +818,11 @@ fn struct_target_features(tcx: TyCtxt<'_>, def_id: LocalDefId) -> &[TargetFeatur tcx.arena.alloc_slice(&features) } -fn struct_reachable_target_features<'tcx>( +fn extend_with_struct_target_features<'tcx>( tcx: TyCtxt<'tcx>, env: ty::ParamEnvAnd<'tcx, Ty<'tcx>>, -) -> &'tcx [TargetFeature] { + target_features: &mut Vec, +) { // Collect target features from types reachable from `env.value` by dereferencing a certain // number of references and resolving aliases. @@ -825,19 +830,16 @@ fn struct_reachable_target_features<'tcx>( if matches!(ty.kind(), ty::Alias(..)) { ty = match tcx.try_normalize_erasing_regions(env.param_env, ty) { Ok(ty) => ty, - Err(_) => return tcx.arena.alloc_slice(&[]), + Err(_) => return, }; } while let ty::Ref(_, inner, _) = ty.kind() { ty = *inner; } - let tf = if let ty::Adt(adt_def, ..) = ty.kind() { - tcx.struct_target_features(adt_def.did()) - } else { - &[] - }; - tcx.arena.alloc_slice(tf) + if let ty::Adt(adt_def, ..) = ty.kind() { + target_features.extend_from_slice(&tcx.struct_target_features(adt_def.did())); + } } pub(crate) fn provide(providers: &mut Providers) { @@ -845,7 +847,6 @@ pub(crate) fn provide(providers: &mut Providers) { codegen_fn_attrs, should_inherit_track_caller, struct_target_features, - struct_reachable_target_features, ..*providers }; } diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index 6f1b64fc72c12..09f9d280086f6 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -1261,10 +1261,6 @@ rustc_queries! { desc { |tcx| "computing target features for struct `{}`", tcx.def_path_str(def_id) } } - query struct_reachable_target_features(env: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> &'tcx [TargetFeature] { - desc { |tcx| "computing target features reachable from {}", env.value } - } - query asm_target_features(def_id: DefId) -> &'tcx FxIndexSet { desc { |tcx| "computing target features for inline asm of `{}`", tcx.def_path_str(def_id) } } From c9b01f0409d15557a2bf9b5604ce70334ff2f50f Mon Sep 17 00:00:00 2001 From: Luca Versari Date: Wed, 11 Sep 2024 00:24:36 +0200 Subject: [PATCH 3/5] Fix unnecessary restrictions in struct-target-features. Allow using struct-tf with functions with non-Rust ABI. Also allow converting struct-tf functions to function pointers / let them implement function traits. --- compiler/rustc_codegen_ssa/src/codegen_attrs.rs | 9 +-------- compiler/rustc_hir_typeck/src/coercion.rs | 13 ++++++++----- .../error_reporting/traits/fulfillment_errors.rs | 4 +--- .../src/traits/select/candidate_assembly.rs | 7 +++---- 4 files changed, 13 insertions(+), 20 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs index 81a33015bdf1e..3ebe3ec3ab1a7 100644 --- a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs +++ b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs @@ -13,7 +13,7 @@ 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, Ty, TyCtxt}; +use rustc_middle::ty::{self as ty, TyCtxt}; use rustc_session::parse::feature_err; use rustc_session::{Session, lint}; use rustc_span::symbol::Ident; @@ -608,13 +608,6 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs { &mut additional_tf, ) } - // FIXME(struct_target_features): is this really necessary? - if !additional_tf.is_empty() && sig.skip_binder().abi() != abi::Abi::Rust { - tcx.dcx().span_err( - tcx.hir().span(tcx.local_def_id_to_hir_id(did)), - "cannot use a struct with target features in a function with non-Rust ABI", - ); - } 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)), diff --git a/compiler/rustc_hir_typeck/src/coercion.rs b/compiler/rustc_hir_typeck/src/coercion.rs index 571fe50a088bd..05b6946b57521 100644 --- a/compiler/rustc_hir_typeck/src/coercion.rs +++ b/compiler/rustc_hir_typeck/src/coercion.rs @@ -920,12 +920,15 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> { return Err(TypeError::IntrinsicCast); } - // Safe `#[target_feature]` functions are not assignable to safe fn pointers (RFC 2396). - // FIXME(struct_target_features): should this be true also for functions that inherit - // target features from structs? - + // 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) + .target_features + .iter() + .any(|x| !x.implied) { return Err(TypeError::TargetFeatureCast(def_id)); } diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs index 0b8b306d63bfb..1f7e6c40ed1b3 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs @@ -464,9 +464,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { let is_target_feature_fn = if let ty::FnDef(def_id, _) = *leaf_trait_ref.skip_binder().self_ty().kind() { - // FIXME(struct_target_features): should a function that inherits - // target_features through arguments implement Fn traits? - !self.tcx.codegen_fn_attrs(def_id).target_features.is_empty() + self.tcx.codegen_fn_attrs(def_id).target_features.iter().any(|x| !x.implied) } else { false }; diff --git a/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs b/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs index 30693c04874d4..3035578696da2 100644 --- a/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs +++ b/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs @@ -546,13 +546,12 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { .push(FnPointerCandidate { fn_host_effect: self.tcx().consts.true_ }); } } - // Provide an impl for suitable functions, rejecting `#[target_feature]` functions (RFC 2396). + // Provide an impl for suitable functions, rejecting functions with explicit + // `#[target_feature]` attributes (RFC 2396). ty::FnDef(def_id, args) => { let tcx = self.tcx(); - // FIXME(struct_target_features): should a function that inherits target_features - // through an argument implement Fn traits? if tcx.fn_sig(def_id).skip_binder().is_fn_trait_compatible() - && tcx.codegen_fn_attrs(def_id).target_features.is_empty() + && !tcx.codegen_fn_attrs(def_id).target_features.iter().any(|x| !x.implied) { candidates.vec.push(FnPointerCandidate { fn_host_effect: tcx From e47b5ae4793588512c8afec134651a394c9ee140 Mon Sep 17 00:00:00 2001 From: Luca Versari Date: Fri, 13 Sep 2024 00:35:23 +0200 Subject: [PATCH 4/5] Require annotating struct-tf functions explicitly. --- compiler/rustc_codegen_ssa/messages.ftl | 2 +- .../rustc_codegen_ssa/src/codegen_attrs.rs | 14 +++++++-- .../rustc_codegen_ssa/src/target_features.rs | 11 ++++++- .../src/middle/codegen_fn_attrs.rs | 3 ++ .../rustc_mir_build/src/check_unsafety.rs | 13 +++----- compiler/rustc_span/src/symbol.rs | 1 + tests/assembly/struct-target-features.rs | 8 +++++ .../trait-impl.stderr | 4 +-- .../struct-target-features-crate.rs | 3 ++ .../struct-target-features-crate.stderr | 11 +++++++ .../target-feature/struct-target-features.rs | 31 ++++++------------- .../struct-target-features.stderr | 14 +++------ 12 files changed, 68 insertions(+), 47 deletions(-) create mode 100644 tests/ui/target-feature/struct-target-features-crate.stderr diff --git a/compiler/rustc_codegen_ssa/messages.ftl b/compiler/rustc_codegen_ssa/messages.ftl index d07274920feaf..b689ac74c6ba5 100644 --- a/compiler/rustc_codegen_ssa/messages.ftl +++ b/compiler/rustc_codegen_ssa/messages.ftl @@ -243,7 +243,7 @@ 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 +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 diff --git a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs index 3ebe3ec3ab1a7..606d72498b405 100644 --- a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs +++ b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs @@ -250,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 @@ -289,6 +294,7 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs { attr, supported_target_features, &mut codegen_fn_attrs.target_features, + Some(&mut codegen_fn_attrs.target_features_from_args), ); } sym::linkage => { @@ -599,7 +605,9 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs { } } - if let Some(sig) = fn_sig_outer() { + 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( @@ -806,7 +814,7 @@ fn struct_target_features(tcx: TyCtxt<'_>, def_id: LocalDefId) -> &[TargetFeatur 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); + from_target_feature(tcx, attr, supported_features, &mut features, None); } tcx.arena.alloc_slice(&features) } diff --git a/compiler/rustc_codegen_ssa/src/target_features.rs b/compiler/rustc_codegen_ssa/src/target_features.rs index dfe8fd616e4a2..b0a34f06ecfc1 100644 --- a/compiler/rustc_codegen_ssa/src/target_features.rs +++ b/compiler/rustc_codegen_ssa/src/target_features.rs @@ -20,6 +20,7 @@ pub(crate) fn from_target_feature( attr: &ast::Attribute, supported_target_features: &UnordMap>, target_features: &mut Vec, + mut features_from_args: Option<&mut bool>, ) { let Some(list) = attr.meta_item_list() else { return }; let bad_item = |span| { @@ -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()); @@ -144,7 +153,7 @@ fn asm_target_features(tcx: TyCtxt<'_>, did: DefId) -> &FxIndexSet { 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) { diff --git a/compiler/rustc_middle/src/middle/codegen_fn_attrs.rs b/compiler/rustc_middle/src/middle/codegen_fn_attrs.rs index 71b17aaad2cf3..71bf110c063a4 100644 --- a/compiler/rustc_middle/src/middle/codegen_fn_attrs.rs +++ b/compiler/rustc_middle/src/middle/codegen_fn_attrs.rs @@ -49,6 +49,8 @@ pub struct CodegenFnAttrs { /// The `#[patchable_function_entry(...)]` attribute. Indicates how many nops should be around /// the function entry. pub patchable_function_entry: Option, + /// Whether the target features can be extended through the arguments of the function. + pub target_features_from_args: bool, } #[derive(Copy, Clone, Debug, TyEncodable, TyDecodable, HashStable)] @@ -156,6 +158,7 @@ impl CodegenFnAttrs { instruction_set: None, alignment: None, patchable_function_entry: None, + target_features_from_args: false, } } diff --git a/compiler/rustc_mir_build/src/check_unsafety.rs b/compiler/rustc_mir_build/src/check_unsafety.rs index f393fabccbd26..a1beaf24f3284 100644 --- a/compiler/rustc_mir_build/src/check_unsafety.rs +++ b/compiler/rustc_mir_build/src/check_unsafety.rs @@ -589,14 +589,11 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> { .copied() .filter(|feature| missing.contains(feature)) .collect(); - self.requires_unsafe( - expr.span, - ConstructingTargetFeaturesTypeWith { - adt: adt_def.did(), - missing, - build_enabled, - }, - ); + self.requires_unsafe(expr.span, ConstructingTargetFeaturesTypeWith { + adt: adt_def.did(), + missing, + build_enabled, + }); } } } diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index ee6619aca4fb2..6868038f327b9 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -946,6 +946,7 @@ symbols! { frem_algebraic, frem_fast, from, + from_args, from_desugaring, from_fn, from_iter, diff --git a/tests/assembly/struct-target-features.rs b/tests/assembly/struct-target-features.rs index b446c22db854c..718c355a4c0dd 100644 --- a/tests/assembly/struct-target-features.rs +++ b/tests/assembly/struct-target-features.rs @@ -15,12 +15,20 @@ struct Avx {} #[target_feature(enable = "fma")] struct Fma {} +#[target_feature(from_args)] pub fn add_simple(_: Avx, v: __m256) -> __m256 { // CHECK-NOT: call // CHECK: vaddps unsafe { _mm256_add_ps(v, v) } } +// Test that the features don't get inherited from the arguments without the attribute. +pub fn add_simple_noattr(_: Avx, v: __m256) -> __m256 { + // CHECK: call + unsafe { _mm256_add_ps(v, v) } +} + +#[target_feature(from_args)] pub fn add_fma_combined(_: &Avx, _: &Fma, v: __m256) -> (__m256, __m256) { // CHECK-NOT: call // CHECK-DAG: vaddps diff --git a/tests/ui/rfcs/rfc-2396-target_feature-11/trait-impl.stderr b/tests/ui/rfcs/rfc-2396-target_feature-11/trait-impl.stderr index 00efbb52f159b..989196b757474 100644 --- a/tests/ui/rfcs/rfc-2396-target_feature-11/trait-impl.stderr +++ b/tests/ui/rfcs/rfc-2396-target_feature-11/trait-impl.stderr @@ -1,4 +1,4 @@ -error: `#[target_feature(..)]` cannot be applied to safe trait method +error: `#[target_feature(enable = ..)]` cannot be applied to safe trait method --> $DIR/trait-impl.rs:13:5 | LL | #[target_feature(enable = "sse2")] @@ -7,7 +7,7 @@ LL | LL | fn foo(&self) {} | ------------- not an `unsafe` function -error: `#[target_feature(..)]` cannot be applied to safe trait method +error: `#[target_feature(enable = ..)]` cannot be applied to safe trait method --> $DIR/trait-impl.rs:22:5 | LL | #[target_feature(enable = "sse2")] diff --git a/tests/ui/target-feature/struct-target-features-crate.rs b/tests/ui/target-feature/struct-target-features-crate.rs index 84bc02b927939..bd18762e6b8ee 100644 --- a/tests/ui/target-feature/struct-target-features-crate.rs +++ b/tests/ui/target-feature/struct-target-features-crate.rs @@ -2,12 +2,15 @@ //@ aux-build: struct-target-features-crate-dep.rs //@ check-pass #![feature(target_feature_11)] +#![feature(struct_target_features)] +//~^ WARNING the feature `struct_target_features` is incomplete and may not be safe to use and/or cause compiler crashes extern crate struct_target_features_crate_dep; #[target_feature(enable = "avx")] fn avx() {} +#[target_feature(from_args)] fn f(_: struct_target_features_crate_dep::Avx) { avx(); } diff --git a/tests/ui/target-feature/struct-target-features-crate.stderr b/tests/ui/target-feature/struct-target-features-crate.stderr new file mode 100644 index 0000000000000..a284ec14ee10d --- /dev/null +++ b/tests/ui/target-feature/struct-target-features-crate.stderr @@ -0,0 +1,11 @@ +warning: the feature `struct_target_features` is incomplete and may not be safe to use and/or cause compiler crashes + --> $DIR/struct-target-features-crate.rs:5:12 + | +LL | #![feature(struct_target_features)] + | ^^^^^^^^^^^^^^^^^^^^^^ + | + = note: see issue #129107 for more information + = note: `#[warn(incomplete_features)]` on by default + +warning: 1 warning emitted + diff --git a/tests/ui/target-feature/struct-target-features.rs b/tests/ui/target-feature/struct-target-features.rs index c74d0edad58ca..4146f33ed93f4 100644 --- a/tests/ui/target-feature/struct-target-features.rs +++ b/tests/ui/target-feature/struct-target-features.rs @@ -25,19 +25,22 @@ impl TFAssociatedType for () { type Assoc = Avx; } +#[target_feature(from_args)] fn avx_self(_: <() as TFAssociatedType>::Assoc) { avx(); } +#[target_feature(from_args)] fn avx_avx(_: Avx) { avx(); } +#[target_feature(from_args)] extern "C" fn bad_fun(_: Avx) {} -//~^ ERROR cannot use a struct with target features in a function with non-Rust ABI #[inline(always)] //~^ ERROR cannot use `#[inline(always)]` with `#[target_feature]` +#[target_feature(from_args)] fn inline_fun(_: Avx) {} //~^ ERROR cannot use a struct with target features in a #[inline(always)] function @@ -46,6 +49,7 @@ trait Simd { } impl Simd for Avx { + #[target_feature(from_args)] fn do_something(&self) { unsafe { println!("{:?}", _mm256_setzero_ps()); @@ -54,6 +58,7 @@ impl Simd for Avx { } impl Simd for Sse { + #[target_feature(from_args)] fn do_something(&self) { unsafe { println!("{:?}", _mm_setzero_ps()); @@ -61,24 +66,7 @@ impl Simd for Sse { } } -struct WithAvx { - #[allow(dead_code)] - avx: Avx, -} - -impl Simd for WithAvx { - fn do_something(&self) { - unsafe { - println!("{:?}", _mm256_setzero_ps()); - } - } -} - -#[inline(never)] -fn dosomething(simd: &S) { - simd.do_something(); -} - +#[target_feature(from_args)] fn avxfn(_: &Avx) { // This is not unsafe because we already have the feature at function-level. let _ = Avx {}; @@ -91,10 +79,9 @@ fn main() { if is_x86_feature_detected!("avx") { let avx = unsafe { Avx {} }; avxfn(&avx); - dosomething(&avx); - dosomething(&WithAvx { avx }); + avx.do_something(); } if is_x86_feature_detected!("sse") { - dosomething(&unsafe { Sse {} }) + unsafe { Sse {} }.do_something(); } } diff --git a/tests/ui/target-feature/struct-target-features.stderr b/tests/ui/target-feature/struct-target-features.stderr index b4fbd557ab3e0..6e016f6c4068f 100644 --- a/tests/ui/target-feature/struct-target-features.stderr +++ b/tests/ui/target-feature/struct-target-features.stderr @@ -7,32 +7,26 @@ LL | #![feature(struct_target_features)] = note: see issue #129107 for more information = note: `#[warn(incomplete_features)]` on by default -error: cannot use a struct with target features in a function with non-Rust ABI - --> $DIR/struct-target-features.rs:36:1 - | -LL | extern "C" fn bad_fun(_: Avx) {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - error: cannot use a struct with target features in a #[inline(always)] function - --> $DIR/struct-target-features.rs:41:1 + --> $DIR/struct-target-features.rs:44:1 | LL | fn inline_fun(_: Avx) {} | ^^^^^^^^^^^^^^^^^^^^^ error: cannot use `#[inline(always)]` with `#[target_feature]` - --> $DIR/struct-target-features.rs:39:1 + --> $DIR/struct-target-features.rs:41:1 | LL | #[inline(always)] | ^^^^^^^^^^^^^^^^^ error[E0133]: initializing type `Avx` with `#[target_feature]` is unsafe and requires unsafe function or block - --> $DIR/struct-target-features.rs:88:5 + --> $DIR/struct-target-features.rs:76:5 | LL | Avx {}; | ^^^^^^ call to function with `#[target_feature]` | = note: the target features being enabled in the build configuration does not remove the requirement to list them in `#[target_feature]` -error: aborting due to 4 previous errors; 1 warning emitted +error: aborting due to 3 previous errors; 1 warning emitted For more information about this error, try `rustc --explain E0133`. From b3628ae95a23100f2dfb195463af9e00ddd44667 Mon Sep 17 00:00:00 2001 From: Luca Versari Date: Tue, 10 Sep 2024 14:33:28 +0200 Subject: [PATCH 5/5] Implement struct_target_features for generic functions. --- compiler/rustc_codegen_gcc/src/attributes.rs | 7 +- compiler/rustc_codegen_llvm/src/attributes.rs | 6 +- .../rustc_codegen_ssa/src/codegen_attrs.rs | 38 ++------- .../rustc_codegen_ssa/src/target_features.rs | 2 +- .../rustc_hir_analysis/src/check/entry.rs | 2 +- compiler/rustc_hir_typeck/src/coercion.rs | 2 +- .../src/middle/codegen_fn_attrs.rs | 84 ++++++++++++++++++- compiler/rustc_middle/src/ty/context.rs | 2 +- .../rustc_mir_build/src/check_unsafety.rs | 4 +- compiler/rustc_mir_transform/src/inline.rs | 15 +++- .../traits/fulfillment_errors.rs | 2 +- .../src/traits/select/candidate_assembly.rs | 4 +- src/tools/miri/src/machine.rs | 8 +- tests/assembly/struct-target-features.rs | 12 +++ 14 files changed, 137 insertions(+), 51 deletions(-) diff --git a/compiler/rustc_codegen_gcc/src/attributes.rs b/compiler/rustc_codegen_gcc/src/attributes.rs index d20e13e15b944..a9f5c7f201877 100644 --- a/compiler/rustc_codegen_gcc/src/attributes.rs +++ b/compiler/rustc_codegen_gcc/src/attributes.rs @@ -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 crate::context::CodegenCx; use crate::gcc_util::to_gcc_features; @@ -70,8 +70,9 @@ pub fn from_fn_attrs<'gcc, 'tcx>( } } - let mut function_features = codegen_fn_attrs - .target_features + let function_features = + codegen_fn_attrs.target_features_for_instance(cx.tcx, ParamEnv::reveal_all(), instance); + let mut function_features = function_features .iter() .map(|features| features.name.as_str()) .flat_map(|feat| to_gcc_features(cx.tcx.sess, feat).into_iter()) diff --git a/compiler/rustc_codegen_llvm/src/attributes.rs b/compiler/rustc_codegen_llvm/src/attributes.rs index 2c5ec9dad59f1..edba23e47575a 100644 --- a/compiler/rustc_codegen_llvm/src/attributes.rs +++ b/compiler/rustc_codegen_llvm/src/attributes.rs @@ -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_target::spec::{FramePointer, SanitizerSet, StackProbeType, StackProtector}; use smallvec::SmallVec; @@ -499,7 +499,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::>(); + 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::>(); let function_features = function_features .iter() diff --git a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs index 606d72498b405..25d5a4c9b624d 100644 --- a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs +++ b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs @@ -10,6 +10,7 @@ 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, TargetFeature, + extend_with_struct_target_features, }; use rustc_middle::mir::mono::Linkage; use rustc_middle::query::Providers; @@ -293,7 +294,7 @@ 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), ); } @@ -600,8 +601,8 @@ 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()); } } @@ -623,7 +624,7 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs { ); } codegen_fn_attrs - .target_features + .def_target_features .extend(additional_tf.iter().map(|tf| TargetFeature { implied: true, ..*tf })); } @@ -631,7 +632,8 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs { // 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( @@ -697,7 +699,7 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs { if let Some(features) = check_tied_features( tcx.sess, &codegen_fn_attrs - .target_features + .def_target_features .iter() .map(|features| (features.name.as_str(), true)) .collect(), @@ -819,30 +821,6 @@ fn struct_target_features(tcx: TyCtxt<'_>, def_id: LocalDefId) -> &[TargetFeatur tcx.arena.alloc_slice(&features) } -fn extend_with_struct_target_features<'tcx>( - tcx: TyCtxt<'tcx>, - env: ty::ParamEnvAnd<'tcx, Ty<'tcx>>, - target_features: &mut Vec, -) { - // Collect target features from types reachable from `env.value` by dereferencing a certain - // number of references and resolving aliases. - - let mut ty = env.value; - if matches!(ty.kind(), ty::Alias(..)) { - ty = match tcx.try_normalize_erasing_regions(env.param_env, ty) { - Ok(ty) => ty, - Err(_) => return, - }; - } - while let ty::Ref(_, inner, _) = ty.kind() { - ty = *inner; - } - - if let ty::Adt(adt_def, ..) = ty.kind() { - target_features.extend_from_slice(&tcx.struct_target_features(adt_def.did())); - } -} - pub(crate) fn provide(providers: &mut Providers) { *providers = Providers { codegen_fn_attrs, diff --git a/compiler/rustc_codegen_ssa/src/target_features.rs b/compiler/rustc_codegen_ssa/src/target_features.rs index b0a34f06ecfc1..e139a712082e6 100644 --- a/compiler/rustc_codegen_ssa/src/target_features.rs +++ b/compiler/rustc_codegen_ssa/src/target_features.rs @@ -137,7 +137,7 @@ fn asm_target_features(tcx: TyCtxt<'_>, did: DefId) -> &FxIndexSet { 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) => { diff --git a/compiler/rustc_hir_analysis/src/check/entry.rs b/compiler/rustc_hir_analysis/src/check/entry.rs index 7da2cd93d4e01..2fdfe92fe1fb1 100644 --- a/compiler/rustc_hir_analysis/src/check/entry.rs +++ b/compiler/rustc_hir_analysis/src/check/entry.rs @@ -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 diff --git a/compiler/rustc_hir_typeck/src/coercion.rs b/compiler/rustc_hir_typeck/src/coercion.rs index 05b6946b57521..3ad1f62663c6d 100644 --- a/compiler/rustc_hir_typeck/src/coercion.rs +++ b/compiler/rustc_hir_typeck/src/coercion.rs @@ -926,7 +926,7 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> { && self .tcx .codegen_fn_attrs(def_id) - .target_features + .def_target_features .iter() .any(|x| !x.implied) { diff --git a/compiler/rustc_middle/src/middle/codegen_fn_attrs.rs b/compiler/rustc_middle/src/middle/codegen_fn_attrs.rs index 71bf110c063a4..3dd3da689b6dd 100644 --- a/compiler/rustc_middle/src/middle/codegen_fn_attrs.rs +++ b/compiler/rustc_middle/src/middle/codegen_fn_attrs.rs @@ -5,6 +5,7 @@ use rustc_target::abi::Align; use rustc_target::spec::SanitizerSet; use crate::mir::mono::Linkage; +use crate::ty::{self, Instance, ParamEnv, Ty, TyCtxt}; #[derive(Clone, TyEncodable, TyDecodable, HashStable, Debug)] pub struct CodegenFnAttrs { @@ -28,7 +29,7 @@ pub struct CodegenFnAttrs { pub link_ordinal: Option, /// All the target features that are enabled for this function. Some features might be enabled /// implicitly. - pub target_features: Vec, + pub def_target_features: Vec, /// The `#[linkage = "..."]` attribute on Rust-defined items and the value we found. pub linkage: Option, /// The `#[linkage = "..."]` attribute on foreign items and the value we found. @@ -139,6 +140,30 @@ bitflags::bitflags! { } rustc_data_structures::external_bitflags_debug! { CodegenFnAttrFlags } +pub fn extend_with_struct_target_features<'tcx>( + tcx: TyCtxt<'tcx>, + env: ty::ParamEnvAnd<'tcx, Ty<'tcx>>, + target_features: &mut Vec, +) { + // Collect target features from types reachable from `env.value` by dereferencing a certain + // number of references and resolving aliases. + + let mut ty = env.value; + if matches!(ty.kind(), ty::Alias(..)) { + ty = match tcx.try_normalize_erasing_regions(env.param_env, ty) { + Ok(ty) => ty, + Err(_) => return, + }; + } + while let ty::Ref(_, inner, _) = ty.kind() { + ty = *inner; + } + + if let ty::Adt(adt_def, ..) = ty.kind() { + target_features.extend_from_slice(&tcx.struct_target_features(adt_def.did())); + } +} + impl CodegenFnAttrs { pub const EMPTY: &'static Self = &Self::new(); @@ -150,7 +175,7 @@ impl CodegenFnAttrs { export_name: None, link_name: None, link_ordinal: None, - target_features: vec![], + def_target_features: vec![], linkage: None, import_linkage: None, link_section: None, @@ -177,4 +202,59 @@ impl CodegenFnAttrs { Some(_) => true, } } + + pub fn target_features_for_instance<'tcx>( + &self, + tcx: TyCtxt<'tcx>, + param_env: ParamEnv<'tcx>, + instance: Instance<'tcx>, + ) -> Vec { + if !self.target_features_from_args { + return self.def_target_features.clone(); + } + let inputs = match tcx.type_of(instance.def_id()).skip_binder().kind() { + ty::Closure(..) => { + let closure = instance.args.as_closure(); + let mut inputs = + tcx.instantiate_bound_regions_with_erased(closure.sig()).inputs().to_vec(); + inputs.extend(closure.upvar_tys()); + inputs + } + ty::CoroutineClosure(..) => { + let closure = instance.args.as_coroutine_closure(); + // FIXME: might be missing inputs to the closure + closure.upvar_tys().to_vec() + } + ty::Coroutine(..) => { + let coro = instance.args.as_coroutine(); + coro.upvar_tys().to_vec() + } + _ => { + let ty = match tcx.try_instantiate_and_normalize_erasing_regions( + instance.args, + param_env, + tcx.type_of(instance.def_id()), + ) { + Ok(ty) => ty, + Err(_) => { + return self.def_target_features.clone(); + } + }; + let sig = tcx.instantiate_bound_regions_with_erased(ty.fn_sig(tcx)); + sig.inputs().to_vec() + } + }; + let mut additional_features = vec![]; + for input in inputs { + extend_with_struct_target_features(tcx, param_env.and(input), &mut additional_features); + } + if additional_features.is_empty() { + self.def_target_features.clone() + } else { + additional_features.extend_from_slice(&self.def_target_features); + additional_features.sort_by_key(|a| (a.name, a.implied)); + additional_features.dedup_by_key(|a| a.name); + additional_features + } + } } diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index 50e7be82a79a3..e70e26033aeea 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -364,7 +364,7 @@ impl<'tcx> Interner for TyCtxt<'tcx> { } fn has_target_features(self, def_id: DefId) -> bool { - !self.codegen_fn_attrs(def_id).target_features.is_empty() + !self.codegen_fn_attrs(def_id).def_target_features.is_empty() } fn require_lang_item(self, lang_item: TraitSolverLangItem) -> DefId { diff --git a/compiler/rustc_mir_build/src/check_unsafety.rs b/compiler/rustc_mir_build/src/check_unsafety.rs index a1beaf24f3284..6487c5a51c6f7 100644 --- a/compiler/rustc_mir_build/src/check_unsafety.rs +++ b/compiler/rustc_mir_build/src/check_unsafety.rs @@ -476,7 +476,7 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> { // Implicit target features are OK because they are either a consequence of some // explicit target feature (which is checked to be present in the caller) or // come from a witness argument. - let callee_features = &self.tcx.codegen_fn_attrs(func_did).target_features; + let callee_features = &self.tcx.codegen_fn_attrs(func_did).def_target_features; if !self.tcx.sess.target.options.is_like_wasm && !callee_features.iter().all(|feature| { feature.implied @@ -1143,7 +1143,7 @@ pub(crate) fn check_unsafety(tcx: TyCtxt<'_>, def: LocalDefId) { SafetyContext::Safe } }); - let body_target_features = &tcx.body_codegen_attrs(def.to_def_id()).target_features; + let body_target_features = &tcx.body_codegen_attrs(def.to_def_id()).def_target_features; let mut warnings = Vec::new(); let mut visitor = UnsafetyVisitor { tcx, diff --git a/compiler/rustc_mir_transform/src/inline.rs b/compiler/rustc_mir_transform/src/inline.rs index c9f24764cc2a1..b101d35461b98 100644 --- a/compiler/rustc_mir_transform/src/inline.rs +++ b/compiler/rustc_mir_transform/src/inline.rs @@ -470,8 +470,19 @@ impl<'tcx> Inliner<'tcx> { return Err("incompatible instruction set"); } - let callee_feature_names = callee_attrs.target_features.iter().map(|f| f.name); - let this_feature_names = self.codegen_fn_attrs.target_features.iter().map(|f| f.name); + if callee_attrs.target_features_from_args || self.codegen_fn_attrs.target_features_from_args + { + // Since these functions inherit features from their arguments and might be + // non-fully-instantiated generics, we give up MIR inlining. + // FIXME: check if these are indeed non-fully-instantiated generics. + // FIXME: we actually don't need to check target_features_from_args in the *caller* + // once #127731 lands and is completed for all targets. Relatedly, we also won't need + // to check equality below. + return Err("using #[target_feature(from_args)]"); + } + + let callee_feature_names = callee_attrs.def_target_features.iter().map(|f| f.name); + let this_feature_names = self.codegen_fn_attrs.def_target_features.iter().map(|f| f.name); if callee_feature_names.ne(this_feature_names) { // In general it is not correct to inline a callee with target features that are a // subset of the caller. This is because the callee might contain calls, and the ABI of diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs index 1f7e6c40ed1b3..fd3238c59317c 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs @@ -464,7 +464,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { let is_target_feature_fn = if let ty::FnDef(def_id, _) = *leaf_trait_ref.skip_binder().self_ty().kind() { - self.tcx.codegen_fn_attrs(def_id).target_features.iter().any(|x| !x.implied) + self.tcx.codegen_fn_attrs(def_id).def_target_features.iter().any(|x| !x.implied) } else { false }; diff --git a/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs b/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs index 3035578696da2..ff5f0465dad73 100644 --- a/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs +++ b/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs @@ -482,7 +482,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { ty::FnDef(def_id, _) => { let tcx = self.tcx(); if tcx.fn_sig(def_id).skip_binder().is_fn_trait_compatible() - && tcx.codegen_fn_attrs(def_id).target_features.is_empty() + && tcx.codegen_fn_attrs(def_id).def_target_features.is_empty() { candidates.vec.push(AsyncClosureCandidate); } @@ -551,7 +551,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { ty::FnDef(def_id, args) => { let tcx = self.tcx(); if tcx.fn_sig(def_id).skip_binder().is_fn_trait_compatible() - && !tcx.codegen_fn_attrs(def_id).target_features.iter().any(|x| !x.implied) + && !tcx.codegen_fn_attrs(def_id).def_target_features.iter().any(|x| !x.implied) { candidates.vec.push(FnPointerCandidate { fn_host_effect: tcx diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index b9cebcfe9cd82..2eaa4c677e07a 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -15,7 +15,9 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_data_structures::static_assert_size; use rustc_middle::mir; use rustc_middle::query::TyCtxtAt; -use rustc_middle::ty::layout::{HasTyCtxt, LayoutCx, LayoutError, LayoutOf, TyAndLayout}; +use rustc_middle::ty::layout::{ + HasParamEnv, HasTyCtxt, LayoutCx, LayoutError, LayoutOf, TyAndLayout, +}; use rustc_middle::ty::{self, Instance, Ty, TyCtxt}; use rustc_session::config::InliningThreshold; use rustc_span::def_id::{CrateNum, DefId}; @@ -964,12 +966,12 @@ impl<'tcx> Machine<'tcx> for MiriMachine<'tcx> { ) -> InterpResult<'tcx> { let attrs = ecx.tcx.codegen_fn_attrs(instance.def_id()); if attrs - .target_features + .target_features_for_instance(ecx.tcx.tcx, ecx.param_env(), instance) .iter() .any(|feature| !ecx.tcx.sess.target_features.contains(&feature.name)) { let unavailable = attrs - .target_features + .target_features_for_instance(ecx.tcx.tcx, ecx.param_env(), instance) .iter() .filter(|&feature| { !feature.implied && !ecx.tcx.sess.target_features.contains(&feature.name) diff --git a/tests/assembly/struct-target-features.rs b/tests/assembly/struct-target-features.rs index 718c355a4c0dd..0d9d6e741b0cb 100644 --- a/tests/assembly/struct-target-features.rs +++ b/tests/assembly/struct-target-features.rs @@ -37,3 +37,15 @@ pub fn add_fma_combined(_: &Avx, _: &Fma, v: __m256) -> (__m256, __m256) { let r2 = unsafe { _mm256_fmadd_ps(v, v, v) }; (r1, r2) } + +#[target_feature(from_args)] +fn add_generic(_: S, v: __m256) -> __m256 { + // CHECK-NOT: call + // CHECK: vaddps + unsafe { _mm256_add_ps(v, v) } +} + +pub fn add_using_generic(v: __m256) -> __m256 { + assert!(is_x86_feature_detected!("avx")); + add_generic(unsafe { Avx {} }, v) +}