[llvm] 4c97c52 - [TableGen] Emit better error message for duplicate Subtarget features. (#102090)

[via llvm-commits]

Author: Rahul Joshi
Date: 2024-08-06T23:39:04+02:00
New Revision: 4c97c52fe05bb2fda4584cea4738d59db31329dc

URL: https://github.com/llvm/llvm-project/commit/4c97c52fe05bb2fda4584cea4738d59db31329dc
DIFF: https://github.com/llvm/llvm-project/commit/4c97c52fe05bb2fda4584cea4738d59db31329dc.diff

LOG: [TableGen] Emit better error message for duplicate Subtarget features. (#102090)

- Keep track of last definition of a feature in a `DenseMap` and use 
  it to report a better error message when a duplicate feature is found.
- Use StringMap instead of a std::map in `EmitStageAndOperandCycleData`
- Add a unit test to check if duplicate names are flagged.

Added: 
    llvm/test/TableGen/SubtargetFeatureUniqueNames.td

Modified: 
    llvm/utils/TableGen/SubtargetEmitter.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/test/TableGen/SubtargetFeatureUniqueNames.td b/llvm/test/TableGen/SubtargetFeatureUniqueNames.td
new file mode 100644
index 0000000000000..87a21d29ec1b1
--- /dev/null
+++ b/llvm/test/TableGen/SubtargetFeatureUniqueNames.td
@@ -0,0 +1,12 @@
+// RUN: not llvm-tblgen -gen-subtarget -I %p/../../include %s 2>&1 | FileCheck %s -DFILE=%s
+// Verify that subtarget features with same names result in an error.
+
+include "llvm/Target/Target.td"
+
+def MyTarget : Target;
+
+def FeatureA : SubtargetFeature<"NameA", "", "", "">;
+
+// CHECK: [[FILE]]:[[@LINE+2]]:5: error: Feature `NameA` already defined.
+// CHECK: [[FILE]]:[[@LINE-3]]:5: note: Previous definition here.
+def FeatureB : SubtargetFeature<"NameA", "", "", "">;

diff  --git a/llvm/utils/TableGen/SubtargetEmitter.cpp b/llvm/utils/TableGen/SubtargetEmitter.cpp
index 1adefea5b0359..163a9dc947060 100644
--- a/llvm/utils/TableGen/SubtargetEmitter.cpp
+++ b/llvm/utils/TableGen/SubtargetEmitter.cpp
@@ -14,9 +14,11 @@
 #include "Common/CodeGenSchedule.h"
 #include "Common/CodeGenTarget.h"
 #include "Common/PredicateExpander.h"
+#include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/MC/MCInstrItineraries.h"
 #include "llvm/MC/MCSchedule.h"
@@ -31,8 +33,6 @@
 #include <cassert>
 #include <cstdint>
 #include <iterator>
-#include <map>
-#include <set>
 #include <string>
 #include <vector>
 
@@ -259,8 +259,8 @@ unsigned SubtargetEmitter::FeatureKeyValues(
 
   llvm::sort(FeatureList, LessRecordFieldName());
 
-  // Check that there are no duplicate keys
-  std::set<StringRef> UniqueKeys;
+  // Check that there are no duplicate features.
+  DenseMap<StringRef, const Record *> UniqueFeatures;
 
   // Begin feature table
   OS << "// Sorted (by key) array of values for CPU features.\n"
@@ -291,9 +291,12 @@ unsigned SubtargetEmitter::FeatureKeyValues(
     OS << " },\n";
     ++NumFeatures;
 
-    if (!UniqueKeys.insert(CommandLineName).second)
-      PrintFatalError("Duplicate key in SubtargetFeatureKV: " +
-                      CommandLineName);
+    auto [It, Inserted] = UniqueFeatures.insert({CommandLineName, Feature});
+    if (!Inserted) {
+      PrintError(Feature, "Feature `" + CommandLineName + "` already defined.");
+      const Record *Previous = It->second;
+      PrintFatalNote(Previous, "Previous definition here.");
+    }
   }
 
   // End feature table
@@ -494,7 +497,7 @@ void SubtargetEmitter::EmitStageAndOperandCycleData(
   // operand cycles, and pipeline bypass tables. Then add the new Itinerary
   // object with computed offsets to the ProcItinLists result.
   unsigned StageCount = 1, OperandCycleCount = 1;
-  std::map<std::string, unsigned> ItinStageMap, ItinOperandMap;
+  StringMap<unsigned> ItinStageMap, ItinOperandMap;
   for (const CodeGenProcModel &ProcModel : SchedModels.procModels()) {
     // Add process itinerary to the list.
     std::vector<InstrItinerary> &ItinList = ProcItinLists.emplace_back();