[llvm] [TableGen] Print a warning when a Processor contains duplicate Features / TuneFeatures (PR #137864)

[Min-Yih Hsu via llvm-commits]

https://github.com/mshockwave updated https://github.com/llvm/llvm-project/pull/137864

>From a5144cc04a8a17e5b09cf3c3a845ae31cfd14fd1 Mon Sep 17 00:00:00 2001
From: Min-Yih Hsu <min.hsu at sifive.com>
Date: Tue, 29 Apr 2025 11:57:02 -0700
Subject: [PATCH 1/2] [TableGen] Print a warning when a Processor contains
 duplicate Features / TuneFeatures

---
 llvm/test/TableGen/DuplicateProcessorFeatures.td | 16 ++++++++++++++++
 llvm/utils/TableGen/SubtargetEmitter.cpp         | 12 ++++++++++++
 2 files changed, 28 insertions(+)
 create mode 100644 llvm/test/TableGen/DuplicateProcessorFeatures.td

diff --git a/llvm/test/TableGen/DuplicateProcessorFeatures.td b/llvm/test/TableGen/DuplicateProcessorFeatures.td
new file mode 100644
index 0000000000000..324a70f2a9d9e
--- /dev/null
+++ b/llvm/test/TableGen/DuplicateProcessorFeatures.td
@@ -0,0 +1,16 @@
+// RUN: llvm-tblgen -gen-subtarget -I %p/../../include %s 2>&1 | FileCheck %s
+
+include "llvm/Target/Target.td"
+
+def MyTarget : Target;
+
+def FeatureA : SubtargetFeature<"NameA", "", "", "">;
+def FeatureB : SubtargetFeature<"NameB", "", "", "">;
+def FeatureC : SubtargetFeature<"NameC", "", "", "">;
+
+// CHECK: warning: Processor CPU0 has duplicate Features: NameA
+def P0 : ProcessorModel<"CPU0", NoSchedModel, [FeatureA, FeatureB, FeatureA]>;
+// CHECK: warning: Processor CPU1 has duplicate TuneFeatures: NameB
+def P1 : ProcessorModel<"CPU1", NoSchedModel,
+                        /*Features=*/[FeatureA, FeatureB, FeatureC],
+                        /*TuneFeatures=*/[FeatureB, FeatureC, FeatureB]>;
diff --git a/llvm/utils/TableGen/SubtargetEmitter.cpp b/llvm/utils/TableGen/SubtargetEmitter.cpp
index 8d2fc99f84670..c608e85cddffa 100644
--- a/llvm/utils/TableGen/SubtargetEmitter.cpp
+++ b/llvm/utils/TableGen/SubtargetEmitter.cpp
@@ -333,6 +333,16 @@ unsigned SubtargetEmitter::cpuNames(raw_ostream &OS) {
   return Names.size();
 }
 
+static void checkDuplicateCPUFeatures(StringRef CPUName, StringRef FeatureKind,
+                                      ConstRecVec Features) {
+  SmallPtrSet<const Record *, 8> FeatureSet;
+  for (const auto *FeatureRec : Features) {
+    if (!FeatureSet.insert(FeatureRec).second)
+      PrintWarning("Processor " + CPUName + " has duplicate " + FeatureKind +
+                   ": " + FeatureRec->getValueAsString("Name"));
+  }
+}
+
 //
 // CPUKeyValues - Emit data of all the subtarget processors.  Used by command
 // line.
@@ -357,8 +367,10 @@ unsigned SubtargetEmitter::cpuKeyValues(raw_ostream &OS,
   for (const Record *Processor : ProcessorList) {
     StringRef Name = Processor->getValueAsString("Name");
     ConstRecVec FeatureList = Processor->getValueAsListOfDefs("Features");
+    checkDuplicateCPUFeatures(Name, "Features", FeatureList);
     ConstRecVec TuneFeatureList =
         Processor->getValueAsListOfDefs("TuneFeatures");
+    checkDuplicateCPUFeatures(Name, "TuneFeatures", TuneFeatureList);
 
     // Emit as "{ "cpu", "description", 0, { f1 , f2 , ... fn } },".
     OS << " { "

>From 2575a093a1e364db7196d3ce083c11216dcff0d5 Mon Sep 17 00:00:00 2001
From: Min-Yih Hsu <min.hsu at sifive.com>
Date: Tue, 29 Apr 2025 13:56:22 -0700
Subject: [PATCH 2/2] fixup! Address review comments

---
 .../TableGen/DuplicateProcessorFeatures.td    |  9 +++---
 llvm/utils/TableGen/SubtargetEmitter.cpp      | 30 ++++++++++++++-----
 2 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/llvm/test/TableGen/DuplicateProcessorFeatures.td b/llvm/test/TableGen/DuplicateProcessorFeatures.td
index 324a70f2a9d9e..45df2d814bcbd 100644
--- a/llvm/test/TableGen/DuplicateProcessorFeatures.td
+++ b/llvm/test/TableGen/DuplicateProcessorFeatures.td
@@ -8,9 +8,10 @@ def FeatureA : SubtargetFeature<"NameA", "", "", "">;
 def FeatureB : SubtargetFeature<"NameB", "", "", "">;
 def FeatureC : SubtargetFeature<"NameC", "", "", "">;
 
-// CHECK: warning: Processor CPU0 has duplicate Features: NameA
+// CHECK: warning: Processor CPU0 contains duplicate feature 'NameA'
 def P0 : ProcessorModel<"CPU0", NoSchedModel, [FeatureA, FeatureB, FeatureA]>;
-// CHECK: warning: Processor CPU1 has duplicate TuneFeatures: NameB
+// CHECK: warning: Processor CPU1 contains duplicate tune feature 'NameB'
+// CHECK: warning: Processor CPU1 has 'NameC' in both feature and tune feature sets
 def P1 : ProcessorModel<"CPU1", NoSchedModel,
-                        /*Features=*/[FeatureA, FeatureB, FeatureC],
-                        /*TuneFeatures=*/[FeatureB, FeatureC, FeatureB]>;
+                        /*Features=*/[FeatureC],
+                        /*TuneFeatures=*/[FeatureB, FeatureB, FeatureC]>;
diff --git a/llvm/utils/TableGen/SubtargetEmitter.cpp b/llvm/utils/TableGen/SubtargetEmitter.cpp
index c608e85cddffa..eb9ea860a9461 100644
--- a/llvm/utils/TableGen/SubtargetEmitter.cpp
+++ b/llvm/utils/TableGen/SubtargetEmitter.cpp
@@ -333,13 +333,27 @@ unsigned SubtargetEmitter::cpuNames(raw_ostream &OS) {
   return Names.size();
 }
 
-static void checkDuplicateCPUFeatures(StringRef CPUName, StringRef FeatureKind,
-                                      ConstRecVec Features) {
-  SmallPtrSet<const Record *, 8> FeatureSet;
+static void checkDuplicateCPUFeatures(StringRef CPUName,
+                                      ArrayRef<const Record *> Features,
+                                      ArrayRef<const Record *> TuneFeatures) {
+  // We had made sure each SubtargetFeature Record has a unique name, so we can
+  // simply use pointer sets here.
+  SmallPtrSet<const Record *, 8> FeatureSet, TuneFeatureSet;
   for (const auto *FeatureRec : Features) {
     if (!FeatureSet.insert(FeatureRec).second)
-      PrintWarning("Processor " + CPUName + " has duplicate " + FeatureKind +
-                   ": " + FeatureRec->getValueAsString("Name"));
+      PrintWarning("Processor " + CPUName + " contains duplicate feature '" +
+                   FeatureRec->getValueAsString("Name") + "'");
+  }
+
+  for (const auto *TuneFeatureRec : TuneFeatures) {
+    if (!TuneFeatureSet.insert(TuneFeatureRec).second)
+      PrintWarning("Processor " + CPUName +
+                   " contains duplicate tune feature '" +
+                   TuneFeatureRec->getValueAsString("Name") + "'");
+    if (FeatureSet.count(TuneFeatureRec))
+      PrintWarning("Processor " + CPUName + " has '" +
+                   TuneFeatureRec->getValueAsString("Name") +
+                   "' in both feature and tune feature sets");
   }
 }
 
@@ -367,10 +381,12 @@ unsigned SubtargetEmitter::cpuKeyValues(raw_ostream &OS,
   for (const Record *Processor : ProcessorList) {
     StringRef Name = Processor->getValueAsString("Name");
     ConstRecVec FeatureList = Processor->getValueAsListOfDefs("Features");
-    checkDuplicateCPUFeatures(Name, "Features", FeatureList);
     ConstRecVec TuneFeatureList =
         Processor->getValueAsListOfDefs("TuneFeatures");
-    checkDuplicateCPUFeatures(Name, "TuneFeatures", TuneFeatureList);
+
+    // Warn the user if there are duplicate processor features or tune
+    // features.
+    checkDuplicateCPUFeatures(Name, FeatureList, TuneFeatureList);
 
     // Emit as "{ "cpu", "description", 0, { f1 , f2 , ... fn } },".
     OS << " { "