[AArch64]Enabling Cortex-A510 Support

[mubashar_]

Summary

This patch enables support for Cortex-A510 CPUs.

[MarkMurrayARM]

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

[dmgreen]

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.

llvm/include/llvm/Support/AArch64TargetParser.def

179

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.

llvm/lib/Target/AArch64/AArch64.td

1051

This should be in some sort of order, next to the Cortex-A55. It should probably be called ProcA510 for consistency too.

1210

Ordering. Please use the CortexA55Model. This is not an out of order core like the A57.

llvm/lib/Target/AArch64/AArch64Subtarget.cpp

196

This can be the same case block as the A53 and A55.

[mubashar_]

Updated release notes to solve a merge conflict.

[MarkMurrayARM]

In D109825#3001622, @dmgreen wrote:

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.

Very good point!

I reduce this to the claim that it has been stable downstream for a couple of weeks now.

[xgupta]

Where are the other patches? Or diff is not uploaded correctly.

[mubashar_]

@xgupta those would be under the history tab.

[xgupta]

Yeah, those changes are in history, But we need them in the present to commit :)

The patch is incorrectly updated, did you followed https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-command-line?

[mubashar_]

@xgupta I followed the request review via web interface section of that page. Let me look into it.

[mubashar_]

Added files that were missing from previous diff.

[xgupta]

It misses llvm/test/CodeGen/AArch64 and llvm/test/MC/AArch64 testcases changes, see for example https://reviews.llvm.org/D36667 (Cortex-A55 support).

[lenary]

In D109825#3055142, @xgupta wrote:

It misses llvm/test/CodeGen/AArch64 and llvm/test/MC/AArch64 testcases changes, see for example https://reviews.llvm.org/D36667 (Cortex-A55 support).

I'm not convinced this is required - those tests are to ensure that the extensions (rcpc, dotprod) can be enabled either from a -march, or from a -mcpu=<cpu> with a cpu that contains that extension -- they are not testing that the extension is enabled from *all* cpus that contain said extension.

[xgupta]

oh thanks for the clarification, I am not aware of it.

[dmgreen]

Thanks for the updates.

In D109825#3055264, @lenary wrote:

In D109825#3055142, @xgupta wrote:

It misses llvm/test/CodeGen/AArch64 and llvm/test/MC/AArch64 testcases changes, see for example https://reviews.llvm.org/D36667 (Cortex-A55 support).

I'm not convinced this is required - those tests are to ensure that the extensions (rcpc, dotprod) can be enabled either from a -march, or from a -mcpu=<cpu> with a cpu that contains that extension -- they are not testing that the extension is enabled from *all* cpus that contain said extension.

Yeah I agree. It would be useful to add some tests for -mcpu=cortex-a510+crypto and -mcpu=cortex-a510+nocrypto, to make sure they do what is expected.

clang/docs/ReleaseNotes.rst

86

Can you add an Arm section below and move this to it.

[mubashar_]

Added crypto and non-crypto related tests for mcpu in aarch64-cpus.c

[dmgreen]

In D109825#3060796, @mubashar_ wrote:

Added crypto and non-crypto related tests for mcpu in aarch64-cpus.c

Thanks, but did this change miss being updated in the review?

clang/docs/ReleaseNotes.rst

86

This still needs doing. Also please add a line to the arm section saying "Added support for the Armv9-A, Armv9.1-A and Armv9.2-A architectures."

It looks like there are some llvm release notes in llvm/docs/ReleaseNotes.rst too, that could have a line added.

llvm/include/llvm/Support/AArch64TargetParser.def

131

AEK_PAUTH is twice here.

[mubashar_]

Added crypto and no-crypto tests for -mcpu and added new section for Arm and AArch64 support in release notes.

[mubashar_]

Corrected PAUTH duplication mistake.

[mubashar_]

clang/docs/ReleaseNotes.rst

86

I didn't add support for these architectures. This may already be in its own commit.

[dmgreen]

Thanks for the changes. Nice working pulling this into shape.

LGTM

llvm/include/llvm/Support/AArch64TargetParser.def

131

Can you make sure you remove the second PAUTH here before committing.

[lenary]

clang/docs/ReleaseNotes.rst

86

I'll do armv9 in a separate NFC commit

[jansvoboda11]

This seems to be breaking the macOS bot, can you take a look? https://green.lab.llvm.org/green/job/clang-stage1-RA/24735/

[lenary]

In D109825#3066853, @jansvoboda11 wrote:

This seems to be breaking the macOS bot, can you take a look? https://green.lab.llvm.org/green/job/clang-stage1-RA/24735/

Will do.

clang/docs/ReleaseNotes.rst

- Support has been added for the following processors (``-mcpu`` identifiers in parentheses):
   - RISC-V SiFive E21 (``sifive-e21``).
   - RISC-V SiFive E24 (``sifive-e24``).
   - RISC-V SiFive E34 (``sifive-e34``).
   - RISC-V SiFive S21 (``sifive-s21``).
   - RISC-V SiFive S51 (``sifive-s51``).
   - RISC-V SiFive S54 (``sifive-s54``).
   - RISC-V SiFive S76 (``sifive-s76``).
 
+- Support has been added for the following processors (command-line identifiers in parentheses):
+  - Arm Cortex-A510 (cortex-a510).
+
 Removed Compiler Flags
 -------------------------
 
 - ...
 
 New Pragmas in Clang
 --------------------
 

clang/test/Driver/aarch64-cpus.c

 // RUN: %clang -target aarch64 -march=armv8.2-a+noras -### -c %s 2>&1 | FileCheck -check-prefix=V82ANORAS -check-prefix=GENERICV82A %s
 // V82ANORAS: "-target-feature" "-ras"
 
 // RAS is off by default for v8a, but can be enabled by +ras (this is not architecturally valid)
 // RUN: %clang -target aarch64 -march=armv8a+ras  -### -c %s 2>&1 | FileCheck -check-prefix=V8ARAS -check-prefix=GENERIC %s
 // RUN: %clang -target aarch64 -march=armv8-a+ras -### -c %s 2>&1 | FileCheck -check-prefix=V8ARAS -check-prefix=GENERIC %s
 // V8ARAS: "-target-feature" "+ras"
 
+// RUN: %clang -target aarch64 -mcpu=cortex-a510 -### -c %s 2>&1 | FileCheck -check-prefix=CORTEX-A510 %s
+// CORTEX-A510: "-cc1" {{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "cortex-a510"
+
 // ================== Check whether -march accepts mixed-case values.
 // RUN: %clang -target aarch64_be -march=ARMV8.1A -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV81A-BE %s
 // RUN: %clang -target aarch64_be -march=ARMV8.1-A -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV81A-BE %s
 // RUN: %clang -target aarch64 -mbig-endian -march=Armv8.1A -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV81A-BE %s
 // RUN: %clang -target aarch64 -mbig-endian -march=Armv8.1-A -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV81A-BE %s
 // RUN: %clang -target aarch64_be -mbig-endian -march=ARMv8.1a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV81A-BE %s
 // RUN: %clang -target aarch64_be -mbig-endian -march=ARMV8.1-a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV81A-BE %s
 // GENERICV81A-BE: "-cc1"{{.*}} "-triple" "aarch64_be{{.*}}" "-target-cpu" "generic" "-target-feature" "+neon" "-target-feature" "+v8.1a"

llvm/include/llvm/Support/AArch64TargetParser.def

 AARCH64_CPU_NAME("cortex-a35", ARMV8A, FK_CRYPTO_NEON_FP_ARMV8, false,
                  (AArch64::AEK_CRC))
 AARCH64_CPU_NAME("cortex-a53", ARMV8A, FK_CRYPTO_NEON_FP_ARMV8, true,
                  (AArch64::AEK_CRC))
 AARCH64_CPU_NAME("cortex-a55", ARMV8_2A, FK_CRYPTO_NEON_FP_ARMV8, false,
                  (AArch64::AEK_FP16 | AArch64::AEK_DOTPROD | AArch64::AEK_RCPC))
 AARCH64_CPU_NAME("cortex-a57", ARMV8A, FK_CRYPTO_NEON_FP_ARMV8, false,
                  (AArch64::AEK_CRC))
 AARCH64_CPU_NAME("cortex-a65", ARMV8_2A, FK_CRYPTO_NEON_FP_ARMV8, false,
                  (AArch64::AEK_DOTPROD | AArch64::AEK_FP16 | AArch64::AEK_RAS |
                   AArch64::AEK_RCPC | AArch64::AEK_SSBS))
 AARCH64_CPU_NAME("cortex-a65ae", ARMV8_2A, FK_CRYPTO_NEON_FP_ARMV8, false,
                  (AArch64::AEK_DOTPROD | AArch64::AEK_FP16 | AArch64::AEK_RAS |
                   AArch64::AEK_RCPC | AArch64::AEK_SSBS))
 AARCH64_CPU_NAME("cortex-a72", ARMV8A, FK_CRYPTO_NEON_FP_ARMV8, false,
                  (AArch64::AEK_CRC))
 AARCH64_CPU_NAME("cortex-a73", ARMV8A, FK_CRYPTO_NEON_FP_ARMV8, false,
AARCH64_CPU_NAME("neoverse-n2", ARMV8_5A, FK_CRYPTO_NEON_FP_ARMV8, false,
                  (AArch64::AEK_BF16 | AArch64::AEK_DOTPROD | AArch64::AEK_FP16 |
                   AArch64::AEK_I8MM | AArch64::AEK_MTE | AArch64::AEK_RAS |
                   AArch64::AEK_RCPC | AArch64::AEK_SB | AArch64::AEK_SSBS |
                   AArch64::AEK_SVE | AArch64::AEK_SVE2 | AArch64::AEK_SVE2BITPERM))
 AARCH64_CPU_NAME("neoverse-v1", ARMV8_4A, FK_CRYPTO_NEON_FP_ARMV8, false,
                  (AArch64::AEK_RAS | AArch64::AEK_SVE | AArch64::AEK_SSBS |
                   AArch64::AEK_RCPC | AArch64::AEK_FP16 | AArch64::AEK_BF16 |
                   AArch64::AEK_DOTPROD ))
+AARCH64_CPU_NAME("cortex-a510", ARMV8_3A, FK_CRYPTO_NEON_FP_ARMV8, false,
+                 (AArch64::AEK_MTE | AArch64::AEK_SVE2 |
+                  AArch64::AEK_SVE2BITPERM | AArch64::AEK_DOTPROD |
+                  AArch64::AEK_RAS))
 AARCH64_CPU_NAME("cyclone", ARMV8A, FK_CRYPTO_NEON_FP_ARMV8, false,
                  (AArch64::AEK_NONE))
 AARCH64_CPU_NAME("apple-a7", ARMV8A, FK_CRYPTO_NEON_FP_ARMV8, false,
                  (AArch64::AEK_NONE))
 AARCH64_CPU_NAME("apple-a8", ARMV8A, FK_CRYPTO_NEON_FP_ARMV8, false,
                  (AArch64::AEK_NONE))
 AARCH64_CPU_NAME("apple-a9", ARMV8A, FK_CRYPTO_NEON_FP_ARMV8, false,
                  (AArch64::AEK_NONE))

llvm/lib/Target/AArch64/AArch64.td

def ProcNeoverseV1 : SubtargetFeature<"neoversev1", "ARMProcFamily",
                                       FeatureNEON,
                                       FeaturePerfMon,
                                       FeaturePostRAScheduler,
                                       FeatureRandGen,
                                       FeatureSPE,
                                       FeatureSSBS,
                                       FeatureSVE]>;
 
+def ProcCortexA510 : SubtargetFeature<"cortex-a510", "ARMProcFamily",
+                                      "CortexA510", "Cortex-A510 ARM processors", [
+                                      HasV8_3aOps,
+                                      FeatureCrypto,
+                                      FeatureFPARMv8,
+                                      FeatureFuseAES,
+                                      FeatureNEON,
+                                      FeaturePerfMon,
+                                      FeaturePostRAScheduler,
+                                      FeatureSPE,
+                                      FeatureAM,
+                                      FeatureMPAM,
+                                      FeatureETE,
+                                      FeatureMTE,
+                                      FeatureSVE2,
+                                      FeatureSVE2BitPerm,
+                                      FeatureFullFP16,
+                                      FeatureFP16FML,
+                                      FeatureDotProd]>;
+
 def ProcSaphira  : SubtargetFeature<"saphira", "ARMProcFamily", "Saphira",
                                    "Qualcomm Saphira processors", [
                                    FeatureCrypto,
                                    FeatureCustomCheapAsMoveHandling,
                                    FeatureFPARMv8,
                                    FeatureNEON,
                                    FeatureSPE,
                                    FeaturePerfMon,

 def : ProcessorModel<"cortex-a78", CortexA57Model, [ProcA78]>;
 def : ProcessorModel<"cortex-a78c", CortexA57Model, [ProcA78C]>;
 def : ProcessorModel<"cortex-r82", CortexA55Model, [ProcR82]>;
 def : ProcessorModel<"cortex-x1", CortexA57Model, [ProcX1]>;
 def : ProcessorModel<"neoverse-e1", CortexA53Model, [ProcNeoverseE1]>;
 def : ProcessorModel<"neoverse-n1", CortexA57Model, [ProcNeoverseN1]>;
 def : ProcessorModel<"neoverse-n2", CortexA57Model, [ProcNeoverseN2]>;
 def : ProcessorModel<"neoverse-v1", CortexA57Model, [ProcNeoverseV1]>;
+def : ProcessorModel<"cortex-a510", CortexA57Model, [ProcCortexA510]>;
 def : ProcessorModel<"exynos-m3", ExynosM3Model, [ProcExynosM3]>;
 def : ProcessorModel<"exynos-m4", ExynosM4Model, [ProcExynosM4]>;
 def : ProcessorModel<"exynos-m5", ExynosM5Model, [ProcExynosM4]>;
 def : ProcessorModel<"falkor", FalkorModel, [ProcFalkor]>;
 def : ProcessorModel<"saphira", FalkorModel, [ProcSaphira]>;
 def : ProcessorModel<"kryo", KryoModel, [ProcKryo]>;
 // Cavium ThunderX/ThunderX T8X  Processors
 def : ProcessorModel<"thunderx", ThunderXT8XModel,  [ProcThunderX]>;

llvm/lib/Target/AArch64/AArch64Subtarget.h

enum ARMProcFamilyEnum : uint8_t {
     CortexA65,
     CortexA72,
     CortexA73,
     CortexA75,
     CortexA76,
     CortexA77,
     CortexA78,
     CortexA78C,
+    CortexA510,
     CortexR82,
     CortexX1,
     ExynosM3,
     Falkor,
     Kryo,
     NeoverseE1,
     NeoverseN1,
     NeoverseN2,

llvm/lib/Target/AArch64/AArch64Subtarget.cpp

case ThunderX3T110:
     PrefLoopLogAlignment = 2;
     MaxInterleaveFactor = 4;
     PrefetchDistance = 128;
     MinPrefetchStride = 1024;
     MaxPrefetchIterationsAhead = 4;
     // FIXME: remove this to enable 64-bit SLP if performance looks good.
     MinVectorRegisterBitWidth = 128;
     break;
+  case CortexA510:
+    PrefFunctionLogAlignment = 4;
+    break;
   }
 }
 
 AArch64Subtarget::AArch64Subtarget(const Triple &TT, const std::string &CPU,
                                    const std::string &FS,
                                    const TargetMachine &TM, bool LittleEndian,
                                    unsigned MinSVEVectorSizeInBitsOverride,
                                    unsigned MaxSVEVectorSizeInBitsOverride)

llvm/unittests/Support/TargetParserTest.cpp

::testing::Values(
         ARMCPUTestParams("cortex-x1", "armv8.2-a", "crypto-neon-fp-armv8",
                          AArch64::AEK_CRC | AArch64::AEK_CRYPTO |
                              AArch64::AEK_FP | AArch64::AEK_RDM |
                              AArch64::AEK_SIMD | AArch64::AEK_RAS |
                              AArch64::AEK_LSE | AArch64::AEK_FP16 |
                              AArch64::AEK_DOTPROD | AArch64::AEK_RCPC |
                              AArch64::AEK_SSBS,
                          "8.2-A"),
+        ARMCPUTestParams("cortex-a510", "armv8.3-a", "crypto-neon-fp-armv8",
+                         AArch64::AEK_CRC | AArch64::AEK_CRYPTO |
+                         AArch64::AEK_FP | AArch64::AEK_SIMD |
+                         AArch64::AEK_RAS | AArch64::AEK_LSE |
+                         AArch64::AEK_RDM | AArch64::AEK_RCPC |
+                         AArch64::AEK_SVE2 | AArch64::AEK_SVE2BITPERM |
+                         AArch64::AEK_DOTPROD | AArch64::AEK_MTE,
+                         "8.3-A"),
         ARMCPUTestParams("cyclone", "armv8-a", "crypto-neon-fp-armv8",
                          AArch64::AEK_NONE | AArch64::AEK_CRYPTO |
                              AArch64::AEK_FP | AArch64::AEK_SIMD,
                          "8-A"),
         ARMCPUTestParams("apple-a7", "armv8-a", "crypto-neon-fp-armv8",
                          AArch64::AEK_NONE | AArch64::AEK_CRYPTO |
                              AArch64::AEK_FP | AArch64::AEK_SIMD,
                          "8-A"),
::testing::Values(
                          "8.2-A"),
         ARMCPUTestParams("carmel", "armv8.2-a", "crypto-neon-fp-armv8",
                          AArch64::AEK_CRC | AArch64::AEK_CRYPTO |
                              AArch64::AEK_FP | AArch64::AEK_SIMD |
                              AArch64::AEK_FP16 | AArch64::AEK_RAS |
                              AArch64::AEK_LSE | AArch64::AEK_RDM,
                          "8.2-A")));
 
-static constexpr unsigned NumAArch64CPUArchs = 48;
+static constexpr unsigned NumAArch64CPUArchs = 49;
 
 TEST(TargetParserTest, testAArch64CPUArchList) {
   SmallVector<StringRef, NumAArch64CPUArchs> List;
   AArch64::fillValidCPUArchList(List);
 
   // No list exists for these in this test suite, so ensure all are
   // valid, and match the expected 'magic' count.
   EXPECT_EQ(List.size(), NumAArch64CPUArchs);