Open v01dXYZ opened 3 months ago
@llvm/issue-subscribers-backend-aarch64
Author: None (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 ?
See https://github.com/briansmith/ring/issues/1858#issuecomment-2093675988. This affects many people compiling crypto code with target-cpu=native
/target_cpu=cortex-a72
on such Raspberry Pi devices.
@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).
@davemgreen Monday ping routine
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:
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.
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.
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.
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.
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!
The Crypto Engine is provided only as an extension and is not part of the base license.
This ARM documentation page states:
One very famous example of a cortex-a72 chip without the crypto engine is the one in the Raspberry 4 (related issue: https://github.com/llvm/llvm-project/issues/85699).
Here is the current definition of the features for
cortex-a72
:https://github.com/llvm/llvm-project/blob/53ff002c6f7ec64a75ab0990b1314cc6b4bb67cf/llvm/include/llvm/TargetParser/AArch64TargetParser.h#L573-L575
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