diff --git a/compiler/rustc_codegen_llvm/src/attributes.rs b/compiler/rustc_codegen_llvm/src/attributes.rs index 13a41388f5e2f..4ab6f1bd0f569 100644 --- a/compiler/rustc_codegen_llvm/src/attributes.rs +++ b/compiler/rustc_codegen_llvm/src/attributes.rs @@ -382,10 +382,7 @@ pub fn from_fn_attrs<'ll, 'tcx>( let mut function_features = function_features .iter() .flat_map(|feat| { - llvm_util::to_llvm_feature(cx.tcx.sess, feat) - .into_iter() - .map(|f| format!("+{}", f)) - .collect::>() + llvm_util::to_llvm_features(cx.tcx.sess, feat).into_iter().map(|f| format!("+{}", f)) }) .chain(codegen_fn_attrs.instruction_set.iter().map(|x| match x { InstructionSetAttr::ArmA32 => "-thumb-mode".to_string(), diff --git a/compiler/rustc_codegen_llvm/src/llvm_util.rs b/compiler/rustc_codegen_llvm/src/llvm_util.rs index 69eacafb1b594..ceba4f297eef3 100644 --- a/compiler/rustc_codegen_llvm/src/llvm_util.rs +++ b/compiler/rustc_codegen_llvm/src/llvm_util.rs @@ -2,14 +2,18 @@ use crate::back::write::create_informational_target_machine; use crate::{llvm, llvm_util}; use libc::c_int; use libloading::Library; -use rustc_codegen_ssa::target_features::{supported_target_features, tied_target_features}; +use rustc_codegen_ssa::target_features::{ + supported_target_features, tied_target_features, RUSTC_SPECIFIC_FEATURES, +}; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; +use rustc_data_structures::small_c_str::SmallCStr; use rustc_fs_util::path_to_c_string; use rustc_middle::bug; use rustc_session::config::PrintRequest; use rustc_session::Session; use rustc_span::symbol::Symbol; use rustc_target::spec::{MergeFunctions, PanicStrategy}; +use smallvec::{smallvec, SmallVec}; use std::ffi::{CStr, CString}; use tracing::debug; @@ -155,9 +159,10 @@ pub fn time_trace_profiler_finish(file_name: &Path) { } } -// WARNING: the features after applying `to_llvm_feature` must be known +// WARNING: the features after applying `to_llvm_features` must be known // to LLVM or the feature detection code will walk past the end of the feature // array, leading to crashes. +// // To find a list of LLVM's names, check llvm-project/llvm/include/llvm/Support/*TargetParser.def // where the * matches the architecture's name // Beware to not use the llvm github project for this, but check the git submodule @@ -165,35 +170,35 @@ pub fn time_trace_profiler_finish(file_name: &Path) { // Though note that Rust can also be build with an external precompiled version of LLVM // which might lead to failures if the oldest tested / supported LLVM version // doesn't yet support the relevant intrinsics -pub fn to_llvm_feature<'a>(sess: &Session, s: &'a str) -> Vec<&'a str> { +pub fn to_llvm_features<'a>(sess: &Session, s: &'a str) -> SmallVec<[&'a str; 2]> { let arch = if sess.target.arch == "x86_64" { "x86" } else { &*sess.target.arch }; match (arch, s) { ("x86", "sse4.2") => { if get_version() >= (14, 0, 0) { - vec!["sse4.2", "crc32"] + smallvec!["sse4.2", "crc32"] } else { - vec!["sse4.2"] + smallvec!["sse4.2"] } } - ("x86", "pclmulqdq") => vec!["pclmul"], - ("x86", "rdrand") => vec!["rdrnd"], - ("x86", "bmi1") => vec!["bmi"], - ("x86", "cmpxchg16b") => vec!["cx16"], - ("x86", "avx512vaes") => vec!["vaes"], - ("x86", "avx512gfni") => vec!["gfni"], - ("x86", "avx512vpclmulqdq") => vec!["vpclmulqdq"], - ("aarch64", "fp") => vec!["fp-armv8"], - ("aarch64", "fp16") => vec!["fullfp16"], - ("aarch64", "fhm") => vec!["fp16fml"], - ("aarch64", "rcpc2") => vec!["rcpc-immo"], - ("aarch64", "dpb") => vec!["ccpp"], - ("aarch64", "dpb2") => vec!["ccdp"], - ("aarch64", "frintts") => vec!["fptoint"], - ("aarch64", "fcma") => vec!["complxnum"], - ("aarch64", "pmuv3") => vec!["perfmon"], - ("aarch64", "paca") => vec!["pauth"], - ("aarch64", "pacg") => vec!["pauth"], - (_, s) => vec![s], + ("x86", "pclmulqdq") => smallvec!["pclmul"], + ("x86", "rdrand") => smallvec!["rdrnd"], + ("x86", "bmi1") => smallvec!["bmi"], + ("x86", "cmpxchg16b") => smallvec!["cx16"], + ("x86", "avx512vaes") => smallvec!["vaes"], + ("x86", "avx512gfni") => smallvec!["gfni"], + ("x86", "avx512vpclmulqdq") => smallvec!["vpclmulqdq"], + ("aarch64", "fp") => smallvec!["fp-armv8"], + ("aarch64", "fp16") => smallvec!["fullfp16"], + ("aarch64", "fhm") => smallvec!["fp16fml"], + ("aarch64", "rcpc2") => smallvec!["rcpc-immo"], + ("aarch64", "dpb") => smallvec!["ccpp"], + ("aarch64", "dpb2") => smallvec!["ccdp"], + ("aarch64", "frintts") => smallvec!["fptoint"], + ("aarch64", "fcma") => smallvec!["complxnum"], + ("aarch64", "pmuv3") => smallvec!["perfmon"], + ("aarch64", "paca") => smallvec!["pauth"], + ("aarch64", "pacg") => smallvec!["pauth"], + (_, s) => smallvec![s], } } @@ -207,7 +212,6 @@ pub fn check_tied_features( // Tied features must be set to the same value, or not set at all let mut tied_iter = tied.iter(); let enabled = features.get(tied_iter.next().unwrap()); - if tied_iter.any(|f| enabled != features.get(f)) { return Some(tied); } @@ -221,15 +225,11 @@ pub fn target_features(sess: &Session) -> Vec { supported_target_features(sess) .iter() .filter_map(|&(feature, gate)| { - if sess.is_nightly_build() || gate.is_none() { - Some(feature) - } else { - None - } + if sess.is_nightly_build() || gate.is_none() { Some(feature) } else { None } }) .filter(|feature| { - for llvm_feature in to_llvm_feature(sess, feature) { - let cstr = CString::new(llvm_feature).unwrap(); + for llvm_feature in to_llvm_features(sess, feature) { + let cstr = SmallCStr::new(llvm_feature); if unsafe { llvm::LLVMRustHasFeature(target_machine, cstr.as_ptr()) } { return true; } @@ -302,9 +302,9 @@ fn print_target_features(sess: &Session, tm: &llvm::TargetMachine) { let mut rustc_target_features = supported_target_features(sess) .iter() .filter_map(|(feature, _gate)| { - for llvm_feature in to_llvm_feature(sess, *feature) { + for llvm_feature in to_llvm_features(sess, *feature) { // LLVM asserts that these are sorted. LLVM and Rust both use byte comparison for these strings. - match target_features.binary_search_by_key(&llvm_feature, |(f, _d)| (*f)).ok().map( + match target_features.binary_search_by_key(&llvm_feature, |(f, _d)| f).ok().map( |index| { let (_f, desc) = target_features.remove(index); (*feature, desc) @@ -374,14 +374,7 @@ pub fn target_cpu(sess: &Session) -> &str { /// The list of LLVM features computed from CLI flags (`-Ctarget-cpu`, `-Ctarget-feature`, /// `--target` and similar). -// FIXME(nagisa): Cache the output of this somehow? Maybe make this a query? We're calling this -// for every function that has `#[target_feature]` on it. The global features won't change between -// the functions; only crates, maybe… -pub fn llvm_global_features(sess: &Session) -> Vec { - // FIXME(nagisa): this should definitely be available more centrally and to other codegen backends. - /// These features control behaviour of rustc rather than llvm. - const RUSTC_SPECIFIC_FEATURES: &[&str] = &["crt-static"]; - +pub(crate) fn global_llvm_features(sess: &Session, diagnostics: bool) -> Vec { // Features that come earlier are overriden by conflicting features later in the string. // Typically we'll want more explicit settings to override the implicit ones, so: // @@ -427,42 +420,108 @@ pub fn llvm_global_features(sess: &Session) -> Vec { Some(_) | None => {} }; - fn strip(s: &str) -> &str { - s.strip_prefix(&['+', '-']).unwrap_or(s) + // Features implied by an implicit or explicit `--target`. + features.extend( + sess.target + .features + .split(',') + .filter(|v| !v.is_empty() && backend_feature_name(v).is_some()) + .map(String::from), + ); + + // -Ctarget-features + let supported_features = supported_target_features(sess); + let feats = sess + .opts + .cg + .target_feature + .split(',') + .filter_map(|s| { + let enable_disable = match s.chars().next() { + None => return None, + Some(c @ '+' | c @ '-') => c, + Some(_) => { + if diagnostics { + let mut diag = sess.struct_warn(&format!( + "unknown feature specified for `-Ctarget-feature`: `{}`", + s + )); + diag.note("features must begin with a `+` to enable or `-` to disable it"); + diag.emit(); + } + return None; + } + }; + + let feature = backend_feature_name(s)?; + // Warn against use of LLVM specific feature names on the CLI. + if diagnostics && !supported_features.iter().any(|&(v, _)| v == feature) { + let rust_feature = supported_features.iter().find_map(|&(rust_feature, _)| { + let llvm_features = to_llvm_features(sess, rust_feature); + if llvm_features.contains(&feature) && !llvm_features.contains(&rust_feature) { + Some(rust_feature) + } else { + None + } + }); + let mut diag = sess.struct_warn(&format!( + "unknown feature specified for `-Ctarget-feature`: `{}`", + feature + )); + diag.note("it is still passed through to the codegen backend"); + if let Some(rust_feature) = rust_feature { + diag.help(&format!("you might have meant: `{}`", rust_feature)); + } else { + diag.note("consider filing a feature request"); + } + diag.emit(); + } + Some((enable_disable, feature)) + }) + .collect::>(); + + if diagnostics { + // FIXME(nagisa): figure out how to not allocate a full hashset here. + let featmap = feats.iter().map(|&(flag, feat)| (feat, flag == '+')).collect(); + if let Some(f) = check_tied_features(sess, &featmap) { + sess.err(&format!( + "target features {} must all be enabled or disabled together", + f.join(", ") + )); + } } - let filter = |s: &str| { - // features must start with a `+` or `-`. - let feature = match s.strip_prefix(&['+', '-'][..]) { - None => return vec![], - // Rustc-specific feature requests like `+crt-static` or `-crt-static` - // are not passed down to LLVM. - Some(feature) if RUSTC_SPECIFIC_FEATURES.contains(&feature) => return vec![], - Some(feature) => feature, - }; - // ... otherwise though we run through `to_llvm_feature` when + features.extend(feats.into_iter().flat_map(|(enable_disable, feature)| { + // rustc-specific features do not get passed down to LLVM… + if RUSTC_SPECIFIC_FEATURES.contains(&feature) { + return SmallVec::<[_; 2]>::new(); + } + // ... otherwise though we run through `to_llvm_feature when // passing requests down to LLVM. This means that all in-language // features also work on the command line instead of having two // different names when the LLVM name and the Rust name differ. - to_llvm_feature(sess, feature).iter().map(|f| format!("{}{}", &s[..1], f)).collect() - }; - - // Features implied by an implicit or explicit `--target`. - features.extend(sess.target.features.split(',').flat_map(&filter)); + to_llvm_features(sess, feature) + .into_iter() + .map(|f| format!("{}{}", enable_disable, f)) + .collect() + })); + features +} - // -Ctarget-features - let feats: Vec<&str> = sess.opts.cg.target_feature.split(',').collect(); - // LLVM enables based on the last occurence of a feature - if let Some(f) = - check_tied_features(sess, &feats.iter().map(|f| (strip(f), !f.starts_with("-"))).collect()) - { - sess.err(&format!( - "target features {} must all be enabled or disabled together", - f.join(", ") - )); +/// Returns a feature name for the given `+feature` or `-feature` string. +/// +/// Only allows features that are backend specific (i.e. not [`RUSTC_SPECIFIC_FEATURES`].) +fn backend_feature_name(s: &str) -> Option<&str> { + // features must start with a `+` or `-`. + let feature = s.strip_prefix(&['+', '-'][..]).unwrap_or_else(|| { + bug!("target feature `{}` must begin with a `+` or `-`", s); + }); + // Rustc-specific feature requests like `+crt-static` or `-crt-static` + // are not passed down to LLVM. + if RUSTC_SPECIFIC_FEATURES.contains(&feature) { + return None; } - features.extend(feats.iter().flat_map(&filter)); - features + Some(feature) } pub fn tune_cpu(sess: &Session) -> Option<&str> { diff --git a/compiler/rustc_codegen_ssa/src/target_features.rs b/compiler/rustc_codegen_ssa/src/target_features.rs index 77166c89735e4..fd4adfea8082c 100644 --- a/compiler/rustc_codegen_ssa/src/target_features.rs +++ b/compiler/rustc_codegen_ssa/src/target_features.rs @@ -4,6 +4,9 @@ use rustc_session::Session; use rustc_span::symbol::sym; use rustc_span::symbol::Symbol; +/// Features that control behaviour of rustc, rather than the codegen. +pub const RUSTC_SPECIFIC_FEATURES: &[&str] = &["crt-static"]; + // When adding features to the below lists // check whether they're named already elsewhere in rust // e.g. in stdarch and whether the given name matches LLVM's diff --git a/src/test/ui/target-feature/missing-plusminus.rs b/src/test/ui/target-feature/missing-plusminus.rs new file mode 100644 index 0000000000000..efee659292342 --- /dev/null +++ b/src/test/ui/target-feature/missing-plusminus.rs @@ -0,0 +1,2 @@ +// compile-flags: -Ctarget-feature=banana --crate-type=rlib +// build-pass diff --git a/src/test/ui/target-feature/missing-plusminus.stderr b/src/test/ui/target-feature/missing-plusminus.stderr new file mode 100644 index 0000000000000..1d446107f7086 --- /dev/null +++ b/src/test/ui/target-feature/missing-plusminus.stderr @@ -0,0 +1,18 @@ +warning: unknown feature specified for `-Ctarget-feature`: `banana` + | + = note: features must begin with a `+` to enable or `-` to disable it + +warning: unknown feature specified for `-Ctarget-feature`: `banana` + | + = note: features must begin with a `+` to enable or `-` to disable it + +warning: unknown feature specified for `-Ctarget-feature`: `banana` + | + = note: features must begin with a `+` to enable or `-` to disable it + +warning: unknown feature specified for `-Ctarget-feature`: `banana` + | + = note: features must begin with a `+` to enable or `-` to disable it + +warning: 4 warnings emitted + diff --git a/src/test/ui/target-feature/similar-feature-suggestion.rs b/src/test/ui/target-feature/similar-feature-suggestion.rs new file mode 100644 index 0000000000000..4e4e2160cac57 --- /dev/null +++ b/src/test/ui/target-feature/similar-feature-suggestion.rs @@ -0,0 +1,6 @@ +// compile-flags: -Ctarget-feature=+rdrnd --crate-type=rlib --target=x86_64-unknown-linux-gnu +// build-pass +// needs-llvm-components: x86 + +#![feature(no_core)] +#![no_core] diff --git a/src/test/ui/target-feature/similar-feature-suggestion.stderr b/src/test/ui/target-feature/similar-feature-suggestion.stderr new file mode 100644 index 0000000000000..de4d0064fbcd0 --- /dev/null +++ b/src/test/ui/target-feature/similar-feature-suggestion.stderr @@ -0,0 +1,22 @@ +warning: unknown feature specified for `-Ctarget-feature`: `rdrnd` + | + = note: it is still passed through to the codegen backend + = help: you might have meant: `rdrand` + +warning: unknown feature specified for `-Ctarget-feature`: `rdrnd` + | + = note: it is still passed through to the codegen backend + = help: did you mean: `rdrand` + +warning: unknown feature specified for `-Ctarget-feature`: `rdrnd` + | + = note: it is still passed through to the codegen backend + = help: did you mean: `rdrand` + +warning: unknown feature specified for `-Ctarget-feature`: `rdrnd` + | + = note: it is still passed through to the codegen backend + = help: did you mean: `rdrand` + +warning: 4 warnings emitted + diff --git a/src/test/ui/target-feature/tied-features-cli.one.stderr b/src/test/ui/target-feature/tied-features-cli.one.stderr index 2bc64a76aae8d..0cc901eecaa2c 100644 --- a/src/test/ui/target-feature/tied-features-cli.one.stderr +++ b/src/test/ui/target-feature/tied-features-cli.one.stderr @@ -1,4 +1,4 @@ -error: Target features paca, pacg must all be enabled or disabled together +error: target features paca, pacg must all be enabled or disabled together error: aborting due to previous error diff --git a/src/test/ui/target-feature/tied-features-cli.rs b/src/test/ui/target-feature/tied-features-cli.rs index ea09d4fc46093..72b7e3da5309d 100644 --- a/src/test/ui/target-feature/tied-features-cli.rs +++ b/src/test/ui/target-feature/tied-features-cli.rs @@ -1,9 +1,20 @@ -// only-aarch64 -// revisions: one two three four -//[one] compile-flags: -C target-feature=+paca -//[two] compile-flags: -C target-feature=-pacg,+pacg -//[three] compile-flags: -C target-feature=+paca,+pacg,-paca -//[four] check-pass -//[four] compile-flags: -C target-feature=-paca,+pacg -C target-feature=+paca +// revisions: one two three +// compile-flags: --crate-type=rlib --target=aarch64-unknown-linux-gnu +// needs-llvm-components: aarch64 +// +// +// [one] check-fail +// [one] compile-flags: -C target-feature=+paca +// [two] check-fail +// [two] compile-flags: -C target-feature=-pacg,+pacg +// [three] check-fail +// [three] compile-flags: -C target-feature=+paca,+pacg,-paca +// [four] build-pass +// [four] compile-flags: -C target-feature=-paca,+pacg -C target-feature=+paca +#![feature(no_core, lang_items)] +#![no_core] + +#[lang="sized"] +trait Sized {} fn main() {} diff --git a/src/test/ui/target-feature/tied-features-cli.three.stderr b/src/test/ui/target-feature/tied-features-cli.three.stderr index 2bc64a76aae8d..0cc901eecaa2c 100644 --- a/src/test/ui/target-feature/tied-features-cli.three.stderr +++ b/src/test/ui/target-feature/tied-features-cli.three.stderr @@ -1,4 +1,4 @@ -error: Target features paca, pacg must all be enabled or disabled together +error: target features paca, pacg must all be enabled or disabled together error: aborting due to previous error diff --git a/src/test/ui/target-feature/tied-features-cli.two.stderr b/src/test/ui/target-feature/tied-features-cli.two.stderr index 2bc64a76aae8d..0cc901eecaa2c 100644 --- a/src/test/ui/target-feature/tied-features-cli.two.stderr +++ b/src/test/ui/target-feature/tied-features-cli.two.stderr @@ -1,4 +1,4 @@ -error: Target features paca, pacg must all be enabled or disabled together +error: target features paca, pacg must all be enabled or disabled together error: aborting due to previous error diff --git a/src/test/ui/target-feature/tied-features.rs b/src/test/ui/target-feature/tied-features.rs index 86400361db314..01353e9f70c59 100644 --- a/src/test/ui/target-feature/tied-features.rs +++ b/src/test/ui/target-feature/tied-features.rs @@ -1,9 +1,15 @@ -// only-aarch64 // build-fail - +// compile-flags: --crate-type=rlib --target=aarch64-unknown-linux-gnu +// needs-llvm-components: aarch64 #![feature(aarch64_target_feature, target_feature_11)] +#![feature(no_core, lang_items)] +#![no_core] + +#[lang="sized"] +trait Sized {} -fn main() { +// FIXME: this should not need to be public. +pub fn main() { #[target_feature(enable = "pacg")] //~^ ERROR must all be either enabled or disabled together unsafe fn inner() {} diff --git a/src/test/ui/target-feature/tied-features.stderr b/src/test/ui/target-feature/tied-features.stderr index 0b1460e0b753f..6362c7ae60b6e 100644 --- a/src/test/ui/target-feature/tied-features.stderr +++ b/src/test/ui/target-feature/tied-features.stderr @@ -1,5 +1,5 @@ error: the target features paca, pacg must all be either enabled or disabled together - --> $DIR/tied-features.rs:7:5 + --> $DIR/tied-features.rs:13:5 | LL | #[target_feature(enable = "pacg")] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -7,7 +7,7 @@ LL | #[target_feature(enable = "pacg")] = help: add the missing features in a `target_feature` attribute error: the target features paca, pacg must all be either enabled or disabled together - --> $DIR/tied-features.rs:19:1 + --> $DIR/tied-features.rs:25:1 | LL | #[target_feature(enable = "paca")] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^