aarch64 is not whitelisted for ARM features

[gnzlbg]

This prevents the target feature neon from being enabled on aarch64.

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @pnkfelix (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[parched]

Hi @gnzlbg arm and aarch64 are separate targets so don't share the same target features. neon just happens to be a common one between them. Regardless, neon (specifically Advanced SIMD) is a default target feature on aarch64 anyway.

[gnzlbg]

@parched yes but cfg(target_feature = "neon") returns always false for aarch64 because it is not white-listed. See: rust-lang/stdarch#121 (comment)

---

EDIT: maybe we could add a different AARCH64_WHITELIST but we can always do that later and that seems overkill for the bug that this is trying to fix.

[pnkfelix]

@bors r+ rollup

[bors]

📌 Commit 3bd9d62 has been approved by pnkfelix

[parched]

@gnzlbg @pnkfelix yes a separate whitelist is needed. I think this is more of a missing feature than a bug, but using the ARM_WHITELIST for AArch64 would be a bug as there are non AArch64 features in that whitelist.

[gnzlbg]

@parched roger that, I will implement a new white list for aarch64.

@pnkfelix don't merge this yet.

[gnzlbg]

@parched I've introduced a new white-list in the last commit.

[kennytm]

@bors r=pnkfelix

[bors]

📌 Commit 6020f30 has been approved by pnkfelix

[parched]

@gnzlbg better but I think this should probably go through a mini RFC as specified by the target feature RFC. Notably my concern is that there is nothing named "NEON" in the Armv8/AArch64 architecture only "Advanced SIMD". Both GCC and Clang just call the feature simd. See https://gcc.gnu.org/onlinedocs/gcc-7.2.0/gcc/AArch64-Options.html#aarch64-feature-modifiers

[kennytm]

@parched what? it is called neon by LLVM.

$ llc -march=aarch64 -mattr=help
Available CPUs for this target:

  cortex-a35   - Select the cortex-a35 processor.
  cortex-a53   - Select the cortex-a53 processor.
  cortex-a57   - Select the cortex-a57 processor.
  cortex-a72   - Select the cortex-a72 processor.
  cortex-a73   - Select the cortex-a73 processor.
  cyclone      - Select the cyclone processor.
  exynos-m1    - Select the exynos-m1 processor.
  exynos-m2    - Select the exynos-m2 processor.
  exynos-m3    - Select the exynos-m3 processor.
  falkor       - Select the falkor processor.
  generic      - Select the generic processor.
  kryo         - Select the kryo processor.
  thunderx     - Select the thunderx processor.
  thunderx2t99 - Select the thunderx2t99 processor.
  thunderxt81  - Select the thunderxt81 processor.
  thunderxt83  - Select the thunderxt83 processor.
  thunderxt88  - Select the thunderxt88 processor.

Available features for this target:

  a35                                - Cortex-A35 ARM processors.
  a53                                - Cortex-A53 ARM processors.
  a57                                - Cortex-A57 ARM processors.
  a72                                - Cortex-A72 ARM processors.
  a73                                - Cortex-A73 ARM processors.
  alternate-sextload-cvt-f32-pattern - Use alternative pattern for sextload convert to f32.
  arith-bcc-fusion                   - CPU fuses arithmetic+bcc operations.
  arith-cbz-fusion                   - CPU fuses arithmetic + cbz/cbnz operations.
  balance-fp-ops                     - balance mix of odd and even D-registers for fp multiply(-accumulate) ops.
  crc                                - Enable ARMv8 CRC-32 checksum instructions.
  crypto                             - Enable cryptographic instructions.
  custom-cheap-as-move               - Use custom code for TargetInstrInfo::isAsCheapAsAMove().
  cyclone                            - Cyclone.
  disable-latency-sched-heuristic    - Disable latency scheduling heuristic.
  exynosm1                           - Samsung Exynos-M1 processors.
  exynosm2                           - Samsung Exynos-M2/M3 processors.
  falkor                             - Qualcomm Falkor processors.
  fp-armv8                           - Enable ARMv8 FP.
  fullfp16                           - Full FP16.
  fuse-aes                           - CPU fuses AES crypto operations.
  fuse-literals                      - CPU fuses literal generation operations.
  kryo                               - Qualcomm Kryo processors.
  lse                                - Enable ARMv8.1 Large System Extension (LSE) atomic instructions.
  lsl-fast                           - CPU has a fastpath logical shift of up to 3 places.
  neon                               - Enable Advanced SIMD instructions.
  no-neg-immediates                  - Convert immediates and instructions to their negated or complemented equivalent when the immediate does not fit in the encoding..
  perfmon                            - Enable ARMv8 PMUv3 Performance Monitors extension.
  predictable-select-expensive       - Prefer likely predicted branches over selects.
  ras                                - Enable ARMv8 Reliability, Availability and Serviceability Extensions.
  rdm                                - Enable ARMv8.1 Rounding Double Multiply Add/Subtract instructions.
  reserve-x18                        - Reserve X18, making it unavailable as a GPR.
  slow-misaligned-128store           - Misaligned 128 bit stores are slow.
  slow-paired-128                    - Paired 128 bit loads and stores are slow.
  spe                                - Enable Statistical Profiling extension.
  strict-align                       - Disallow all unaligned memory access.
  sve                                - Enable Scalable Vector Extension (SVE) instructions.
  thunderx                           - Cavium ThunderX processors.
  thunderx2t99                       - Cavium ThunderX2 processors.
  thunderxt81                        - Cavium ThunderX processors.
  thunderxt83                        - Cavium ThunderX processors.
  thunderxt88                        - Cavium ThunderX processors.
  use-aa                             - Use alias analysis during codegen.
  use-postra-scheduler               - Schedule again after register allocation.
  use-reciprocal-square-root         - Use the reciprocal square root approximation.
  v8.1a                              - Support ARM v8.1a instructions.
  v8.2a                              - Support ARM v8.2a instructions.
  zcm                                - Has zero-cycle register moves.
  zcz                                - Has zero-cycle zeroing instructions.

Use +feature to enable a feature, or -feature to disable it.
For example, llc -mcpu=mycpu -mattr=+feature1,-feature2
'+help' is not a recognized feature for this target (ignoring feature)

[gnzlbg]

@parched

FWIW, the reference I've been following to implement NEON is ARM's NEON Intrinsics Reference which starts with:

This draft document is a reference for the Advanced SIMD Architecture Extension (NEON) Intrinsics for ARMv7 and ARMv8 architectures.

So both "Advanced SIMD" and "NEON" seem to be fine, but @parched argument is very good: if only for consistency with clang and gcc we should probably choose "simd" as the feature name.

---

@alexcrichton what do you think?

I agree in that this should go through the RFC process, but I think that the right point for that is when we propose to stabilize the ARM SIMD intrinsics. There, we might choose "neon", "simd", or something else (who knows), and I wouldn't like to deprecate some name twice.

Also, it is a shame that "neon" is already "stable" for "arm", and it is not my intent here to make it insta-stable for aarch64 as well (but AFAIK there isn't any way to do that).

So my preference would be to: check this as "neon" right now for consistency with "arm". Finish the ARM intrinsics in the stdsimd crate, and write an RFC to stabilize the ARM intrinsics where we can discuss the target feature name (I will propose "simd" as a name as @parched suggests). Afterwards we can somehow deprecate "neon", offer "simd" in parallel, and one or two release cycles later remove "neon" forever.

@parched what do you think of this approach? would you be ok with this?

[alexcrichton]

Thanks for the PR @gnzlbg! I think neon is fine for now, we'll have a round of discussion at least before stabilizing any names here.

[parched]

what? it is called neon by LLVM.

Yes, but the LLVM features aren't designed to be user facing names, they regularly change between releases, that's one of the reasons these whitelists were added in the first place I believe.

@parched what do you think of this approach? would you be ok with this?

Yes, that sounds reasonable to me.

[gnzlbg]

Yes, that sounds reasonable to me.

I've filled an issue in the stdsimd repo to keep track of this: rust-lang/stdarch#125

[gnzlbg]

@parched FWIW you were correct, we should call the Aarch64 feature asimd instead of neon.