[AArch64] Cortex-A72: Default Model should not enable crypto features

[v01dXYZ]

The Crypto Engine is provided only as an extension and is not part of the base license.

This ARM documentation page states:

The optional Cryptography engine is not included in the base product of the processor. ARM requires licensees to have contractual rights to obtain the Cortex-A72 processor Cryptography engine.

One very famous example of a cortex-a72 chip without the crypto engine is the one in the Raspberry 4 (related issue: #85699).

Here is the current definition of the features for cortex-a72:

llvm-project/llvm/include/llvm/TargetParser/AArch64TargetParser.h

Lines 573 to 575 in 53ff002

	{"cortex-a72", ARMV8A,
	AArch64::ExtensionBitset(
	{AArch64::AEK_AES, AArch64::AEK_SHA2, AArch64::AEK_CRC})},

It's likely that's also the case for other models.

[0] Arm Cortex-A Processor Comparison Table: https://developer.arm.com/-/media/Arm%20Developer%20Community/PDF/Cortex-A%20R%20M%20datasheets/Arm%20Cortex-A%20Comparison%20Table_v4.ashx

[llvmbot]

@llvm/issue-subscribers-backend-aarch64

Author: None (v01dXYZ)

The Crypto Engine is provided only as an extension and is not part of the base license.

This ARM documentation page states:

> The optional Cryptography engine is not included in the base product of the processor. ARM requires licensees to have contractual rights to obtain the Cortex-A72 processor Cryptography engine.

One very famous example of a cortex-a72 chip without the crypto engine is the one in the Raspberry 4 (related issue: #85699).

Here is the current definition of the features for cortex-a72:

llvm-project/llvm/include/llvm/TargetParser/AArch64TargetParser.h

Lines 573 to 575 in 53ff002

	{"cortex-a72", ARMV8A,
	AArch64::ExtensionBitset(
	{AArch64::AEK_AES, AArch64::AEK_SHA2, AArch64::AEK_CRC})},

It's likely that's also the case for other models.

[0] Arm Cortex-A Processor Comparison Table: https://developer.arm.com/-/media/Arm%20Developer%20Community/PDF/Cortex-A%20R%20M%20datasheets/Arm%20Cortex-A%20Comparison%20Table_v4.ashx

[v01dXYZ]

@davemgreen What do you think of this problem ? Is there a reason to consider some models provide crypto extensions per default ? Is that even right to "fix" this problem as it could be a breaking change ? Is that easily fixable as it could be also the case for many other models and picking for this one would introduce inconsistencies with the other ones ?

[briansmith]

See briansmith/ring#1858 (comment). This affects many people compiling crypto code with target-cpu=native/target_cpu=cortex-a72 on such Raspberry Pi devices.

[v01dXYZ]

@davemgreen ping (I ask you bc you are the one that changed that line feel free to tell me if someone else is more appropriate or have more time to handle this issue).

[v01dXYZ]

@davemgreen Monday ping routine

[davemgreen]

Hi - sorry for the lack of reply. Unfortunately I don't seem to receive any notifications for issues with backend:AArch64. They are the only ones I really want to see! But for whatever reason they don't get through, I get everything else :(

As far as I understand the rules we try and keep are fairly simple, if not universally followed:

• For -march=armv8.x-a we enable the minimal set of features (so all required extensions). Optional features can be added with +feat.
• For -mcpu=xyz, we enable the maximal set of features for the cpu (so long as they are relatively common), which can be disabled with +nofeat. This often works well especially for M and R class cpus, where many features are optional but often supported. For A-class cpu's there are often less interesting features that are optional. The idea is that users get decent performance by default, and if they have less features can turn down the default.

Unfortunately GCC didn't follow that scheme at the time for crypto instructions, and had them disabled by default. Clang did, so there was a difference in whether crypto was enabled. We did not decide to retro-actively change old CPU definitions (it could be a breaking change), but going forward "Armv-9" cpus have been changed to not include crypto by default.

@smithp35 might remember more of the reasons too, if I have any of it wrong.

[davemgreen]

The -mcpu=native not detecting crypto correctly is probably something that we should fix, if that doesn't happen already. It looks like there is already some parsing of cpuinfo in Host.cpp.

[briansmith]

Unfortunately GCC didn't follow that scheme at the time for crypto instructions, and had them disabled by default. Clang did, so there was a difference in whether crypto was enabled. We did not decide to retro-actively change old CPU definitions (it could be a breaking change), but going forward "Armv-9" cpus have been changed to not include crypto by default.

I do think it is worth changing the existing CPU definitions to exclude optional features by default. I also think it would be great to document the general policy, which hopefully would become "never enable optional features by default."

If that can't be done, then there should be a way to disable all optional features on the clang command line, without knowing the names of the optional features. e.g. an "-optional" modifier to -mcpu or a separate flag. This way, other tools that want to enforce a "no optional features enabled by default" policy can easily integrate with clang, instead of having to track each CPU definition.

[workingjubilee]

For -mcpu=xyz, we enable the maximal set of features for the cpu (so long as they are relatively common),

I'm not sure "the CPU for the Raspberry Pi 4 doesn't have this, despite being a Cortex-A72" implies this feature is truly "relatively common" for Cortex-A72s.

[workingjubilee]

Unfortunately GCC didn't follow that scheme at the time for crypto instructions, and had them disabled by default. Clang did, so there was a difference in whether crypto was enabled. We did not decide to retro-actively change old CPU definitions (it could be a breaking change), but going forward "Armv-9" cpus have been changed to not include crypto by default.

@davemgreen "This is a breaking change" has never actually stopped you before. LLVM has changed its internal definitions of target features and demanded downstreams keep up, even changing definitions of 20-year-old SSE features. Yes, I am including Clang in those downstreams. Are you saying that because Clang made a bad decision, that all downstreams of LLVM are forced to live with it? Are you suddenly promising stability for all the target features that you currently support? Because that's news to me!