Message ID | 20250724170600.1113476-1-raj.khem@gmail.com |
---|---|
State | New |
Headers | show |
Series | [v4] feature-arm-crypto: Add +nocrypto to -mcpu when crypto not in features | expand |
Hi Khem On 7/24/25 7:06 PM, Khem Raj wrote: > When crypto is not in tune features then add +nocryto to > -mcpu explicitly. This makes the behavior between clang > and gcc match. Currently -mcpu=cortex-a72 has different > behavior in clang and gcc in terms of what features are > considered default. Clang enables different set of common > features than gcc on other hand. For example clang > enables crypto with default set but gcc > does not, gcc recommends to disable unavailable extensions > in -mcpu [1] explicitly. crypto is optional on cortex-a53 > and cortex-a72. This is not as common but Broadcom > SOCs in raspberrypi3/4 have dropped crypto for cost > reasons [2]. This results in illegal instruction > traps [3] [4] when building components e.g. chromium, > qtwebengine, weston etc. with clang using -mcpu=cortex-a72 > for rpi4 target. > > Adding +nocrypto makes clang behave like gcc does today. We > do have separate tune if crypto enabled cortex-a72 cores > are to be targeted (cortexa72-cryto) as DEFAULTTUNE > > They are added to default feature file since crypto extension > is available in multiple arm architecture versions e.g. armv8, > armv9. It is optional extension as per spec [5] > > Extensions can be enabled and disabled with -mcpu using the same > syntax as with -march, and have same effect thats why it is > intrumented via TUNE_CCARGS_MARCH_OPTS > I believe this is incorrect. TUNE_CCARGS_MARCH_OPTS applies to -march, not -mcpu. If that is correct, then The whole explanation should be targeting march and not mcpu as we're changing march and not mcpu (though I assume they trickle from march to mcpu inside the toolchain). Cheers, Quentin
On Thu, Jul 24, 2025 at 10:10 AM Quentin Schulz <quentin.schulz@cherry.de> wrote: > > Hi Khem > > On 7/24/25 7:06 PM, Khem Raj wrote: > > When crypto is not in tune features then add +nocryto to > > -mcpu explicitly. This makes the behavior between clang > > and gcc match. Currently -mcpu=cortex-a72 has different > > behavior in clang and gcc in terms of what features are > > considered default. Clang enables different set of common > > features than gcc on other hand. For example clang > > enables crypto with default set but gcc > > does not, gcc recommends to disable unavailable extensions > > in -mcpu [1] explicitly. crypto is optional on cortex-a53 > > and cortex-a72. This is not as common but Broadcom > > SOCs in raspberrypi3/4 have dropped crypto for cost > > reasons [2]. This results in illegal instruction > > traps [3] [4] when building components e.g. chromium, > > qtwebengine, weston etc. with clang using -mcpu=cortex-a72 > > for rpi4 target. > > > > Adding +nocrypto makes clang behave like gcc does today. We > > do have separate tune if crypto enabled cortex-a72 cores > > are to be targeted (cortexa72-cryto) as DEFAULTTUNE > > > > They are added to default feature file since crypto extension > > is available in multiple arm architecture versions e.g. armv8, > > armv9. It is optional extension as per spec [5] > > > > Extensions can be enabled and disabled with -mcpu using the same > > syntax as with -march, and have same effect thats why it is > > intrumented via TUNE_CCARGS_MARCH_OPTS > > > > I believe this is incorrect. TUNE_CCARGS_MARCH_OPTS applies to -march, > not -mcpu. Well, it applied to both in cpu specific tune files ( e.g. tune-cortexa55.inc ) we override arch specific TUNE_FEATURES and replace with then CPU specific TUNE_FEATURES This results in TUNE_CCARGS_MARCH being empty and TUNE_CCARGS_MARCH_OPTS are still valid and they can appended to TUNE_CCARGS in conf/machine/include/arm/arch-arm.inc we have TUNE_CCARGS .= "${TUNE_CCARGS_MARCH}${TUNE_CCARGS_MARCH_OPTS }" Due to include order this append appears after the TUNE_CCARGS is appended with -mcpu option in a mcpu tune file e.g. tune-cortexa55.inc TUNE_CCARGS .= "${@bb.utils.contains('TUNE_FEATURES', 'cortexa55', ' -mcpu=cortex-a55', '', d)}" so in the end we get -mcpu=cortex-a55+nocrypto or -mcpu=cortex-a55+crypto in this case if you were optimizing widely for an arch then the effect will be via TUNE_CCARGS_MARCH and TUNE_CCARGS_MARCH_OPTS I do wonder whether renaming both TUNE_CCARGS_MARCH and TUNE_CCARGS_MARCH_OPTS to something more generic could help lessen the confusion. It's there because we have -march and introduced -mcpu options later on. > > If that is correct, then The whole explanation should be targeting march > and not mcpu as we're changing march and not mcpu (though I assume they > trickle from march to mcpu inside the toolchain). > your assumption is right. Although, now thinking of this a bit more. I think we can rename the TUNE_CCARGS_MARCH_OPTS as I suggested in prior email to something like TUNE_CCARGS_CPU_EXTENSIONS but in the end they are march options from gcc/clang's perspective, its just they are instrumented via either march or mcpu hope that helps. > Cheers, > Quentin
diff --git a/meta/conf/machine/include/arm/feature-arm-crypto.inc b/meta/conf/machine/include/arm/feature-arm-crypto.inc index aade6ce08d5..8202de78e46 100644 --- a/meta/conf/machine/include/arm/feature-arm-crypto.inc +++ b/meta/conf/machine/include/arm/feature-arm-crypto.inc @@ -2,4 +2,5 @@ # armv8-a, armv8.1-a, armv8.3-a, armv8.4-a, armv8.5-a, armv8.6-a, and armv8-r TUNEVALID[crypto] = "Enable cryptographic instructions for ARMv8" -TUNE_CCARGS_MARCH_OPTS .= "${@bb.utils.contains('TUNE_FEATURES', 'crypto', '+crypto', '', d)}" +# See https://github.com/llvm/llvm-project/issues/85699 +TUNE_CCARGS_MARCH_OPTS .= "${@bb.utils.contains('TUNE_FEATURES', 'crypto', '+crypto', '+nocrypto', d)}"
When crypto is not in tune features then add +nocryto to -mcpu explicitly. This makes the behavior between clang and gcc match. Currently -mcpu=cortex-a72 has different behavior in clang and gcc in terms of what features are considered default. Clang enables different set of common features than gcc on other hand. For example clang enables crypto with default set but gcc does not, gcc recommends to disable unavailable extensions in -mcpu [1] explicitly. crypto is optional on cortex-a53 and cortex-a72. This is not as common but Broadcom SOCs in raspberrypi3/4 have dropped crypto for cost reasons [2]. This results in illegal instruction traps [3] [4] when building components e.g. chromium, qtwebengine, weston etc. with clang using -mcpu=cortex-a72 for rpi4 target. Adding +nocrypto makes clang behave like gcc does today. We do have separate tune if crypto enabled cortex-a72 cores are to be targeted (cortexa72-cryto) as DEFAULTTUNE They are added to default feature file since crypto extension is available in multiple arm architecture versions e.g. armv8, armv9. It is optional extension as per spec [5] Extensions can be enabled and disabled with -mcpu using the same syntax as with -march, and have same effect thats why it is intrumented via TUNE_CCARGS_MARCH_OPTS [1] https://gcc.gnu.org/onlinedocs/gcc/ARM-Options.html#index-mcpu-2 [2] https://forums.raspberrypi.com/viewtopic.php?f=63&t=207888#p1332960 [3] https://github.com/llvm/llvm-project/issues/85699 [4] https://github.com/llvm/llvm-project/issues/90365 [5] https://developer.arm.com/documentation/109697/2025_06/Feature-descriptions/The-Armv9-0-architecture-extension Signed-off-by: Khem Raj <raj.khem@gmail.com> Cc: Quentin Schulz <quentin.schulz@cherry.de> --- v2: Move the change to feature-arm-crypto.inc. v3: Adjust commit message for clarity on reason to be in common feature file. v4: Address commit msg comments, add section on why use TUNE_CCARGS_MARCH_OPTS meta/conf/machine/include/arm/feature-arm-crypto.inc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)