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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
