[PATCH] D141606: [AArch64] Remove AES, SHA2, SHA3 and SM4 features from armv8.6-a+

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

In D141606#4049025 <https://reviews.llvm.org/D141606#4049025>, @lenary wrote:

> I'm slightly confused by this, tbh, as I don't know why we include CRYPTO in v8.5a but not v8.6a to v8.9a. (I did make the explicit decision not to include it in v9a cores, which is mostly followed except the mislabelled v9a core)

It appears if I have this right that the +crypto in the architecture definitions was only enabling the features in -mcpus options, not -march=.. options. https://godbolt.org/z/4eY1bh9q1 shows it not setting preprocessor macros. Crypto was excluded from arch features in D141411 <https://reviews.llvm.org/D141411>.

In D141606#4049048 <https://reviews.llvm.org/D141606#4049048>, @philipp.tomsich wrote:

> I reread the ARMv8-A architecture today and am a little confused now: it looks as if the crypto features (i.e., FEAT_AES and friends) are always optional.
> This is not surprising as the the section "The ARMv8 Cryptographic Extension" states that "[t]he presence of the Cryptographic Extension in an implementation is subject to export license controls."
>
> So should we remove these from the defaults for all architecture-levels and include them in the relevant cores only?
> Or am I reading the architecture wrong?

Crypto features are optional, yes. They have been enabled by default in cpu implementations in llvm in armv8 cpus (but that worked differently in gcc). The idea to remove the default crypto feature too sounds like a good one. I can give it a go and see if anything looks troubling.

@philipp.tomsich One extra thing that I noticed in GCC is that SHA3 seems to be enabled, where SHA2 isn't (https://godbolt.org/z/1h1WeTTj5), which looks like a bug. Do you know if that should enable both SHA2 and SHA3, or enable just SHA2 in ampere1? Thanks


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141606/new/

https://reviews.llvm.org/D141606