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

[dmgreen]

Summary

The Armv8.6-a and later architecture definitions included AES, SHA2, SHA3 and SM4, but this did not have an effect when specifying -march=armv8.6-a. The did not set preprocessor features (https://godbolt.org/z/1YKad6M8e) or enable the relevant instructions (like eor3 from sha3: https://godbolt.org/z/vY9v4MqvG). Similarly architectures armv8 to armv8.5 defined "+crypto", but this did not effect the -march's, only the -mcpu with those architectures. I believe this was working as intended.

After D141411 we now add the default features for architectures except for "+crypto", which has had the effect of enabling aes/sha2/sha3/sm4 when -march=armv8.6-a is used. This patch removed those crypto features again, going back to how things were before. It also removes the AEK_CRYPTO feature from lower architecture levels, moving it to the cpus that use it. This shouldn't make any changes, but a few extra tests have been added for preprocessor features that have improved since llvm 15.

The -mcpu=ampere1 cpu is the only armv8.6+ cpu at present. For that, the AES, SHA2 and SHA3 features have been re-added to the CPU definition to keep it in-line with the gcc definition from https://github.com/gcc-mirror/gcc/commit/db2f5d661239737157cf131de7d4df1c17d8d88d.

[lenary]

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)

[philipp.tomsich]

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?

[dmgreen]

In 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.

In 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

[dmgreen]

This now also removes AEK_CRYPTO from arch definitions and adds it to the relevant cpu definitions. This shouldn't alter anything, but I've added some extra tests for cases that have improved since llvm 15.

[philipp.tomsich]

Ampere1 supports FEAT_SHA1, FEAT_SHA2, FEAT_SHA3, FEAT_SHA512.
Thanks for spotting this, I'll have to go investigate on the GCC side how that happened (as we make sure that -mcpu=native and -mcpu=ampere1 specify the same .arch).

[tnfchris]

In D141606#4051559, @philipp.tomsich wrote:

Ampere1 supports FEAT_SHA1, FEAT_SHA2, FEAT_SHA3, FEAT_SHA512.
Thanks for spotting this, I'll have to go investigate on the GCC side how that happened (as we make sure that -mcpu=native and -mcpu=ampere1 specify the same .arch).

The guards around the intrinsics currently check for +crypto (as in the option explicitly), this is why most likely the intrinsics isn't enabled. This patch https://gcc.gnu.org/pipermail/gcc-patches/2023-January/609981.html should fix it.

[dmgreen]

Rebase

[dmgreen]

In D141606#4051559, @philipp.tomsich wrote:

Ampere1 supports FEAT_SHA1, FEAT_SHA2, FEAT_SHA3, FEAT_SHA512.

Thanks for confirming. Clang combines sha3 and sha512 into a single sha3 feature, so this should now be in-line.

[dmgreen]

Ping

[lenary]

I think this is the right direction forwards.

An Apple CPU had to recently be marked at the wrong architecture, because clang enabled more crypto due to the architecture version than the CPU had. Maybe this can be resolved now? @t.p.northover @ab https://reviews.llvm.org/D134351 was the relevant revision.

[ilinpv]

Sorry, commit rG5474d7d93271 is not related to this, I put wrong differential revision link

[dmgreen]

In D141606#4074204, @ilinpv wrote:

Sorry, commit rG5474d7d93271 is not related to this, I put wrong differential revision link

Not a problem, I was just about to hit submit :)

clang/docs/ReleaseNotes.rst

- Updated the value returned by ``__has_c_attribute(fallthrough)`` to ``201910L``
   used ``201904L`` (the date the proposal was seen by the committee) by mistake.
   There were no other changes to the attribute behavior.
 
 - Introduced a new record declaration attribute ``__attribute__((enforce_read_only_placement))``
   to support analysis of instances of a given type focused on read-only program
   memory placement. It emits a warning if something in the code provably prevents
   an instance from a read-only memory placement.
 
+- Introduced new attribute ``__attribute__((target_version("cpu_features")))``
+  and expanded the functionality of the existing attribute
+  ``__attribute__((target_clones("cpu_features1","cpu_features2",...)))`` to
+  support Function Multi Versioning on AArch64 target. It detects at runtime
+  which function versions are supported by CPU and calls the one with highest
+  priority. Refer to `clang attributes
+  <https://clang.llvm.org/docs/AttributeReference.html#target-version>`_ for
+  more details.
+
 Windows Support
 ---------------
 - For the MinGW driver, added the options ``-mguard=none``, ``-mguard=cf`` and
   ``-mguard=cf-nochecks`` (equivalent to ``/guard:cf-``, ``/guard:cf`` and
   ``/guard:cf,nochecks`` in clang-cl) for enabling Control Flow Guard checks
   and generation of address-taken function table.
 
 - Switched from SHA1 to BLAKE3 for PDB type hashing / ``-gcodeview-ghash``

llvm/docs/ReleaseNotes.rst

 Changes to the AArch64 Backend
 ------------------------------
 
 * Added support for the Cortex-A715 CPU.
 * Added support for the Cortex-X3 CPU.
 * Added support for the Neoverse V2 CPU.
 * Added support for assembly for RME MEC (Memory Encryption Contexts).
 * Added codegen support for the Armv8.3 Complex Number extension.
+* Implemented `Function Multi Versioning
+  <https://arm-software.github.io/acle/main/acle.html#function-multi-versioning>`_
+  in accordance with Arm C Language Extensions specification. Currently in Beta
+  state.
 
 Changes to the AMDGPU Backend
 -----------------------------
 
 Changes to the ARM Backend
 --------------------------
 
 * Support for targeting Armv2, Armv2A, Armv3 and Armv3M has been removed.