[PATCH] D109825: [AArch64]Enabling Cortex-A510 Support

Wed Sep 15 06:50:09 PDT 2021

dmgreen requested changes to this revision.
dmgreen added a comment.
This revision now requires changes to proceed.

> As I did the downstream work for this, I'm happy with it to go in in this form.

This doesn't seem.. wise. Please make sure the reviews you do are at a sufficient quality, and it is probably best not to review patches you write yourself.

================
Comment at: llvm/include/llvm/Support/AArch64TargetParser.def:179
                   AArch64::AEK_DOTPROD ))
+AARCH64_CPU_NAME("cortex-a510", ARMV8_3A, FK_CRYPTO_NEON_FP_ARMV8, false,
+                 (AArch64::AEK_MTE | AArch64::AEK_SVE2 |
----------------
Why is this 8.3? The TRM (https://developer.arm.com/documentation/101604/0003/The-Cortex-A510--core) describes it as implementing the 9.0-A architecture.

================
Comment at: llvm/lib/Target/AArch64/AArch64.td:1055

+def ProcCortexA510 : SubtargetFeature<"cortex-a510", "ARMProcFamily",
+                                      "CortexA510", "Cortex-A510 ARM processors", [
----------------
This should be in some sort of order, next to the Cortex-A55. It should probably be called ProcA510 for consistency too.

================
Comment at: llvm/lib/Target/AArch64/AArch64.td:1214
 def : ProcessorModel<"neoverse-v1", CortexA57Model, [ProcNeoverseV1]>;
+def : ProcessorModel<"cortex-a510", CortexA57Model, [ProcCortexA510]>;
 def : ProcessorModel<"exynos-m3", ExynosM3Model, [ProcExynosM3]>;
----------------
Ordering. Please use the CortexA55Model. This is not an out of order core like the A57.

================
Comment at: llvm/lib/Target/AArch64/AArch64Subtarget.cpp:196
     break;
+  case CortexA510:
+    PrefFunctionLogAlignment = 4;
----------------
This can be the same case block as the A53 and A55.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109825