[PATCH] D134352: [AArch64] Add Neoverse V2 CPU support

[David Sherwood via Phabricator via cfe-commits]

david-arm marked 3 inline comments as done.
david-arm added inline comments.


================
Comment at: llvm/include/llvm/Support/AArch64TargetParser.def:239
+AARCH64_CPU_NAME("neoverse-v2", ARMV9A, FK_NEON_FP_ARMV8, false,
+                 (AArch64::AEK_SVE | AArch64::AEK_SVE2 | AArch64::AEK_SSBS |
+                  AArch64::AEK_FP16 | AArch64::AEK_BF16 | AArch64::AEK_RAND |
----------------
Matt wrote:
> Should `AEK_SVE2BITPERM` be present? (Noticed that N2 has ` AArch64::AEK_SVE2 | AArch64::AEK_SVE2BITPERM`).
Good spot! I've made this consistent with the definition in AArch64.td.


================
Comment at: llvm/lib/Target/AArch64/AArch64.td:1111
+                                       FeaturePerfMon, FeatureETE, FeatureMatMulInt8,
+                                       FeatureNEON, FeatureSVE2BitPerm, FeatureFP16FML,
+                                       FeatureMTE, FeatureRandGen];
----------------
Matt wrote:
> `FeatureNEON` may be redundant (note that it's in `HasV8_3aOps`).
> 
> OTOH, `NeoverseV1` also has `FeatureCrypto`: is this no longer the case for `NeoverseV2`?
HasV8_3aOps does imply FeatureNEON, but only indirectly through FeatureComplxNum. I thought it was clearer to explicitly add it, since it doesn't do any harm.

With regards to FeatureCrypto, I am following the precedent set for Cortex-A510, Cortex-A710 and Cortex-X2 where it also wasn't enabled by default. The user can always enable crypto with -mcpu=neoverse-v2+crypto if required.


================
Comment at: llvm/lib/Target/AArch64/AArch64Subtarget.cpp:202
   case NeoverseN2:
+  case NeoverseV2:
     PrefFunctionLogAlignment = 4;
----------------
Matt wrote:
> Are `CacheLineSize` (`= 0` by default) and `MaxInterleaveFactor` (`= 2` by default) the same / correct for both N2 and V2?
I don't know the answer for neoverse-v2 I'm afraid, and neoverse-n2 isn't part of this patch. Performance tuning can be done in a later patch I think.


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

https://reviews.llvm.org/D134352