dmgreen added inline comments.
================ Comment at: llvm/include/llvm/Support/AArch64TargetParser.def:181 + AArch64::AEK_SVE2BITPERM | AArch64::AEK_BF16)) AARCH64_CPU_NAME("cortex-a78c", ARMV8_2A, FK_CRYPTO_NEON_FP_ARMV8, false, (AArch64::AEK_FP16 | AArch64::AEK_DOTPROD | AArch64::AEK_RCPC | ---------------- I would go after this, it being a a78-like cpu it's probably worth keeping the two together. ================ Comment at: llvm/include/llvm/Support/AArch64TargetParser.def:159-160 AArch64::AEK_RCPC | AArch64::AEK_SSBS)) +AARCH64_CPU_NAME("cortex-a710", ARMV9A, FK_NEON_FP_ARMV8, false, + (AArch64::AEK_MTE | AArch64::AEK_PAUTH | AArch64::AEK_FLAGM | + AArch64::AEK_SB | AArch64::AEK_I8MM | AArch64::AEK_FP16FML | ---------------- dmgreen wrote: > lenary wrote: > > dmgreen wrote: > > > Natural order would be better I think, where the new A710 is added after > > > the A78. > > > > > > FlagM should already be included as a part of 8.4, so isn't needed here. > > > Should BFloat16 be added? > > > FlagM should already be included as a part of 8.4, so isn't needed here. > > > > FlagM is not part of the `AARCH64_ARCH("armv9-a", ARMV9A ...)` definition, > > nor part of the equivalents for armv8.4a and armv8.5a, so it was added > > explicitly here. > > > > > Should BFloat16 be added? > > > > Yes > > > > > > > FeatureFlagM is included in HasV8_4aOps. So should already be included (much > like something like dotprod). > > Why AEK_FLAGM isn't part of ARMV8_4A I don't know. And why we have two maps > for the same set of information... I think that AEK_FLAGM should be included in architecture that is 8.4+. Then it shouldn't be needed here. Perhaps lets do that as a separate commit though. ================ Comment at: llvm/include/llvm/Support/ARMTargetParser.def:315 + ARM::AEK_I8MM)) ARM_CPU_NAME("cortex-a78c", ARMV8_2A, FK_CRYPTO_NEON_FP_ARMV8, false, ARM::AEK_FP16 | ARM::AEK_DOTPROD) ---------------- Ditto and same in other places. ================ Comment at: llvm/lib/Target/AArch64/AArch64Subtarget.cpp:87 PrefFunctionLogAlignment = 4; - VScaleForTuning = 1; break; ---------------- Make sure you keep this in. It looks like it might have been removed by accident. ================ Comment at: llvm/unittests/Support/TargetParserTest.cpp:278 "8-A"), + ARMCPUTestParams("cortex-a710", "armv9-a", "neon-fp-armv8", + ARM::AEK_SEC | ARM::AEK_MP | ARM::AEK_VIRT | ---------------- Ordering. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113256/new/ https://reviews.llvm.org/D113256 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits