Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Hide implicit target features from diagnostics when possible
  • Loading branch information
calebzulawski committed Aug 7, 2024
commit 83276f568032f14b1af7e5cd9f7d928734af8d09
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_gcc/src/attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ pub fn from_fn_attrs<'gcc, 'tcx>(
let function_features = codegen_fn_attrs
.target_features
.iter()
.map(|features| features.as_str())
.map(|features| features.name.as_str())
.collect::<Vec<&str>>();

if let Some(features) = check_tied_features(
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_llvm/src/attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,7 @@ pub fn from_fn_attrs<'ll, 'tcx>(
to_add.extend(tune_cpu_attr(cx));

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

if let Some(f) = llvm_util::check_tied_features(
cx.tcx.sess,
Expand Down
28 changes: 21 additions & 7 deletions compiler/rustc_codegen_ssa/src/target_features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use rustc_errors::Applicability;
use rustc_hir::def::DefKind;
use rustc_hir::def_id::{DefId, LocalDefId, LOCAL_CRATE};
use rustc_middle::bug;
use rustc_middle::middle::codegen_fn_attrs::TargetFeature;
use rustc_middle::query::Providers;
use rustc_middle::ty::TyCtxt;
use rustc_session::parse::feature_err;
Expand All @@ -18,7 +19,7 @@ pub fn from_target_feature(
tcx: TyCtxt<'_>,
attr: &ast::Attribute,
supported_target_features: &UnordMap<String, Option<Symbol>>,
target_features: &mut Vec<Symbol>,
target_features: &mut Vec<TargetFeature>,
) {
let Some(list) = attr.meta_item_list() else { return };
let bad_item = |span| {
Expand Down Expand Up @@ -99,14 +100,27 @@ pub fn from_target_feature(
}));
}

// Add both explicit and implied target features, using a set to deduplicate
let mut target_features_set = UnordSet::new();
// Add explicit features
target_features.extend(
added_target_features.iter().copied().map(|name| TargetFeature { name, implied: false }),
);

// Add implied features
let mut implied_target_features = UnordSet::new();
for feature in added_target_features.iter() {
target_features_set
implied_target_features
.extend_unord(tcx.implied_target_features(*feature).clone().into_items());
}
target_features_set.extend(added_target_features);
target_features.extend(target_features_set.into_sorted_stable_ord())
for feature in added_target_features.iter() {
implied_target_features.remove(feature);
}
target_features.extend(
implied_target_features
.into_sorted_stable_ord()
.iter()
.copied()
.map(|name| TargetFeature { name, implied: true }),
)
}

/// Computes the set of target features used in a function for the purposes of
Expand All @@ -115,7 +129,7 @@ fn asm_target_features(tcx: TyCtxt<'_>, did: DefId) -> &FxIndexSet<Symbol> {
let mut target_features = tcx.sess.unstable_target_features.clone();
if tcx.def_kind(did).has_codegen_attrs() {
let attrs = tcx.codegen_fn_attrs(did);
target_features.extend(&attrs.target_features);
target_features.extend(attrs.target_features.iter().map(|feature| feature.name));
match attrs.instruction_set {
None => {}
Some(InstructionSetAttr::ArmA32) => {
Expand Down
13 changes: 10 additions & 3 deletions compiler/rustc_const_eval/src/interpret/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -317,19 +317,26 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
&& attrs
.target_features
.iter()
.any(|feature| !self.tcx.sess.target_features.contains(feature))
.any(|feature| !self.tcx.sess.target_features.contains(&feature.name))
{
// Don't include implicit features in the error, unless only implicit features are
// missing. This should be rare, because it can only happen when an implicit feature
// is disabled, e.g. `+avx2,-avx`
let missing_explicit_features = attrs.target_features.iter().any(|feature| {
!feature.implied && !self.tcx.sess.target_features.contains(&feature.name)
});
throw_ub_custom!(
fluent::const_eval_unavailable_target_features_for_fn,
unavailable_feats = attrs
.target_features
.iter()
.filter(|&feature| !self.tcx.sess.target_features.contains(feature))
.filter(|&feature| !(missing_explicit_features && feature.implied)
&& !self.tcx.sess.target_features.contains(&feature.name))
.fold(String::new(), |mut s, feature| {
if !s.is_empty() {
s.push_str(", ");
}
s.push_str(feature.as_str());
s.push_str(feature.name.as_str());
s
}),
);
Expand Down
11 changes: 10 additions & 1 deletion compiler/rustc_middle/src/middle/codegen_fn_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ pub struct CodegenFnAttrs {
pub link_ordinal: Option<u16>,
/// The `#[target_feature(enable = "...")]` attribute and the enabled
/// features (only enabled features are supported right now).
pub target_features: Vec<Symbol>,
pub target_features: Vec<TargetFeature>,
/// The `#[linkage = "..."]` attribute on Rust-defined items and the value we found.
pub linkage: Option<Linkage>,
/// The `#[linkage = "..."]` attribute on foreign items and the value we found.
Expand All @@ -51,6 +51,15 @@ pub struct CodegenFnAttrs {
pub patchable_function_entry: Option<PatchableFunctionEntry>,
}

#[derive(Copy, Clone, Debug, TyEncodable, TyDecodable, HashStable)]
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
pub implied: bool,
}

#[derive(Copy, Clone, Debug, TyEncodable, TyDecodable, HashStable)]
pub struct PatchableFunctionEntry {
/// Nops to prepend to the function
Expand Down
26 changes: 21 additions & 5 deletions compiler/rustc_mir_build/src/check_unsafety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use std::ops::Bound;
use rustc_errors::DiagArgValue;
use rustc_hir::def::DefKind;
use rustc_hir::{self as hir, BindingMode, ByRef, HirId, Mutability};
use rustc_middle::middle::codegen_fn_attrs::TargetFeature;
use rustc_middle::mir::BorrowKind;
use rustc_middle::span_bug;
use rustc_middle::thir::visit::Visitor;
Expand All @@ -31,7 +32,7 @@ struct UnsafetyVisitor<'a, 'tcx> {
safety_context: SafetyContext,
/// The `#[target_feature]` attributes of the body. Used for checking
/// calls to functions with `#[target_feature]` (RFC 2396).
body_target_features: &'tcx [Symbol],
body_target_features: &'tcx [TargetFeature],
/// When inside the LHS of an assignment to a field, this is the type
/// of the LHS and the span of the assignment expression.
assignment_info: Option<Ty<'tcx>>,
Expand Down Expand Up @@ -442,14 +443,29 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
// is_like_wasm check in hir_analysis/src/collect.rs
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.contains(feature))
&& !callee_features.iter().all(|feature| {
self.body_target_features.iter().any(|f| f.name == feature.name)
})
{
// Don't include implicit features in the error, unless only implicit
// features are missing.
let missing_explicit_features = callee_features.iter().any(|feature| {
!feature.implied
&& !self.body_target_features.iter().any(|body_feature| {
!feature.implied && body_feature.name == feature.name
})
});
let missing: Vec<_> = callee_features
.iter()
.copied()
.filter(|feature| !self.body_target_features.contains(feature))
.filter(|feature| {
!(missing_explicit_features && 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
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_mir_transform/src/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,9 @@ impl<'tcx> Inliner<'tcx> {
return Err("incompatible instruction set");
}

if callee_attrs.target_features != self.codegen_fn_attrs.target_features {
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_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
// those calls depends on the target features of the surrounding function. By moving a
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: Undefined Behavior: calling a function that requires unavailable target features: avx, sse3, sse4.1, sse4.2, ssse3
error: Undefined Behavior: calling a function that requires unavailable target features: avx
--> $DIR/simd_feature_flag_difference.rs:LL:CC
|
LL | unsafe { foo(0.0, x) }
| ^^^^^^^^^^^ calling a function that requires unavailable target features: avx, sse3, sse4.1, sse4.2, ssse3
| ^^^^^^^^^^^ calling a function that requires unavailable target features: avx
|
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/tests/fail/function_calls/target_feature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
fn main() {
assert!(!is_x86_feature_detected!("ssse3"));
unsafe {
ssse3_fn(); //~ ERROR: calling a function that requires unavailable target features: sse3, ssse3
ssse3_fn(); //~ ERROR: calling a function that requires unavailable target features: ssse3
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: Undefined Behavior: calling a function that requires unavailable target features: sse3, ssse3
error: Undefined Behavior: calling a function that requires unavailable target features: ssse3
--> $DIR/target_feature.rs:LL:CC
|
LL | ssse3_fn();
| ^^^^^^^^^^ calling a function that requires unavailable target features: sse3, ssse3
| ^^^^^^^^^^ calling a function that requires unavailable target features: ssse3
|
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
//@ignore-target-s390x
//@ignore-target-thumbv7em
//@ignore-target-wasm32
//@compile-flags: -C target-feature=+aes,+vaes,+avx512f,+sse4.2
//@compile-flags: -C target-feature=+aes,+vaes,+avx512f

#![feature(avx512_target_feature, stdarch_x86_avx512)]

Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/tests/pass/shims/x86/intrinsics-x86-avx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
//@ignore-target-s390x
//@ignore-target-thumbv7em
//@ignore-target-wasm32
//@compile-flags: -C target-feature=+avx,+sse4.2
//@compile-flags: -C target-feature=+avx

#[cfg(target_arch = "x86")]
use std::arch::x86::*;
Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/tests/pass/shims/x86/intrinsics-x86-avx2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
//@ignore-target-s390x
//@ignore-target-thumbv7em
//@ignore-target-wasm32
//@compile-flags: -C target-feature=+avx2,+sse4.2
//@compile-flags: -C target-feature=+avx2

#[cfg(target_arch = "x86")]
use std::arch::x86::*;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
//@ignore-target-s390x
//@ignore-target-thumbv7em
//@ignore-target-wasm32
//@compile-flags: -C target-feature=+avx512f,+avx512vl,+avx512bitalg,+avx512vpopcntdq,+sse4.2
//@compile-flags: -C target-feature=+avx512f,+avx512vl,+avx512bitalg,+avx512vpopcntdq

#![feature(avx512_target_feature)]
#![feature(stdarch_x86_avx512)]
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/consts/const-eval/const_fn_target_feature.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error[E0080]: evaluation of constant value failed
--> $DIR/const_fn_target_feature.rs:11:24
|
LL | const B: () = unsafe { avx2_fn() };
| ^^^^^^^^^ calling a function that requires unavailable target features: avx, avx2, sse4.1, sse4.2
| ^^^^^^^^^ calling a function that requires unavailable target features: avx2

error: aborting due to 1 previous error

Expand Down
26 changes: 12 additions & 14 deletions tests/ui/rfcs/rfc-2396-target_feature-11/safe-calls.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,42 +4,40 @@ error[E0133]: call to function `sse2` with `#[target_feature]` is unsafe and req
LL | sse2();
| ^^^^^^ call to function with `#[target_feature]`
|
= help: in order for the call to be safe, the context requires the following additional target features: sse and sse2
= note: the sse and sse2 target features being enabled in the build configuration does not remove the requirement to list them in `#[target_feature]`
= help: in order for the call to be safe, the context requires the following additional target feature: sse2
= note: the sse2 target feature being enabled in the build configuration does not remove the requirement to list it in `#[target_feature]`

error[E0133]: call to function `avx_bmi2` with `#[target_feature]` is unsafe and requires unsafe function or block
--> $DIR/safe-calls.rs:29:5
|
LL | avx_bmi2();
| ^^^^^^^^^^ call to function with `#[target_feature]`
|
= help: in order for the call to be safe, the context requires the following additional target features: avx, sse, sse2, sse3, sse4.1, sse4.2, ssse3, and bmi2
= note: the sse and sse2 target features being enabled in the build configuration does not remove the requirement to list them in `#[target_feature]`
= help: in order for the call to be safe, the context requires the following additional target features: avx and bmi2

error[E0133]: call to function `Quux::avx_bmi2` with `#[target_feature]` is unsafe and requires unsafe function or block
--> $DIR/safe-calls.rs:31:5
|
LL | Quux.avx_bmi2();
| ^^^^^^^^^^^^^^^ call to function with `#[target_feature]`
|
= help: in order for the call to be safe, the context requires the following additional target features: avx, sse, sse2, sse3, sse4.1, sse4.2, ssse3, and bmi2
= note: the sse and sse2 target features being enabled in the build configuration does not remove the requirement to list them in `#[target_feature]`
= help: in order for the call to be safe, the context requires the following additional target features: avx and bmi2

error[E0133]: call to function `avx_bmi2` with `#[target_feature]` is unsafe and requires unsafe function or block
--> $DIR/safe-calls.rs:38:5
|
LL | avx_bmi2();
| ^^^^^^^^^^ call to function with `#[target_feature]`
|
= help: in order for the call to be safe, the context requires the following additional target features: avx, sse3, sse4.1, sse4.2, ssse3, and bmi2
= help: in order for the call to be safe, the context requires the following additional target features: avx and bmi2

error[E0133]: call to function `Quux::avx_bmi2` with `#[target_feature]` is unsafe and requires unsafe function or block
--> $DIR/safe-calls.rs:40:5
|
LL | Quux.avx_bmi2();
| ^^^^^^^^^^^^^^^ call to function with `#[target_feature]`
|
= help: in order for the call to be safe, the context requires the following additional target features: avx, sse3, sse4.1, sse4.2, ssse3, and bmi2
= help: in order for the call to be safe, the context requires the following additional target features: avx and bmi2

error[E0133]: call to function `avx_bmi2` with `#[target_feature]` is unsafe and requires unsafe function or block
--> $DIR/safe-calls.rs:47:5
Expand All @@ -63,17 +61,17 @@ error[E0133]: call to function `sse2` with `#[target_feature]` is unsafe and req
LL | const _: () = sse2();
| ^^^^^^ call to function with `#[target_feature]`
|
= help: in order for the call to be safe, the context requires the following additional target features: sse and sse2
= note: the sse and sse2 target features being enabled in the build configuration does not remove the requirement to list them in `#[target_feature]`
= help: in order for the call to be safe, the context requires the following additional target feature: sse2
= note: the sse2 target feature being enabled in the build configuration does not remove the requirement to list it in `#[target_feature]`

error[E0133]: call to function `sse2_and_fxsr` with `#[target_feature]` is unsafe and requires unsafe function or block
--> $DIR/safe-calls.rs:64:15
|
LL | const _: () = sse2_and_fxsr();
| ^^^^^^^^^^^^^^^ call to function with `#[target_feature]`
|
= help: in order for the call to be safe, the context requires the following additional target features: sse, sse2, and fxsr
= note: the fxsr, sse, and sse2 target features being enabled in the build configuration does not remove the requirement to list them in `#[target_feature]`
= help: in order for the call to be safe, the context requires the following additional target features: sse2 and fxsr
= note: the fxsr and sse2 target features being enabled in the build configuration does not remove the requirement to list them in `#[target_feature]`

error[E0133]: call to function `sse2` with `#[target_feature]` is unsafe and requires unsafe block
--> $DIR/safe-calls.rs:69:5
Expand All @@ -82,8 +80,8 @@ LL | sse2();
| ^^^^^^ call to function with `#[target_feature]`
|
= note: for more information, see issue #71668 <https://github.com/rust-lang/rust/issues/71668>
= help: in order for the call to be safe, the context requires the following additional target features: sse and sse2
= note: the sse and sse2 target features being enabled in the build configuration does not remove the requirement to list them in `#[target_feature]`
= help: in order for the call to be safe, the context requires the following additional target feature: sse2
= note: the sse2 target feature being enabled in the build configuration does not remove the requirement to list it in `#[target_feature]`
note: an unsafe function restricts its caller, but its body is safe by default
--> $DIR/safe-calls.rs:68:1
|
Expand Down