Skip to content

Commit

Permalink
Hide implicit target features from diagnostics when possible
Browse files Browse the repository at this point in the history
  • Loading branch information
calebzulawski committed Aug 7, 2024
1 parent 6b96a60 commit 83276f5
Show file tree
Hide file tree
Showing 16 changed files with 89 additions and 43 deletions.
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

0 comments on commit 83276f5

Please sign in to comment.