Skip to content

Commit

Permalink
Rollup merge of rust-lang#91608 - workingjubilee:fold-neon-fp, r=Amanieu
Browse files Browse the repository at this point in the history
Fold aarch64 feature +fp into +neon

Arm's FEAT_FP and Feat_AdvSIMD describe the same thing on AArch64:
The Neon unit, which handles both floating point and SIMD instructions.
Moreover, a configuration for AArch64 must include both or neither.
Arm says "entirely proprietary" toolchains may omit floating point:
https://developer.arm.com/documentation/102374/0101/Data-processing---floating-point
In the Programmer's Guide for Armv8-A, Arm says AArch64 can have
both FP and Neon or neither in custom implementations:
https://developer.arm.com/documentation/den0024/a/AArch64-Floating-point-and-NEON

In "Bare metal boot code for Armv8-A", enabling Neon and FP
is just disabling the same trap flag:
https://developer.arm.com/documentation/dai0527/a

In an unlikely future where "Neon and FP" become unrelated,
we can add "[+-]fp" as its own feature flag.
Until then, we can simplify programming with Rust on AArch64 by
folding both into "[+-]neon", which is valid as it supersets both.

"[+-]neon" is retained for niche uses such as firmware, kernels,
"I just hate floats", and so on.

I am... pretty sure no one is relying on this.

An argument could be made that, as we are not an "entirely proprietary" toolchain, we should not support AArch64 without floats at all. I think that's a bit excessive. However, I want to recognize the intent: programming for AArch64 should be simplified where possible. For x86-64, programmers regularly set up illegal feature configurations because it's hard to understand them, see rust-lang#89586. And per the above notes, plus the discussion in rust-lang#86941, there should be no real use cases for leaving these features split: the two should in fact always go together.

Fixes rust-lang#95002.
Fixes rust-lang#95122.
  • Loading branch information
Dylan-DPC authored Mar 22, 2022
2 parents 3ea4493 + f4e7fde commit e77d593
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 10 deletions.
10 changes: 4 additions & 6 deletions compiler/rustc_codegen_llvm/src/llvm_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,6 @@ pub fn to_llvm_features<'a>(sess: &Session, s: &'a str) -> SmallVec<[&'a str; 2]
("x86", "avx512vaes") => smallvec!["vaes"],
("x86", "avx512gfni") => smallvec!["gfni"],
("x86", "avx512vpclmulqdq") => smallvec!["vpclmulqdq"],
("aarch64", "fp") => smallvec!["fp-armv8"],
("aarch64", "rcpc2") => smallvec!["rcpc-immo"],
("aarch64", "dpb") => smallvec!["ccpp"],
("aarch64", "dpb2") => smallvec!["ccdp"],
Expand Down Expand Up @@ -238,12 +237,11 @@ pub fn target_features(sess: &Session) -> Vec<Symbol> {
.filter_map(|&(feature, gate)| {
if sess.is_nightly_build() || gate.is_none() { Some(feature) } else { None }
})
.flat_map(|feature| to_llvm_features(sess, feature))
.filter(|feature| {
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;
}
let cstr = SmallCStr::new(feature);
if unsafe { llvm::LLVMRustHasFeature(target_machine, cstr.as_ptr()) } {
return true;
}
false
})
Expand Down
4 changes: 1 addition & 3 deletions compiler/rustc_codegen_ssa/src/target_features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,8 @@ const ARM_ALLOWED_FEATURES: &[(&str, Option<Symbol>)] = &[
];

const AARCH64_ALLOWED_FEATURES: &[(&str, Option<Symbol>)] = &[
// FEAT_AdvSimd
// FEAT_AdvSimd & FEAT_FP
("neon", None),
// FEAT_FP
("fp", None),
// FEAT_FP16
("fp16", None),
// FEAT_SVE
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_target/src/asm/aarch64.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ impl AArch64InlineAsmRegClass {
match self {
Self::reg => types! { _: I8, I16, I32, I64, F32, F64; },
Self::vreg | Self::vreg_low16 => types! {
fp: I8, I16, I32, I64, F32, F64,
neon: I8, I16, I32, I64, F32, F64,
VecI8(8), VecI16(4), VecI32(2), VecI64(1), VecF32(2), VecF64(1),
VecI8(16), VecI16(8), VecI32(4), VecI64(2), VecF32(4), VecF64(2);
},
Expand Down

0 comments on commit e77d593

Please sign in to comment.