[PATCH] D117112: [AArch64] Support for Ampere1 core

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

You could consider adding some scheduling tests, like those in llvm/test/tools/llvm-mca/AArch64/Cortex/A55-basic-instructions.s. In the past we have tended not to add them for schedules, so they are not necessary, but can be useful for checking the details are as you expect.



================
Comment at: llvm/include/llvm/Support/AArch64TargetParser.def:288
+AARCH64_CPU_NAME("ampere1", ARMV8_6A, FK_CRYPTO_NEON_FP_ARMV8, false,
+                 (AArch64::AEK_BF16 | AArch64::AEK_DOTPROD | AArch64::AEK_FP16 |
+                  AArch64::AEK_I8MM | AArch64::AEK_MTE | AArch64::AEK_RAS |
----------------
I believe a lot of these (BF16, DotProd etc) are mandatory in armv8.6 and should already be included without needing to specify them here.


================
Comment at: llvm/lib/Target/AArch64/AArch64.td:1191
+def : ProcessorModel<"ampere1", Ampere1Model, ProcessorFeatures.Ampere1,
+		     [TuneAmpere1]>;
+
----------------
The formatting here is a bit off, compared to all the others.


================
Comment at: llvm/lib/Target/AArch64/AArch64SchedAmpere1.td:14
+
+//===----------------------------------------------------------------------===//
+
----------------
This line doesn't add a lot :)


================
Comment at: llvm/lib/Target/AArch64/AArch64SchedAmpere1.td:16
+
+// The Ampere-1 core is anout-of-order micro-architecture.  The front
+// end has branch prediction, with a 10-cycle recovery time from a
----------------
an out-of-order micro-architecture


================
Comment at: llvm/lib/Target/AArch64/AArch64SchedPredAmpere.td:17
+// Check for a LSL shift <= 4
+def AmpereCheapLSL : MCSchedPredicate<
+				CheckAny<[CheckShiftBy0,
----------------
I don't deal with these predicates a lot, but the formatting looks like it could be better. Is it using tabs?


================
Comment at: llvm/lib/Target/AArch64/AArch64Subtarget.h:83
+    TSV110,
+    Ampere1
   };
----------------
This list can be alphabetical.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117112