Should the `p64` functions be gated under `aes`?

[sayantn]

Currently we gate a lot of p64 functions under aes, but I couldn't spot any pattern on when we add aes. We have aes in the following functions

• vld1{q}_p64{_x2,_x3,_x4}
• vld{1,2,3,4}{q}{,_dup, _lane}_p64
• vmull{_high}_p64
• vs{r,l}i{q}_n_p64
• vst1{q}_p64
• vst1{q}_p64_{x2,x3,x4}
• vst{2,3,4}{_lane, q_lane,}_p64
• vcreate_p64
• vreinterpret_*_p{64,128}
• vreinterpret_p{64,128}_*
• vset{q}_lane_p64

whereas, the following functions, even though they have a p64 argument, don't have aes

• vceq{z}{q}_p64
• vcopy{q}_lane{q}_p64
• vdup{q}_lane{q}_p64
• vextq_p64
• vldap1{q}_lane_p64
• vtrn{1,2}q_p64,
• vtst{q}_p64
• vuzp{1,2}q_p64
• vzip{1,2}q_p64
• vadd{q}_p64
• vget_{high,low}_p64
• vget{q}_lane_p64
• vreinterpret{q}_{f32,f64,s64,u64}_p64

The only reference of this in ACLE I could find was the fact that GCC gates all p64 functions and types under target ("fpu=crypto-neon-fp-armv8"), but afaik this does not translate to the Rust feature aes. I believe the closest would be the LLVM codegen feature crypto.

@rustbot ping arm

[rustbot]

Hey Arm-interested people! This issue could use some guidance on how it can be
resolved on Arm platforms.
Could one of you weigh in please? In case it's useful, here are some
instructions for tackling these sorts of bugs.
Thanks!

cc @adamgemmell @davidtwco @hug-dev @Jamesbarford @joaopaulocarreiro @lqd @raw-bin @Stammark

[adamgemmell]

aes represents FEAT_AES and FEAT_PMULL - from the architecture reference manual, these just add 4 new aes* instructions.

Edit: and the p64 variants of PMULL/PMULL2

Clang's header doesn't seem to gate any poly types on the aes feature, so I suspect we don't need it either. Many of the intrinsics with the aes feature are nops or just load/stores, for which the aes feature shouldn't matter. I'm going to try removing it from many of the intrinsics and see what breaks.

The crypto feature basically means aes+sha2 (FEAT_AES, FEAT_PMULL, FEAT_SHA1 and FEAT_SHA256 in the architecture).

[sayantn]

We should note that gcc does not gate the p64 type under crypto -- it gates it under -fpu=crypto-neon-fp-armv8, which I do not think is equivalent to the crypto feature (-march=armv8-a+crypto+simd doesn't imply -mfpu=crypto-neon-fp-armv8 or vice versa, for example). Clang doesn't seem to any such thing, at least I couldn't find anything similar in arm_neon.h. Honestly I do not know what would be the Rust analogue of this option.

[adamgemmell]

Where did you find that gate? I can't reproduce it - the typedef in arm_neon.h works fine, and so does the underlying builtin without the header: https://godbolt.org/z/z5GohYc4e

[sayantn]

For some weird reason, the gate is only there for A32 (https://github.com/gcc-mirror/gcc/blob/a9883211f8891a3cb5966cbe2fbec35e25631d83/gcc/config/arm/arm_neon.h#L82-L85), everything is gated under fpu=crypto-neon-fp-armv8.

edit: looks like the type gate doesn't actually work, but the function gates work https://godbolt.org/z/aqjj39aG9

[adamgemmell]

Unless I'm misunderstanding, the #pragma GCC target ("fpu=crypto-neon-fp-armv8") line there just specifies what fpu hardware is available during the functions (similar to how rust's target_features behave). The error in that example is because the always_inline property fails due to a difference in properties on the caller

It looks like #pragma GCC target only applies to functions, according to this. It looks like the types themselves can exist without the fpu option, so my patch is probably fine? Target specific support for the GCC backend is quite bare right now so I'm not sure how easy this will be to properly test, but we could add a note to the ARM Neon intrinsics tracking issue so that it's addressed properly before stabilisation.