Skip to content

Commit acb4e8b

Browse files
committedAug 28, 2024·
Auto merge of rust-lang#127537 - veluca93:struct_tf, r=BoxyUwU
Implement a first version of RFC 3525: struct target features This PR is an attempt at implementing rust-lang/rfcs#3525, behind a feature gate `struct_target_features`. There's obviously a few tasks that ought to be done before this is merged; in no particular order: - add proper error messages - add tests - create a tracking issue for the RFC - properly serialize/deserialize the new target_features field in `rmeta` (assuming I even understood that correctly :-)) That said, as I am definitely not a `rustc` expert, I'd like to get some early feedback on the overall approach before fixing those things (and perhaps some pointers for `rmeta`...), hence this early PR :-) Here's an example piece of code that I have been using for testing - with the new code, the calls to intrinsics get correctly inlined: ```rust #![feature(struct_target_features)] use std::arch::x86_64::*; /* // fails to compile #[target_feature(enable = "avx")] struct Invalid(u32); */ #[target_feature(enable = "avx")] struct Avx {} #[target_feature(enable = "sse")] struct Sse(); /* // fails to compile extern "C" fn bad_fun(_: Avx) {} */ /* // fails to compile #[inline(always)] fn inline_fun(_: Avx) {} */ 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<S: Simd>(simd: &S) { simd.do_something(); } fn main() { /* // fails to compile Avx {}; */ if is_x86_feature_detected!("avx") { let avx = unsafe { Avx {} }; dosomething(&avx); dosomething(&WithAvx { avx }); } if is_x86_feature_detected!("sse") { dosomething(&unsafe { Sse {} }) } } ``` Tracking: - rust-lang#129107
2 parents 100fde5 + 7eb4cfe commit acb4e8b

File tree

25 files changed

+511
-27
lines changed

25 files changed

+511
-27
lines changed
 

‎compiler/rustc_codegen_ssa/src/codegen_attrs.rs

+115-10
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use rustc_ast::{ast, attr, MetaItemKind, NestedMetaItem};
22
use rustc_attr::{list_contains_name, InlineAttr, InstructionSetAttr, OptimizeAttr};
3+
use rustc_data_structures::fx::FxHashSet;
34
use rustc_errors::codes::*;
45
use rustc_errors::{struct_span_code_err, DiagMessage, SubdiagMessage};
56
use rustc_hir as hir;
@@ -8,7 +9,7 @@ use rustc_hir::def_id::{DefId, LocalDefId, LOCAL_CRATE};
89
use rustc_hir::weak_lang_items::WEAK_LANG_ITEMS;
910
use rustc_hir::{lang_items, LangItem};
1011
use rustc_middle::middle::codegen_fn_attrs::{
11-
CodegenFnAttrFlags, CodegenFnAttrs, PatchableFunctionEntry,
12+
CodegenFnAttrFlags, CodegenFnAttrs, PatchableFunctionEntry, TargetFeature,
1213
};
1314
use rustc_middle::mir::mono::Linkage;
1415
use rustc_middle::query::Providers;
@@ -17,6 +18,7 @@ use rustc_session::lint;
1718
use rustc_session::parse::feature_err;
1819
use rustc_span::symbol::Ident;
1920
use rustc_span::{sym, Span};
21+
use rustc_target::abi::VariantIdx;
2022
use rustc_target::spec::{abi, SanitizerSet};
2123

2224
use crate::errors;
@@ -78,23 +80,26 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
7880
let mut link_ordinal_span = None;
7981
let mut no_sanitize_span = None;
8082

83+
let fn_sig_outer = || {
84+
use DefKind::*;
85+
86+
let def_kind = tcx.def_kind(did);
87+
if let Fn | AssocFn | Variant | Ctor(..) = def_kind { Some(tcx.fn_sig(did)) } else { None }
88+
};
89+
8190
for attr in attrs.iter() {
8291
// In some cases, attribute are only valid on functions, but it's the `check_attr`
8392
// pass that check that they aren't used anywhere else, rather this module.
8493
// In these cases, we bail from performing further checks that are only meaningful for
8594
// functions (such as calling `fn_sig`, which ICEs if given a non-function). We also
8695
// report a delayed bug, just in case `check_attr` isn't doing its job.
8796
let fn_sig = || {
88-
use DefKind::*;
89-
90-
let def_kind = tcx.def_kind(did);
91-
if let Fn | AssocFn | Variant | Ctor(..) = def_kind {
92-
Some(tcx.fn_sig(did))
93-
} else {
97+
let sig = fn_sig_outer();
98+
if sig.is_none() {
9499
tcx.dcx()
95100
.span_delayed_bug(attr.span, "this attribute can only be applied to functions");
96-
None
97101
}
102+
sig
98103
};
99104

100105
let Some(Ident { name, .. }) = attr.ident() else {
@@ -613,7 +618,93 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
613618
}
614619
}
615620

616-
// If a function uses #[target_feature] it can't be inlined into general
621+
if let Some(sig) = fn_sig_outer() {
622+
// Collect target features from types reachable from arguments.
623+
// We define a type as "reachable" if:
624+
// - it is a function argument
625+
// - it is a field of a reachable struct
626+
// - there is a reachable reference to it
627+
// FIXME(struct_target_features): we may want to cache the result of this computation.
628+
let mut visited_types = FxHashSet::default();
629+
let mut reachable_types: Vec<_> = sig.skip_binder().inputs().skip_binder().to_owned();
630+
let mut additional_tf = vec![];
631+
632+
while let Some(ty) = reachable_types.pop() {
633+
if visited_types.contains(&ty) {
634+
continue;
635+
}
636+
visited_types.insert(ty);
637+
match ty.kind() {
638+
ty::Alias(..) => {
639+
if let Ok(t) =
640+
tcx.try_normalize_erasing_regions(tcx.param_env(did.to_def_id()), ty)
641+
{
642+
reachable_types.push(t)
643+
}
644+
}
645+
646+
ty::Ref(_, inner, _) => reachable_types.push(*inner),
647+
ty::Tuple(tys) => reachable_types.extend(tys.iter()),
648+
ty::Adt(adt_def, args) => {
649+
additional_tf.extend_from_slice(tcx.struct_target_features(adt_def.did()));
650+
// This only recurses into structs as i.e. an Option<TargetFeature> is an ADT
651+
// that doesn't actually always contain a TargetFeature.
652+
if adt_def.is_struct() {
653+
reachable_types.extend(
654+
adt_def
655+
.variant(VariantIdx::from_usize(0))
656+
.fields
657+
.iter()
658+
.map(|field| field.ty(tcx, args)),
659+
);
660+
}
661+
}
662+
ty::Bool
663+
| ty::Char
664+
| ty::Int(..)
665+
| ty::Uint(..)
666+
| ty::Float(..)
667+
| ty::Foreign(..)
668+
| ty::Str
669+
| ty::Array(..)
670+
| ty::Pat(..)
671+
| ty::Slice(..)
672+
| ty::RawPtr(..)
673+
| ty::FnDef(..)
674+
| ty::FnPtr(..)
675+
| ty::Dynamic(..)
676+
| ty::Closure(..)
677+
| ty::CoroutineClosure(..)
678+
| ty::Coroutine(..)
679+
| ty::CoroutineWitness(..)
680+
| ty::Never
681+
| ty::Param(..)
682+
| ty::Bound(..)
683+
| ty::Placeholder(..)
684+
| ty::Infer(..)
685+
| ty::Error(..) => (),
686+
}
687+
}
688+
689+
// FIXME(struct_target_features): is this really necessary?
690+
if !additional_tf.is_empty() && sig.skip_binder().abi() != abi::Abi::Rust {
691+
tcx.dcx().span_err(
692+
tcx.hir().span(tcx.local_def_id_to_hir_id(did)),
693+
"cannot use a struct with target features in a function with non-Rust ABI",
694+
);
695+
}
696+
if !additional_tf.is_empty() && codegen_fn_attrs.inline == InlineAttr::Always {
697+
tcx.dcx().span_err(
698+
tcx.hir().span(tcx.local_def_id_to_hir_id(did)),
699+
"cannot use a struct with target features in a #[inline(always)] function",
700+
);
701+
}
702+
codegen_fn_attrs
703+
.target_features
704+
.extend(additional_tf.iter().map(|tf| TargetFeature { implied: true, ..*tf }));
705+
}
706+
707+
// If a function uses non-default target_features it can't be inlined into general
617708
// purpose functions as they wouldn't have the right target features
618709
// enabled. For that reason we also forbid #[inline(always)] as it can't be
619710
// respected.
@@ -758,6 +849,20 @@ fn check_link_name_xor_ordinal(
758849
}
759850
}
760851

852+
fn struct_target_features(tcx: TyCtxt<'_>, def_id: LocalDefId) -> &[TargetFeature] {
853+
let mut features = vec![];
854+
let supported_features = tcx.supported_target_features(LOCAL_CRATE);
855+
for attr in tcx.get_attrs(def_id, sym::target_feature) {
856+
from_target_feature(tcx, attr, supported_features, &mut features);
857+
}
858+
tcx.arena.alloc_slice(&features)
859+
}
860+
761861
pub fn provide(providers: &mut Providers) {
762-
*providers = Providers { codegen_fn_attrs, should_inherit_track_caller, ..*providers };
862+
*providers = Providers {
863+
codegen_fn_attrs,
864+
should_inherit_track_caller,
865+
struct_target_features,
866+
..*providers
867+
};
763868
}

‎compiler/rustc_feature/src/unstable.rs

+2
Original file line numberDiff line numberDiff line change
@@ -594,6 +594,8 @@ declare_features! (
594594
(unstable, strict_provenance, "1.61.0", Some(95228)),
595595
/// Allows string patterns to dereference values to match them.
596596
(unstable, string_deref_patterns, "1.67.0", Some(87121)),
597+
/// Allows structs to carry target_feature information.
598+
(incomplete, struct_target_features, "CURRENT_RUSTC_VERSION", Some(129107)),
597599
/// Allows the use of `#[target_feature]` on safe functions.
598600
(unstable, target_feature_11, "1.45.0", Some(69098)),
599601
/// Allows using `#[thread_local]` on `static` items.

‎compiler/rustc_hir/src/def.rs

+35
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,41 @@ impl DefKind {
326326
| DefKind::ExternCrate => false,
327327
}
328328
}
329+
330+
/// Whether `query struct_target_features` should be used with this definition.
331+
pub fn has_struct_target_features(self) -> bool {
332+
match self {
333+
DefKind::Struct | DefKind::Union | DefKind::Enum => true,
334+
DefKind::Fn
335+
| DefKind::AssocFn
336+
| DefKind::Ctor(..)
337+
| DefKind::Closure
338+
| DefKind::Static { .. }
339+
| DefKind::Mod
340+
| DefKind::Variant
341+
| DefKind::Trait
342+
| DefKind::TyAlias
343+
| DefKind::ForeignTy
344+
| DefKind::TraitAlias
345+
| DefKind::AssocTy
346+
| DefKind::Const
347+
| DefKind::AssocConst
348+
| DefKind::Macro(..)
349+
| DefKind::Use
350+
| DefKind::ForeignMod
351+
| DefKind::OpaqueTy
352+
| DefKind::Impl { .. }
353+
| DefKind::Field
354+
| DefKind::TyParam
355+
| DefKind::ConstParam
356+
| DefKind::LifetimeParam
357+
| DefKind::AnonConst
358+
| DefKind::InlineConst
359+
| DefKind::SyntheticCoroutineBody
360+
| DefKind::GlobalAsm
361+
| DefKind::ExternCrate => false,
362+
}
363+
}
329364
}
330365

331366
/// The resolution of a path or export.

‎compiler/rustc_hir_typeck/src/coercion.rs

+2
Original file line numberDiff line numberDiff line change
@@ -850,6 +850,8 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
850850
}
851851

852852
// Safe `#[target_feature]` functions are not assignable to safe fn pointers (RFC 2396).
853+
// FIXME(struct_target_features): should this be true also for functions that inherit
854+
// target features from structs?
853855

854856
if b_hdr.safety == hir::Safety::Safe
855857
&& !self.tcx.codegen_fn_attrs(def_id).target_features.is_empty()

‎compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs

+1
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,7 @@ provide! { tcx, def_id, other, cdata,
224224
variances_of => { table }
225225
fn_sig => { table }
226226
codegen_fn_attrs => { table }
227+
struct_target_features => { table }
227228
impl_trait_header => { table }
228229
const_param_default => { table }
229230
object_lifetime_default => { table }

‎compiler/rustc_metadata/src/rmeta/encoder.rs

+3
Original file line numberDiff line numberDiff line change
@@ -1392,6 +1392,9 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
13921392
if def_kind.has_codegen_attrs() {
13931393
record!(self.tables.codegen_fn_attrs[def_id] <- self.tcx.codegen_fn_attrs(def_id));
13941394
}
1395+
if def_kind.has_struct_target_features() {
1396+
record_array!(self.tables.struct_target_features[def_id] <- self.tcx.struct_target_features(def_id));
1397+
}
13951398
if should_encode_visibility(def_kind) {
13961399
let vis =
13971400
self.tcx.local_visibility(local_id).map_id(|def_id| def_id.local_def_index);

‎compiler/rustc_metadata/src/rmeta/mod.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ use rustc_macros::{
1919
Decodable, Encodable, MetadataDecodable, MetadataEncodable, TyDecodable, TyEncodable,
2020
};
2121
use rustc_middle::metadata::ModChild;
22-
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrs;
22+
use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrs, TargetFeature};
2323
use rustc_middle::middle::debugger_visualizer::DebuggerVisualizerFile;
2424
use rustc_middle::middle::exported_symbols::{ExportedSymbol, SymbolExportInfo};
2525
use rustc_middle::middle::lib_features::FeatureStability;
@@ -427,6 +427,7 @@ define_tables! {
427427
variances_of: Table<DefIndex, LazyArray<ty::Variance>>,
428428
fn_sig: Table<DefIndex, LazyValue<ty::EarlyBinder<'static, ty::PolyFnSig<'static>>>>,
429429
codegen_fn_attrs: Table<DefIndex, LazyValue<CodegenFnAttrs>>,
430+
struct_target_features: Table<DefIndex, LazyArray<TargetFeature>>,
430431
impl_trait_header: Table<DefIndex, LazyValue<ty::ImplTraitHeader<'static>>>,
431432
const_param_default: Table<DefIndex, LazyValue<ty::EarlyBinder<'static, rustc_middle::ty::Const<'static>>>>,
432433
object_lifetime_default: Table<DefIndex, LazyValue<ObjectLifetimeDefault>>,

‎compiler/rustc_middle/src/middle/codegen_fn_attrs.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@ pub struct CodegenFnAttrs {
2626
/// be set when `link_name` is set. This is for foreign items with the
2727
/// "raw-dylib" kind.
2828
pub link_ordinal: Option<u16>,
29-
/// The `#[target_feature(enable = "...")]` attribute and the enabled
30-
/// features (only enabled features are supported right now).
29+
/// All the target features that are enabled for this function. Some features might be enabled
30+
/// implicitly.
3131
pub target_features: Vec<TargetFeature>,
3232
/// The `#[linkage = "..."]` attribute on Rust-defined items and the value we found.
3333
pub linkage: Option<Linkage>,
@@ -55,8 +55,8 @@ pub struct CodegenFnAttrs {
5555
pub struct TargetFeature {
5656
/// The name of the target feature (e.g. "avx")
5757
pub name: Symbol,
58-
/// The feature is implied by another feature, rather than explicitly added by the
59-
/// `#[target_feature]` attribute
58+
/// The feature is implied by another feature or by an argument, rather than explicitly
59+
/// added by the `#[target_feature]` attribute
6060
pub implied: bool,
6161
}
6262

‎compiler/rustc_middle/src/query/mod.rs

+6-1
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ use {rustc_ast as ast, rustc_attr as attr, rustc_hir as hir};
4747
use crate::infer::canonical::{self, Canonical};
4848
use crate::lint::LintExpectation;
4949
use crate::metadata::ModChild;
50-
use crate::middle::codegen_fn_attrs::CodegenFnAttrs;
50+
use crate::middle::codegen_fn_attrs::{CodegenFnAttrs, TargetFeature};
5151
use crate::middle::debugger_visualizer::DebuggerVisualizerFile;
5252
use crate::middle::exported_symbols::{ExportedSymbol, SymbolExportInfo};
5353
use crate::middle::lib_features::LibFeatures;
@@ -1245,6 +1245,11 @@ rustc_queries! {
12451245
feedable
12461246
}
12471247

1248+
query struct_target_features(def_id: DefId) -> &'tcx [TargetFeature] {
1249+
separate_provide_extern
1250+
desc { |tcx| "computing target features for struct `{}`", tcx.def_path_str(def_id) }
1251+
}
1252+
12481253
query asm_target_features(def_id: DefId) -> &'tcx FxIndexSet<Symbol> {
12491254
desc { |tcx| "computing target features for inline asm of `{}`", tcx.def_path_str(def_id) }
12501255
}

‎compiler/rustc_middle/src/ty/parameterized.rs

+1
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ trivially_parameterized_over_tcx! {
5959
std::string::String,
6060
crate::metadata::ModChild,
6161
crate::middle::codegen_fn_attrs::CodegenFnAttrs,
62+
crate::middle::codegen_fn_attrs::TargetFeature,
6263
crate::middle::debugger_visualizer::DebuggerVisualizerFile,
6364
crate::middle::exported_symbols::SymbolExportInfo,
6465
crate::middle::lib_features::FeatureStability,

‎compiler/rustc_mir_build/messages.ftl

+16
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,17 @@ mir_build_initializing_type_with_requires_unsafe_unsafe_op_in_unsafe_fn_allowed
125125
.note = initializing a layout restricted type's field with a value outside the valid range is undefined behavior
126126
.label = initializing type with `rustc_layout_scalar_valid_range` attr
127127
128+
mir_build_initializing_type_with_target_feature_requires_unsafe =
129+
initializing type with `target_feature` attr is unsafe and requires unsafe block
130+
.note = this struct can only be constructed if the corresponding `target_feature`s are available
131+
.label = initializing type with `target_feature` attr
132+
133+
mir_build_initializing_type_with_target_feature_requires_unsafe_unsafe_op_in_unsafe_fn_allowed =
134+
initializing type with `target_feature` attr is unsafe and requires unsafe function or block
135+
.note = this struct can only be constructed if the corresponding `target_feature`s are available
136+
.label = initializing type with `target_feature` attr
137+
138+
128139
mir_build_inline_assembly_requires_unsafe =
129140
use of inline assembly is unsafe and requires unsafe block
130141
.note = inline assembly is entirely unchecked and can cause undefined behavior
@@ -387,6 +398,11 @@ mir_build_unsafe_op_in_unsafe_fn_initializing_type_with_requires_unsafe =
387398
.note = initializing a layout restricted type's field with a value outside the valid range is undefined behavior
388399
.label = initializing type with `rustc_layout_scalar_valid_range` attr
389400
401+
mir_build_unsafe_op_in_unsafe_fn_initializing_type_with_target_feature_requires_unsafe =
402+
initializing type with `target_feature` attr is unsafe and requires unsafe block
403+
.note = this struct can only be constructed if the corresponding `target_feature`s are available
404+
.label = initializing type with `target_feature` attr
405+
390406
mir_build_unsafe_op_in_unsafe_fn_inline_assembly_requires_unsafe =
391407
use of inline assembly is unsafe and requires unsafe block
392408
.note = inline assembly is entirely unchecked and can cause undefined behavior

0 commit comments

Comments
 (0)
Please sign in to comment.