[PATCH] D136589: [AArch64] Add support for the Cortex-X3 CPU

Dave Green via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Nov 2 04:32:41 PDT 2022


dmgreen added inline comments.


================
Comment at: clang/docs/ReleaseNotes.rst:696
-- Add driver and tuning support for Neoverse V2 via the flag ``-mcpu=neoverse-v2``.
-  Native detection is also supported via ``-mcpu=native``.
 
----------------
vhscampos wrote:
> tschuett wrote:
> > dmgreen wrote:
> > > tschuett wrote:
> > > > What happened with native detection?
> > > I think I prefer the new wording, FWIW, as we add more CPUs. The native detection should just be automatically implied.
> > But the word `native` disappeared.
> I've looked at https://reviews.llvm.org/D134352 and also in code related to -mcpu=native implementation and I couldn't find anything related to Neoverse V2. Can you please point me to where that release note relates to?
I think the comment just meant that -mcpu=native on a neoverse-v2 would correctly turn into -mcpu=neoverse-v2 because the code added to Host.cpp. That should pretty much always be true, so I'm not sure it deserves to be called out.
It sounds fine either way though, if you do think it is important.


================
Comment at: llvm/unittests/Support/TargetParserTest.cpp:1083
+                             AArch64::AEK_SVE | AArch64::AEK_SVE2 |
+                             AArch64::AEK_SVE2BITPERM | AArch64::AEK_SB |
+                             AArch64::AEK_PROFILE | AArch64::AEK_PERFMON |
----------------
vhscampos wrote:
> dmgreen wrote:
> > Should SSBS be included in here? As far as I understand it is enabled by default in 8.5-A. That should mean it's included automatically, but that can be said for a lot of the other features here too.
> > Same for AEK_FLAGM, which I noticed as a difference between A715 and X3.
> SSBS is an optional feature for all Armv8-A architectures.
> 
> But FlagM should be present here as it's mandatory in v8.4-A. Fixed.
But if the CPU has SSBS, should it not be present? It seems to be mentioned in the TRM.

According to the reference I have it became mandatory in 8.5-A, so it should be enabled automatically and I'm not sure adding the AEK feature will have much effect. It sounds better to add it for consistency though, if we can.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136589



More information about the cfe-commits mailing list