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

Jump to bottom

Update list of allowed aarch64 features #84665

Merged
merged 5 commits into from May 20, 2021
Merged

Update list of allowed aarch64 features #84665

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3f5f54c
Update list of allowed aarch64 features
adamgemmell Apr 28, 2021
523b4d1
Remove LSE2
adamgemmell May 19, 2021
c71e58d
Rename fptoint to frintts
adamgemmell May 19, 2021
904467a
Ensure all crypto components (AES, PMULL, SHA1/2) are available on ar…
adamgemmell May 19, 2021
d3737a6
Remove test for crypto feature ahead of its removal
adamgemmell May 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions compiler/rustc_codegen_llvm/src/llvm_util.rs
View file
Open in desktop
Expand Up @@ -152,6 +152,12 @@ pub fn to_llvm_feature<'a>(sess: &Session, s: &'a str) -> &'a str {
("x86", "avx512vpclmulqdq") => "vpclmulqdq",
("aarch64", "fp") => "fp-armv8",
("aarch64", "fp16") => "fullfp16",
("aarch64", "fhm") => "fp16fml",
("aarch64", "lse2") => "outline-atomics",
Copy link
Member

Choose a reason for hiding this comment

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

lse2 doesn't have an LLVM equivalent feature: it doesn't add any new instructions, it just relaxes the restrictions on existing atomic instructions.

outline-atomics is something different: it's a codegen option on LLVM to call external functions for atomic operations so that they can take advantage of LSE on targets that support it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch, thanks a lot!

("aarch64", "rcpc2") => "rcpc-immo",
("aarch64", "dpb") => "ccpp",
("aarch64", "dpb2") => "ccdp",
("aarch64", "fcma") => "complxnum",
(_, s) => s,
}
}
Expand Down
77 changes: 75 additions & 2 deletions compiler/rustc_codegen_ssa/src/target_features.rs
View file
Open in desktop
Expand Up @@ -33,22 +33,95 @@ const ARM_ALLOWED_FEATURES: &[(&str, Option<Symbol>)] = &[
("thumb-mode", Some(sym::arm_target_feature)),
];

// Commented features are not available in LLVM 9.0, or have since been renamed
const AARCH64_ALLOWED_FEATURES: &[(&str, Option<Symbol>)] = &[
("fp", Some(sym::aarch64_target_feature)),
// FEAT_AdvSimd
("neon", Some(sym::aarch64_target_feature)),
// FEAT_FP
("fp", Some(sym::aarch64_target_feature)),
// FEAT_FP16
("fp16", Some(sym::aarch64_target_feature)),
// FEAT_SVE
("sve", Some(sym::aarch64_target_feature)),
// FEAT_CRC
("crc", Some(sym::aarch64_target_feature)),
// Cryptographic extension
("crypto", Some(sym::aarch64_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.

LLVM has moved away from a combined "crypto" feature and now uses separate "aes" and "sha2" features. We should do the same 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.

I agree - the crypto feature doesn't really mean much any more and the separate features are better. Wouldn't removing this here affect any users detecting this feature? Likewise with removing it from std_detect.

Copy link
Member

Choose a reason for hiding this comment

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

AArch64 target features are still unstable, so it's fine to rename/remove them.

// FEAT_RAS
("ras", Some(sym::aarch64_target_feature)),
// FEAT_LSE
("lse", Some(sym::aarch64_target_feature)),
// FEAT_LSE2
// ("lse2", Some(sym::aarch64_target_feature)),
// FEAT_RDM
("rdm", Some(sym::aarch64_target_feature)),
("fp16", Some(sym::aarch64_target_feature)),
// FEAT_RCPC
("rcpc", Some(sym::aarch64_target_feature)),
// FEAT_RCPC2
("rcpc2", Some(sym::aarch64_target_feature)),
// FEAT_DotProd
("dotprod", Some(sym::aarch64_target_feature)),
// FEAT_TME
("tme", Some(sym::aarch64_target_feature)),
// FEAT_FHM
("fhm", Some(sym::aarch64_target_feature)),
// FEAT_DIT
("dit", Some(sym::aarch64_target_feature)),
// FEAT_FLAGM
// ("flagm", Some(sym::aarch64_target_feature)),
// FEAT_SSBS
("ssbs", Some(sym::aarch64_target_feature)),
// FEAT_SB
("sb", Some(sym::aarch64_target_feature)),
// FEAT_PAUTH
// ("pauth", Some(sym::aarch64_target_feature)),
// FEAT_DPB
("dpb", Some(sym::aarch64_target_feature)),
// FEAT_DPB2
("dpb2", Some(sym::aarch64_target_feature)),
// FEAT_SVE2
("sve2", Some(sym::aarch64_target_feature)),
// FEAT_SVE2_AES
("sve2-aes", Some(sym::aarch64_target_feature)),
// FEAT_SVE2_SM4
("sve2-sm4", Some(sym::aarch64_target_feature)),
// FEAT_SVE2_SHA3
("sve2-sha3", Some(sym::aarch64_target_feature)),
// FEAT_SVE2_BitPerm
("sve2-bitperm", Some(sym::aarch64_target_feature)),
// FEAT_FRINTTS
("fptoint", Some(sym::aarch64_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.

"fptoint" is a poor name for this feature, I prefer the original ARM name "frintts".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Happy to rename this

// FEAT_I8MM
// ("i8mm", Some(sym::aarch64_target_feature)),
// FEAT_F32MM
// ("f32mm", Some(sym::aarch64_target_feature)),
// FEAT_F64MM
// ("f64mm", Some(sym::aarch64_target_feature)),
// FEAT_BF16
// ("bf16", Some(sym::aarch64_target_feature)),
// FEAT_RAND
("rand", Some(sym::aarch64_target_feature)),
// FEAT_BTI
("bti", Some(sym::aarch64_target_feature)),
// FEAT_MTE
("mte", Some(sym::aarch64_target_feature)),
// FEAT_JSCVT
("jsconv", Some(sym::aarch64_target_feature)),
// FEAT_FCMA
("fcma", Some(sym::aarch64_target_feature)),
// FEAT_SHA1 & FEAT_SHA256
("sha2", Some(sym::aarch64_target_feature)),
// FEAT_SHA512 & FEAT_SHA3
("sha3", Some(sym::aarch64_target_feature)),
// FEAT_SM3 & FEAT_SM4
("sm4", Some(sym::aarch64_target_feature)),
("v8.1a", Some(sym::aarch64_target_feature)),
("v8.2a", Some(sym::aarch64_target_feature)),
("v8.3a", Some(sym::aarch64_target_feature)),
("v8.4a", Some(sym::aarch64_target_feature)),
("v8.5a", Some(sym::aarch64_target_feature)),
// ("v8.6a", Some(sym::aarch64_target_feature)),
// ("v8.7a", Some(sym::aarch64_target_feature)),
];

const X86_ALLOWED_FEATURES: &[(&str, Option<Symbol>)] = &[
Expand Down