From 177eb792e8b2b9dd4d3af2ae170495ce390b7202 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 | 107 +++++++++++--- 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 | 52 ++++++- .../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, 632 insertions(+), 38 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 e99c3a462711c..583fae66d916e 100644 --- a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs +++ b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs @@ -1,23 +1,23 @@ -use rustc_ast::{MetaItemKind, NestedMetaItem, ast, attr}; -use rustc_attr::{InlineAttr, InstructionSetAttr, OptimizeAttr, list_contains_name}; +use rustc_ast::{ast, attr, MetaItemKind, NestedMetaItem}; +use rustc_attr::{list_contains_name, InlineAttr, InstructionSetAttr, OptimizeAttr}; use rustc_errors::codes::*; -use rustc_errors::{DiagMessage, SubdiagMessage, struct_span_code_err}; +use rustc_errors::{struct_span_code_err, DiagMessage, SubdiagMessage}; use rustc_hir as hir; use rustc_hir::def::DefKind; -use rustc_hir::def_id::{DefId, LOCAL_CRATE, LocalDefId}; +use rustc_hir::def_id::{DefId, LocalDefId, LOCAL_CRATE}; use rustc_hir::weak_lang_items::WEAK_LANG_ITEMS; -use rustc_hir::{LangItem, lang_items}; +use rustc_hir::{lang_items, LangItem}; 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::lint; use rustc_session::parse::feature_err; use rustc_span::symbol::Ident; -use rustc_span::{Span, sym}; -use rustc_target::spec::{SanitizerSet, abi}; +use rustc_span::{sym, Span}; +use rustc_target::spec::{abi, SanitizerSet}; use crate::errors; use crate::target_features::{check_target_feature_trait_unsafe, from_target_feature}; @@ -78,6 +78,17 @@ 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. @@ -85,16 +96,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 { @@ -595,7 +602,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. @@ -738,6 +768,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 380e36fe40566..f575b8681acf2 100644 --- a/compiler/rustc_feature/src/unstable.rs +++ b/compiler/rustc_feature/src/unstable.rs @@ -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. 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 fb462eec1b9aa..dedc4a18fd476 100644 --- a/compiler/rustc_hir_typeck/src/coercion.rs +++ b/compiler/rustc_hir_typeck/src/coercion.rs @@ -910,6 +910,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 610c682d3a461..5e4470bab1f73 100644 --- a/compiler/rustc_metadata/src/rmeta/encoder.rs +++ b/compiler/rustc_metadata/src/rmeta/encoder.rs @@ -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); 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..fa5c14882d5e3 100644 --- a/compiler/rustc_mir_build/messages.ftl +++ b/compiler/rustc_mir_build/messages.ftl @@ -116,15 +116,56 @@ mir_build_extern_static_requires_unsafe_unsafe_op_in_unsafe_fn_allowed = mir_build_inform_irrefutable = `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant mir_build_initializing_type_with_requires_unsafe = - initializing type with `rustc_layout_scalar_valid_range` attr is unsafe and requires unsafe block - .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 + call to function `{$function}` 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_requires_unsafe_unsafe_op_in_unsafe_fn_allowed = initializing type with `rustc_layout_scalar_valid_range` attr is unsafe and requires unsafe function or block .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 +429,11 @@ 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 with `target_feature` attr is unsafe and requires unsafe block + .note = this struct can only be constructed if the corresponding `target_feature`s are available + .label = initializing type with `target_feature` attr + 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 5369f54afb908..6359a376a059f 100644 --- a/compiler/rustc_passes/messages.ftl +++ b/compiler/rustc_passes/messages.ftl @@ -675,6 +675,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 b3334bb70aa8d..cd579e382c5d1 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 29a087bf75975..c5bad36103269 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 1527600e764c0..601f0492b8393 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -1913,6 +1913,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 084b61115dbcf..d1a329f5d08c8 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 72cebcc15085e4cb6d26e1067e3690f4dae43db4 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 583fae66d916e..b28b7086ff2fd 100644 --- a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs +++ b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs @@ -603,26 +603,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 @@ -777,10 +781,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. @@ -788,19 +793,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) { @@ -808,7 +810,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 b43c5d362ea033c65d4c5a45f198c8a6be9e4aa1 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 | 7 ------- 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, 12 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs index b28b7086ff2fd..705c1e04ae2c8 100644 --- a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs +++ b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs @@ -611,13 +611,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 dedc4a18fd476..fcc01a0a267b5 100644 --- a/compiler/rustc_hir_typeck/src/coercion.rs +++ b/compiler/rustc_hir_typeck/src/coercion.rs @@ -909,12 +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). - // 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 d1a329f5d08c8..40e227d7892d8 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 5ae19573c3aed84cf2da3c4da0378a5a514fb893 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 | 34 +++++++++++-------- .../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, 76 insertions(+), 59 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 f02b0f7267430..589a7d054a4e3 100644 --- a/compiler/rustc_codegen_ssa/messages.ftl +++ b/compiler/rustc_codegen_ssa/messages.ftl @@ -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 diff --git a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs index 705c1e04ae2c8..85d86cee17845 100644 --- a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs +++ b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs @@ -1,12 +1,12 @@ -use rustc_ast::{ast, attr, MetaItemKind, NestedMetaItem}; -use rustc_attr::{list_contains_name, InlineAttr, InstructionSetAttr, OptimizeAttr}; +use rustc_ast::{MetaItemKind, NestedMetaItem, ast, attr}; +use rustc_attr::{InlineAttr, InstructionSetAttr, OptimizeAttr, list_contains_name}; use rustc_errors::codes::*; -use rustc_errors::{struct_span_code_err, DiagMessage, SubdiagMessage}; +use rustc_errors::{DiagMessage, SubdiagMessage, struct_span_code_err}; use rustc_hir as hir; use rustc_hir::def::DefKind; -use rustc_hir::def_id::{DefId, LocalDefId, LOCAL_CRATE}; +use rustc_hir::def_id::{DefId, LOCAL_CRATE, LocalDefId}; use rustc_hir::weak_lang_items::WEAK_LANG_ITEMS; -use rustc_hir::{lang_items, LangItem}; +use rustc_hir::{LangItem, lang_items}; use rustc_middle::middle::codegen_fn_attrs::{ CodegenFnAttrFlags, CodegenFnAttrs, PatchableFunctionEntry, TargetFeature, }; @@ -16,8 +16,8 @@ use rustc_middle::ty::{self as ty, Ty, TyCtxt}; use rustc_session::lint; use rustc_session::parse::feature_err; use rustc_span::symbol::Ident; -use rustc_span::{sym, Span}; -use rustc_target::spec::{abi, SanitizerSet}; +use rustc_span::{Span, sym}; +use rustc_target::spec::{SanitizerSet, abi}; use crate::errors; use crate::target_features::{check_target_feature_trait_unsafe, from_target_feature}; @@ -82,11 +82,7 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs { use DefKind::*; let def_kind = tcx.def_kind(did); - if let Fn | AssocFn | Variant | Ctor(..) = def_kind { - Some(tcx.fn_sig(did)) - } else { - None - } + if let Fn | AssocFn | Variant | Ctor(..) = def_kind { Some(tcx.fn_sig(did)) } else { None } }; for attr in attrs.iter() { @@ -253,7 +249,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 @@ -292,6 +293,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 => { @@ -602,7 +604,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( @@ -769,7 +773,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 601f0492b8393..cd3a7550efa73 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -941,6 +941,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 d752ae8f89366720f0084f6b9492dfbba675d497 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 | 11 ++- 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, 138 insertions(+), 54 deletions(-) diff --git a/compiler/rustc_codegen_gcc/src/attributes.rs b/compiler/rustc_codegen_gcc/src/attributes.rs index 5fdf2680aac88..9a4dde7c3614b 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 rustc_span::symbol::sym; use crate::context::CodegenCx; @@ -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::>(); + 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::>(); if let Some(features) = check_tied_features( cx.tcx.sess, diff --git a/compiler/rustc_codegen_llvm/src/attributes.rs b/compiler/rustc_codegen_llvm/src/attributes.rs index 489259da85646..cb4876aea5a58 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_span::symbol::sym; use rustc_target::spec::{FramePointer, SanitizerSet, StackProbeType, StackProtector}; @@ -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::>(); + 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::>(); if let Some(f) = llvm_util::check_tied_features( cx.tcx.sess, diff --git a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs index 85d86cee17845..88be48d70973a 100644 --- a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs +++ b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs @@ -9,10 +9,11 @@ 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; -use rustc_middle::ty::{self as ty, Ty, TyCtxt}; +use rustc_middle::ty::{self as ty, TyCtxt}; use rustc_session::lint; use rustc_session::parse::feature_err; use rustc_span::symbol::Ident; @@ -292,7 +293,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), ); } @@ -599,8 +600,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()); } } @@ -622,7 +623,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 })); } @@ -630,7 +631,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( @@ -778,30 +780,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 fcc01a0a267b5..f057061975224 100644 --- a/compiler/rustc_hir_typeck/src/coercion.rs +++ b/compiler/rustc_hir_typeck/src/coercion.rs @@ -915,7 +915,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 27c1b88f93f74..2caf038344396 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 40e227d7892d8..54307dc51a861 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) +}