[llvm][AArch64][TableGen] Create a ProcessorAlias record #96249

jroelofs · 2024-06-20T23:31:06Z

... and use it to organize all of the AArch64 CPU aliases.

... and use it to organize all of the Apple CPU aliases.

llvmbot · 2024-06-20T23:31:36Z

@llvm/pr-subscribers-clang
@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-backend-aarch64

Author: Jon Roelofs (jroelofs)

Changes

... and use it to organize all of the Apple CPU aliases.

Full diff: https://github.com/llvm/llvm-project/pull/96249.diff

2 Files Affected:

(modified) llvm/lib/Target/AArch64/AArch64Processors.td (+16-19)
(modified) llvm/utils/TableGen/ARMTargetDefEmitter.cpp (+22-3)

diff --git a/llvm/lib/Target/AArch64/AArch64Processors.td b/llvm/lib/Target/AArch64/AArch64Processors.td
index 53b46ff42b72f..46f665cb15a9a 100644
--- a/llvm/lib/Target/AArch64/AArch64Processors.td
+++ b/llvm/lib/Target/AArch64/AArch64Processors.td
@@ -930,6 +930,12 @@ def ProcessorFeatures {
   list<SubtargetFeature> Generic = [FeatureFPARMv8, FeatureNEON, FeatureETE];
 }
 
+// Define an alternative name for a given Processor.
+class ProcessorAlias<string n, string alias> {
+  string Name = n;
+  string Alias = alias;
+}
+
 // FeatureFuseAdrpAdd is enabled under Generic to allow linker merging
 // optimizations.
 def : ProcessorModel<"generic", CortexA510Model, ProcessorFeatures.Generic,
@@ -1050,15 +1056,12 @@ def : ProcessorModel<"tsv110", TSV110Model, ProcessorFeatures.TSV110,
 
 // Apple CPUs
 
-// Support cyclone as an alias for apple-a7 so we can still LTO old bitcode.
-def : ProcessorModel<"cyclone", CycloneModel, ProcessorFeatures.AppleA7,
-                     [TuneAppleA7]>;
 def : ProcessorModel<"apple-a7", CycloneModel, ProcessorFeatures.AppleA7,
                      [TuneAppleA7]>;
-def : ProcessorModel<"apple-a8", CycloneModel, ProcessorFeatures.AppleA7,
-                     [TuneAppleA7]>;
-def : ProcessorModel<"apple-a9", CycloneModel, ProcessorFeatures.AppleA7,
-                     [TuneAppleA7]>;
+// Support cyclone as an alias for apple-a7 so we can still LTO old bitcode.
+def : ProcessorAlias<"cyclone", "apple-a7">;
+def : ProcessorAlias<"apple-a8", "apple-a7">;
+def : ProcessorAlias<"apple-a9", "apple-a7">;
 
 def : ProcessorModel<"apple-a10", CycloneModel, ProcessorFeatures.AppleA10,
                      [TuneAppleA10]>;
@@ -1068,28 +1071,23 @@ def : ProcessorModel<"apple-a11", CycloneModel, ProcessorFeatures.AppleA11,
 
 def : ProcessorModel<"apple-a12", CycloneModel, ProcessorFeatures.AppleA12,
                      [TuneAppleA12]>;
-def : ProcessorModel<"apple-s4", CycloneModel, ProcessorFeatures.AppleA12,
-                     [TuneAppleA12]>;
-def : ProcessorModel<"apple-s5", CycloneModel, ProcessorFeatures.AppleA12,
-                     [TuneAppleA12]>;
+def : ProcessorAlias<"apple-s4", "apple-a12">;
+def : ProcessorAlias<"apple-s5", "apple-a12">;
 
 def : ProcessorModel<"apple-a13", CycloneModel, ProcessorFeatures.AppleA13,
                      [TuneAppleA13]>;
 
 def : ProcessorModel<"apple-a14", CycloneModel, ProcessorFeatures.AppleA14,
                      [TuneAppleA14]>;
-def : ProcessorModel<"apple-m1", CycloneModel, ProcessorFeatures.AppleA14,
-                     [TuneAppleA14]>;
+def : ProcessorAlias<"apple-m1", "apple-a14">;
 
 def : ProcessorModel<"apple-a15", CycloneModel, ProcessorFeatures.AppleA15,
                      [TuneAppleA15]>;
-def : ProcessorModel<"apple-m2", CycloneModel, ProcessorFeatures.AppleA15,
-                     [TuneAppleA15]>;
+def : ProcessorAlias<"apple-m2", "apple-a15">;
 
 def : ProcessorModel<"apple-a16", CycloneModel, ProcessorFeatures.AppleA16,
                      [TuneAppleA16]>;
-def : ProcessorModel<"apple-m3", CycloneModel, ProcessorFeatures.AppleA16,
-                     [TuneAppleA16]>;
+def : ProcessorAlias<"apple-m3", "apple-a16">;
 
 def : ProcessorModel<"apple-a17", CycloneModel, ProcessorFeatures.AppleA17,
                      [TuneAppleA17]>;
@@ -1098,8 +1096,7 @@ def : ProcessorModel<"apple-m4", CycloneModel, ProcessorFeatures.AppleM4,
                      [TuneAppleM4]>;
 
 // Alias for the latest Apple processor model supported by LLVM.
-def : ProcessorModel<"apple-latest", CycloneModel, ProcessorFeatures.AppleM4,
-                     [TuneAppleM4]>;
+def : ProcessorAlias<"apple-latest", "apple-m4">;
 
 
 // Fujitsu A64FX
diff --git a/llvm/utils/TableGen/ARMTargetDefEmitter.cpp b/llvm/utils/TableGen/ARMTargetDefEmitter.cpp
index e22f353b451f9..3e111813280a3 100644
--- a/llvm/utils/TableGen/ARMTargetDefEmitter.cpp
+++ b/llvm/utils/TableGen/ARMTargetDefEmitter.cpp
@@ -221,8 +221,28 @@ static void EmitARMTargetDef(RecordKeeper &RK, raw_ostream &OS) {
   OS << "#ifdef EMIT_CPU_INFO\n"
      << "inline constexpr CpuInfo CpuInfos[] = {\n";
 
+  std::map<std::string, std::pair<std::string, const Record *>> ProcessorModels;
   for (const Record *Rec : RK.getAllDerivedDefinitions("ProcessorModel")) {
     auto Name = Rec->getValueAsString("Name");
+    ProcessorModels.insert(std::make_pair(Name, std::make_pair(Name, Rec)));
+  }
+
+  for (const Record *Rec : RK.getAllDerivedDefinitions("ProcessorAlias")) {
+    std::string Name = Rec->getValueAsString("Name").str();
+    auto Alias = Rec->getValueAsString("Alias");
+    auto It = ProcessorModels.find(Alias.str());
+    if (!ProcessorModels
+             .insert(
+                 std::make_pair(Name, std::make_pair(Alias, It->second.second)))
+             .second)
+      PrintFatalError(
+          Rec, "Alias duplicates an existing ProcessorAlias or ProcessorModel");
+  }
+
+  for (auto &[K, V] : ProcessorModels) {
+    auto Name = K;
+    auto Alias = V.first;
+    auto *Rec = V.second;
     auto Features = Rec->getValueAsListOfDefs("Features");
 
     // "apple-latest" is backend-only, should not be accepted by TargetParser.
@@ -253,9 +273,8 @@ static void EmitARMTargetDef(RecordKeeper &RK, raw_ostream &OS) {
     auto Profile = Arch->getValueAsString("Profile");
     auto ArchInfo = ArchInfoName(Major, Minor, Profile);
 
-    // The apple-latest alias is backend only, do not expose it to -mcpu.
-    if (Name == "apple-latest")
-      continue;
+    if (Name != Alias)
+      OS << "  // Alias: " << Name << " -> " << Alias << "\n";
 
     OS << "  {\n"
        << "    \"" << Name << "\",\n"

tmatheson-arm

There is an existing mechanism for processor aliases in TargetParser:

inline constexpr Alias CpuAliases[] = {{"cobalt-100", "neoverse-n2"},
                                       {"grace", "neoverse-v2"}};

Please could you either incorporate these CPUs into what you're doing here, or use the existing mechanism?

llvm/utils/TableGen/ARMTargetDefEmitter.cpp

davemgreen · 2024-06-21T06:08:31Z

We have at the moment:

inline constexpr Alias CpuAliases[] = {{"cobalt-100", "neoverse-n2"},
                                       {"grace", "neoverse-v2"}};

Can all the cpu aliases work in the same way? Or do you want them to work different, where they are not just frontend aliases?

This reverts commit ee1389a.

jroelofs · 2024-06-21T20:36:25Z

Can all the cpu aliases work in the same way? Or do you want them to work different, where they are not just frontend aliases?

I hadn't noticed that mechanism. There were some holes that needed to be addressed, but after fixing them, this does look like a better way to handle them.

tmatheson-arm

LGTM, I've added some thoughts but it's fine as it is.

tmatheson-arm · 2024-06-26T13:31:12Z

llvm/lib/TargetParser/AArch64TargetParser.cpp


  for (const auto &Alias : CpuAliases)
-    Values.push_back(Alias.AltName);
+    // The apple-latest alias is backend only, do not expose it to clang's -mcpu.
+    if (Alias.AltName != "apple-latest")


I don't love this special case. But, not sure what to do about it.

apple-latest is a bit of a special snowflake, meant for disassemblers and debuggers. I'm not sure what else to do with it either.

llvm/include/llvm/TargetParser/AArch64TargetParser.h

tmatheson-arm · 2024-06-26T16:57:59Z

llvm/include/llvm/TargetParser/AArch64TargetParser.h

+inline constexpr Alias CpuAliases[] = {
+    {"cobalt-100", "neoverse-n2"},
+    {"grace", "neoverse-v2"},
+    // Support cyclone as an alias for apple-a7 so we can still LTO old bitcode.


If you really want this to work only for bitcode (and not appear on -mcpu), could it be handled in the bitcode importer? Same for "apple-latest"?

I think we're stuck with it permanently, bitcode or not. I copied the comment over to keep some "provenance" of why it's still there, given we've moved entirely to apple-[am]\d+ ish spellings in open source.

tmatheson-arm · 2024-06-27T10:16:14Z

clang/test/Misc/target-invalid-cpu-note.c

@@ -5,11 +5,11 @@

 // RUN: not %clang_cc1 -triple arm64--- -target-cpu not-a-cpu -fsyntax-only %s 2>&1 | FileCheck %s --check-prefix AARCH64
 // AARCH64: error: unknown target CPU 'not-a-cpu'
-// AARCH64-NEXT: note: valid target CPU values are: generic, cortex-a35, cortex-a34, cortex-a53, cortex-a55, cortex-a510, cortex-a520, cortex-a520ae, cortex-a57, cortex-a65, cortex-a65ae, cortex-a72, cortex-a73, cortex-a75, cortex-a76, cortex-a76ae, cortex-a77, cortex-a78, cortex-a78ae, cortex-a78c, cortex-a710, cortex-a715, cortex-a720, cortex-a720ae, cortex-a725, cortex-r82, cortex-r82ae, cortex-x1, cortex-x1c, cortex-x2, cortex-x3, cortex-x4, cortex-x925, neoverse-e1, neoverse-n1, neoverse-n2, neoverse-n3, neoverse-512tvb, neoverse-v1, neoverse-v2, neoverse-v3, neoverse-v3ae, exynos-m3, exynos-m4, exynos-m5, falkor, saphira, kryo, thunderx, thunderxt88, thunderxt81, thunderxt83, thunderx2t99, thunderx3t110, tsv110, cyclone, apple-a7, apple-a8, apple-a9, apple-a10, apple-a11, apple-a12, apple-s4, apple-s5, apple-a13, apple-a14, apple-m1, apple-a15, apple-m2, apple-a16, apple-m3, apple-a17, apple-m4, a64fx, carmel, ampere1, ampere1a, ampere1b, oryon-1, cobalt-100, grace{{$}}
+// AARCH64-NEXT: note: valid target CPU values are: a64fx, ampere1, ampere1a, ampere1b, apple-a10, apple-a11, apple-a12, apple-a13, apple-a14, apple-a15, apple-a16, apple-a17, apple-a7, apple-a8, apple-a9, apple-m1, apple-m2, apple-m3, apple-m4, apple-s4, apple-s5, carmel, cobalt-100, cortex-a34, cortex-a35, cortex-a510, cortex-a520, cortex-a520ae, cortex-a53, cortex-a55, cortex-a57, cortex-a65, cortex-a65ae, cortex-a710, cortex-a715, cortex-a72, cortex-a720, cortex-a720ae, cortex-a725, cortex-a73, cortex-a75, cortex-a76, cortex-a76ae, cortex-a77, cortex-a78, cortex-a78ae, cortex-a78c, cortex-r82, cortex-r82ae, cortex-x1, cortex-x1c, cortex-x2, cortex-x3, cortex-x4, cortex-x925, cyclone, exynos-m3, exynos-m4, exynos-m5, falkor, generic, grace, kryo, neoverse-512tvb, neoverse-e1, neoverse-n1, neoverse-n2, neoverse-n3, neoverse-v1, neoverse-v2, neoverse-v3, neoverse-v3ae, oryon-1, saphira, thunderx, thunderx2t99, thunderx3t110, thunderxt81, thunderxt83, thunderxt88, tsv110{{$}}


Split into multiple lines?

Do you mean:

- Diags.Report(diag::note_valid_options) << llvm::join(ValidList, ", "); + Diags.Report(diag::note_valid_options) << llvm::join(ValidList, "\n");

or to add a few AARCH64-SAME:'s? The latter seems less preferable, since it leaves holes. For ex. writing:

// AARCH64-NEXT: note: valid target CPU values are: a64fx, ampere1 // AARCH64-SAME: ampere1b, apple-a10

could accidentally miss out on checking for ampere1a, whereas the current form forces complete coverage with the {{$}}.

I meant AARCH64-SAME, but you are right, that will potentially miss things. I'd be happy with changing the actual output format, but it doesn't have to be done in this PR.

... and use it to organize all of the AArch64 CPU aliases.

They were accidentally dropped in llvm#96249 rdar://140853882

…118581) They were accidentally dropped in #96249 rdar://140853882

…lvm#118581) They were accidentally dropped in llvm#96249 rdar://140853882

[llvm][AArch64][TableGen] Create a ProcessorAlias record. NFC

ee1389a

... and use it to organize all of the Apple CPU aliases.

jroelofs requested review from ahmedbougacha and tmatheson-arm June 20, 2024 23:31

llvmbot added the backend:AArch64 label Jun 20, 2024

tmatheson-arm reviewed Jun 21, 2024

View reviewed changes

llvm/utils/TableGen/ARMTargetDefEmitter.cpp Outdated Show resolved Hide resolved

jroelofs added 2 commits June 21, 2024 09:04

Revert "[llvm][AArch64][TableGen] Create a ProcessorAlias record. NFC"

5d2fce1

This reverts commit ee1389a.

use CpuAlias table instead

7adcf8e

llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Jun 21, 2024

jroelofs changed the title ~~[llvm][AArch64][TableGen] Create a ProcessorAlias record. NFC~~ [llvm][AArch64] Move Apple aliases into the CpuAlias map Jun 21, 2024

tmatheson-arm approved these changes Jun 27, 2024

View reviewed changes

define aliases through tablegen

a57a46f

jroelofs changed the title ~~[llvm][AArch64] Move Apple aliases into the CpuAlias map~~ [llvm][AArch64][TableGen] Create a ProcessorAlias record Jun 27, 2024

tmatheson-arm approved these changes Jun 28, 2024

View reviewed changes

grace is neoverse-v2, not neoverse-n2

7ea665d

jroelofs merged commit 6b4a13a into llvm:main Jun 28, 2024
4 of 5 checks passed

jroelofs deleted the jroelofs/cpu-alias branch June 28, 2024 19:45

lravenclaw pushed a commit to lravenclaw/llvm-project that referenced this pull request Jul 3, 2024

[llvm][AArch64][TableGen] Create a ProcessorAlias record (llvm#96249)

280f22a

... and use it to organize all of the AArch64 CPU aliases.

jroelofs added a commit to jroelofs/llvm-project that referenced this pull request Dec 4, 2024

Add processor aliases back to -print-supported-cpus and -mcpu=help

46775c3

They were accidentally dropped in llvm#96249 rdar://140853882

jroelofs mentioned this pull request Dec 4, 2024

Add processor aliases back to -print-supported-cpus and -mcpu=help #118581

Merged

jroelofs added a commit that referenced this pull request Dec 9, 2024

Add processor aliases back to -print-supported-cpus and -mcpu=help (#…

b6c22a4

…118581) They were accidentally dropped in #96249 rdar://140853882

broxigarchen pushed a commit to broxigarchen/llvm-project that referenced this pull request Dec 10, 2024

Add processor aliases back to -print-supported-cpus and -mcpu=help (l…

621b0ce

…lvm#118581) They were accidentally dropped in llvm#96249 rdar://140853882

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[llvm][AArch64][TableGen] Create a ProcessorAlias record #96249

[llvm][AArch64][TableGen] Create a ProcessorAlias record #96249

jroelofs commented Jun 20, 2024 •

edited

Loading

llvmbot commented Jun 20, 2024 •

edited

Loading

tmatheson-arm left a comment

davemgreen commented Jun 21, 2024

jroelofs commented Jun 21, 2024

tmatheson-arm left a comment

tmatheson-arm Jun 26, 2024

jroelofs Jun 27, 2024

tmatheson-arm Jun 26, 2024

jroelofs Jun 27, 2024

tmatheson-arm Jun 27, 2024

jroelofs Jun 27, 2024

tmatheson-arm Jun 28, 2024

[llvm][AArch64][TableGen] Create a ProcessorAlias record #96249

[llvm][AArch64][TableGen] Create a ProcessorAlias record #96249

Conversation

jroelofs commented Jun 20, 2024 • edited Loading

llvmbot commented Jun 20, 2024 • edited Loading

tmatheson-arm left a comment

Choose a reason for hiding this comment

davemgreen commented Jun 21, 2024

jroelofs commented Jun 21, 2024

tmatheson-arm left a comment

Choose a reason for hiding this comment

tmatheson-arm Jun 26, 2024

Choose a reason for hiding this comment

jroelofs Jun 27, 2024

Choose a reason for hiding this comment

tmatheson-arm Jun 26, 2024

Choose a reason for hiding this comment

jroelofs Jun 27, 2024

Choose a reason for hiding this comment

tmatheson-arm Jun 27, 2024

Choose a reason for hiding this comment

jroelofs Jun 27, 2024

Choose a reason for hiding this comment

tmatheson-arm Jun 28, 2024

Choose a reason for hiding this comment

jroelofs commented Jun 20, 2024 •

edited

Loading

llvmbot commented Jun 20, 2024 •

edited

Loading