Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated the list of white-listed target features for x86 #78361

Merged
merged 4 commits into from
Nov 18, 2020
Merged

Updated the list of white-listed target features for x86 #78361

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9feb567
Updated the list of white-listed target features for x86
DevJPM Oct 25, 2020
cd95e93
Removed movbe from run-time-detect
DevJPM Oct 25, 2020
3daa93f
Updated documentation, x86 feature detection testing, and removed LLV…
DevJPM Oct 26, 2020
72b83af
Re-enable LLVM 9 target features with LLVM 9 being the minimum now
DevJPM Nov 15, 2020
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
5 changes: 5 additions & 0 deletions compiler/rustc_codegen_llvm/src/llvm_util.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,13 +137,18 @@ pub fn time_trace_profiler_finish(file_name: &str) {
// WARNING: the features after applying `to_llvm_feature` 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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should remind people to check out the submodule in our repo and its current commit, e.g. currently https://github.com/rust-lang/llvm-project/tree/ee1617457899ef2eb55dcf7ee2758b4340b6533f

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added that as well as a note that the git submodule isn't the whole truth but instead external supported LLVM versions need to also be considered - the internal one seems to be LLVM 11 (ish)

pub fn to_llvm_feature<'a>(sess: &Session, s: &'a str) -> &'a str {
let arch = if sess.target.arch == "x86_64" { "x86" } else { &*sess.target.arch };
match (arch, s) {
("x86", "pclmulqdq") => "pclmul",
("x86", "rdrand") => "rdrnd",
("x86", "bmi1") => "bmi",
("x86", "cmpxchg16b") => "cx16",
("x86", "avx512vaes") => "vaes",
("x86", "avx512gfni") => "gfni",
("x86", "avx512vpclmulqdq") => "vpclmulqdq",
("aarch64", "fp") => "fp-armv8",
("aarch64", "fp16") => "fullfp16",
(_, s) => s,
Expand Down
13 changes: 13 additions & 0 deletions compiler/rustc_codegen_ssa/src/target_features.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@ use rustc_session::Session;
use rustc_span::symbol::sym;
use rustc_span::symbol::Symbol;

// 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
// if it doesn't, to_llvm_feature in llvm_util in rustc_codegen_llvm needs to be adapted

const ARM_ALLOWED_FEATURES: &[(&str, Option<Symbol>)] = &[
("aclass", Some(sym::arm_target_feature)),
("mclass", Some(sym::arm_target_feature)),
Expand Down Expand Up @@ -50,15 +55,23 @@ const X86_ALLOWED_FEATURES: &[(&str, Option<Symbol>)] = &[
("aes", None),
("avx", None),
("avx2", None),
("avx512bf16", Some(sym::avx512_target_feature)),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests are going absolutely bonkers over the absence of this feature.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have commented this feature out, it seems the support was only added in LLVM 9 and apparently we still support LLVM 8 (if the name of the relevant CI check is to be believed and my interpretation of the CI is correct). Is there some tracking issue for the supported external LLVM version where I can follow to re-add this change once the base support LLVM version is bumped?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mmm? Not aware of any policy around that, sorry!

("avx512bitalg", Some(sym::avx512_target_feature)),
("avx512bw", Some(sym::avx512_target_feature)),
("avx512cd", Some(sym::avx512_target_feature)),
("avx512dq", Some(sym::avx512_target_feature)),
("avx512er", Some(sym::avx512_target_feature)),
("avx512f", Some(sym::avx512_target_feature)),
("avx512gfni", Some(sym::avx512_target_feature)),
("avx512ifma", Some(sym::avx512_target_feature)),
("avx512pf", Some(sym::avx512_target_feature)),
("avx512vaes", Some(sym::avx512_target_feature)),
("avx512vbmi", Some(sym::avx512_target_feature)),
("avx512vbmi2", Some(sym::avx512_target_feature)),
("avx512vl", Some(sym::avx512_target_feature)),
("avx512vnni", Some(sym::avx512_target_feature)),
("avx512vp2intersect", Some(sym::avx512_target_feature)),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto.

("avx512vpclmulqdq", Some(sym::avx512_target_feature)),
("avx512vpopcntdq", Some(sym::avx512_target_feature)),
("bmi1", None),
("bmi2", None),
Expand Down
63 changes: 38 additions & 25 deletions library/std/tests/run-time-detect.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,42 +54,55 @@ fn powerpc64_linux() {
#[test]
#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
fn x86_all() {
// the below is in alphabetical order and matches
// the order of X86_ALLOWED_FEATURES in rustc_codegen_ssa's target_features.rs

println!("adx: {:?}", is_x86_feature_detected!("adx"));
println!("aes: {:?}", is_x86_feature_detected!("aes"));
println!("pcmulqdq: {:?}", is_x86_feature_detected!("pclmulqdq"));
println!("avx: {:?}", is_x86_feature_detected!("avx"));
println!("avx2: {:?}", is_x86_feature_detected!("avx2"));
println!("avx512bf16: {:?}", is_x86_feature_detected!("avx512bf16"));
println!("avx512bitalg: {:?}", is_x86_feature_detected!("avx512bitalg"));
println!("avx512bw: {:?}", is_x86_feature_detected!("avx512bw"));
println!("avx512cd: {:?}", is_x86_feature_detected!("avx512cd"));
println!("avx512dq: {:?}", is_x86_feature_detected!("avx512dq"));
println!("avx512er: {:?}", is_x86_feature_detected!("avx512er"));
println!("avx512f: {:?}", is_x86_feature_detected!("avx512f"));
println!("avx512gfni: {:?}", is_x86_feature_detected!("avx512gfni"));
println!("avx512ifma: {:?}", is_x86_feature_detected!("avx512ifma"));
println!("avx512pf: {:?}", is_x86_feature_detected!("avx512pf"));
println!("avx512vaes: {:?}", is_x86_feature_detected!("avx512vaes"));
println!("avx512vbmi: {:?}", is_x86_feature_detected!("avx512vbmi"));
println!("avx512vbmi2: {:?}", is_x86_feature_detected!("avx512vbmi2"));
println!("avx512vl: {:?}", is_x86_feature_detected!("avx512vl"));
println!("avx512vnni: {:?}", is_x86_feature_detected!("avx512vnni"));
println!("avx512vp2intersect: {:?}", is_x86_feature_detected!("avx512vp2intersect"));
println!("avx512vpclmulqdq: {:?}", is_x86_feature_detected!("avx512vpclmulqdq"));
println!("avx512vpopcntdq: {:?}", is_x86_feature_detected!("avx512vpopcntdq"));
println!("bmi1: {:?}", is_x86_feature_detected!("bmi1"));
println!("bmi2: {:?}", is_x86_feature_detected!("bmi2"));
println!("cmpxchg16b: {:?}", is_x86_feature_detected!("cmpxchg16b"));
println!("f16c: {:?}", is_x86_feature_detected!("f16c"));
println!("fma: {:?}", is_x86_feature_detected!("fma"));
println!("fxsr: {:?}", is_x86_feature_detected!("fxsr"));
println!("lzcnt: {:?}", is_x86_feature_detected!("lzcnt"));
//println!("movbe: {:?}", is_x86_feature_detected!("movbe")); // movbe is unsupported as a target feature
println!("pclmulqdq: {:?}", is_x86_feature_detected!("pclmulqdq"));
println!("popcnt: {:?}", is_x86_feature_detected!("popcnt"));
println!("rdrand: {:?}", is_x86_feature_detected!("rdrand"));
println!("rdseed: {:?}", is_x86_feature_detected!("rdseed"));
println!("tsc: {:?}", is_x86_feature_detected!("tsc"));
println!("mmx: {:?}", is_x86_feature_detected!("mmx"));
Comment on lines -61 to -62
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not intended that MMX feature detection be removed from Rust, in spite of the fact we don't do anything with it. The detection of capabilities and emission of such code are two entirely different concerns. Likewise it is not clear why tsc is being removed here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This removal (as well as the removal of the test for the abm detection) were unintentional. I have re-added them, though above the other features to preserve consistency with X86_ALLOWED_FEATURES.

println!("rtm: {:?}", is_x86_feature_detected!("rtm"));
println!("sha: {:?}", is_x86_feature_detected!("sha"));
println!("sse: {:?}", is_x86_feature_detected!("sse"));
println!("sse2: {:?}", is_x86_feature_detected!("sse2"));
println!("sse3: {:?}", is_x86_feature_detected!("sse3"));
println!("ssse3: {:?}", is_x86_feature_detected!("ssse3"));
println!("sse4.1: {:?}", is_x86_feature_detected!("sse4.1"));
println!("sse4.2: {:?}", is_x86_feature_detected!("sse4.2"));
println!("sse4a: {:?}", is_x86_feature_detected!("sse4a"));
println!("sha: {:?}", is_x86_feature_detected!("sha"));
println!("avx: {:?}", is_x86_feature_detected!("avx"));
println!("avx2: {:?}", is_x86_feature_detected!("avx2"));
println!("avx512f {:?}", is_x86_feature_detected!("avx512f"));
println!("avx512cd {:?}", is_x86_feature_detected!("avx512cd"));
println!("avx512er {:?}", is_x86_feature_detected!("avx512er"));
println!("avx512pf {:?}", is_x86_feature_detected!("avx512pf"));
println!("avx512bw {:?}", is_x86_feature_detected!("avx512bw"));
println!("avx512dq {:?}", is_x86_feature_detected!("avx512dq"));
println!("avx512vl {:?}", is_x86_feature_detected!("avx512vl"));
println!("avx512_ifma {:?}", is_x86_feature_detected!("avx512ifma"));
println!("avx512_vbmi {:?}", is_x86_feature_detected!("avx512vbmi"));
println!("avx512_vpopcntdq {:?}", is_x86_feature_detected!("avx512vpopcntdq"));
println!("fma: {:?}", is_x86_feature_detected!("fma"));
println!("bmi1: {:?}", is_x86_feature_detected!("bmi1"));
println!("bmi2: {:?}", is_x86_feature_detected!("bmi2"));
println!("abm: {:?}", is_x86_feature_detected!("abm"));
println!("lzcnt: {:?}", is_x86_feature_detected!("lzcnt"));
println!("ssse3: {:?}", is_x86_feature_detected!("ssse3"));
println!("tbm: {:?}", is_x86_feature_detected!("tbm"));
println!("popcnt: {:?}", is_x86_feature_detected!("popcnt"));
println!("fxsr: {:?}", is_x86_feature_detected!("fxsr"));
println!("xsave: {:?}", is_x86_feature_detected!("xsave"));
println!("xsavec: {:?}", is_x86_feature_detected!("xsavec"));
println!("xsaveopt: {:?}", is_x86_feature_detected!("xsaveopt"));
println!("xsaves: {:?}", is_x86_feature_detected!("xsaves"));
println!("xsavec: {:?}", is_x86_feature_detected!("xsavec"));
}