[llvm] [TableGen] Emit better error message for duplicate Subtarget features. (PR #102090)
Rahul Joshi via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 6 09:56:02 PDT 2024
https://github.com/jurahul updated https://github.com/llvm/llvm-project/pull/102090
>From 197a5f48dac2468c5d5a6154838caa415923b6e0 Mon Sep 17 00:00:00 2001
From: Rahul Joshi <rjoshi at nvidia.com>
Date: Mon, 5 Aug 2024 18:17:24 -0700
Subject: [PATCH 1/4] [NFC] Use more efficient data structures in
SubtargetEmitter
- Use StringSet and unorderd_map instead of set and map.
- Add a unit test to check if duplicate names are flagged.
---
llvm/test/TableGen/SubtargetFeatureUniqueNames.td | 10 ++++++++++
llvm/utils/TableGen/SubtargetEmitter.cpp | 8 ++++----
2 files changed, 14 insertions(+), 4 deletions(-)
create mode 100644 llvm/test/TableGen/SubtargetFeatureUniqueNames.td
diff --git a/llvm/test/TableGen/SubtargetFeatureUniqueNames.td b/llvm/test/TableGen/SubtargetFeatureUniqueNames.td
new file mode 100644
index 00000000000000..a482b50b539ed3
--- /dev/null
+++ b/llvm/test/TableGen/SubtargetFeatureUniqueNames.td
@@ -0,0 +1,10 @@
+// RUN: not llvm-tblgen -gen-subtarget -I %p/../../include %s 2>&1 | FileCheck %s
+// Verify that subtarget features with same names result in an error.
+
+include "llvm/Target/Target.td"
+
+def MyTarget : Target;
+
+// CHECK: error: Duplicate key in SubtargetFeatureKV: NameA
+def FeatureA : SubtargetFeature<"NameA", "", "", "">;
+def FeatureB : SubtargetFeature<"NameA", "", "", "">;
diff --git a/llvm/utils/TableGen/SubtargetEmitter.cpp b/llvm/utils/TableGen/SubtargetEmitter.cpp
index 1adefea5b03599..f02b08bafe2357 100644
--- a/llvm/utils/TableGen/SubtargetEmitter.cpp
+++ b/llvm/utils/TableGen/SubtargetEmitter.cpp
@@ -18,6 +18,7 @@
#include "llvm/ADT/SmallPtrSet.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSet.h"
#include "llvm/MC/MCInstrItineraries.h"
#include "llvm/MC/MCSchedule.h"
#include "llvm/Support/Debug.h"
@@ -31,9 +32,8 @@
#include <cassert>
#include <cstdint>
#include <iterator>
-#include <map>
-#include <set>
#include <string>
+#include <unordered_map>
#include <vector>
using namespace llvm;
@@ -260,7 +260,7 @@ unsigned SubtargetEmitter::FeatureKeyValues(
llvm::sort(FeatureList, LessRecordFieldName());
// Check that there are no duplicate keys
- std::set<StringRef> UniqueKeys;
+ llvm::StringSet<> UniqueKeys;
// Begin feature table
OS << "// Sorted (by key) array of values for CPU features.\n"
@@ -494,7 +494,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;
+ std::unordered_map<std::string, unsigned> ItinStageMap, ItinOperandMap;
for (const CodeGenProcModel &ProcModel : SchedModels.procModels()) {
// Add process itinerary to the list.
std::vector<InstrItinerary> &ItinList = ProcItinLists.emplace_back();
>From 8f18ac57f24c2e64ca92406140b5e336fd18f557 Mon Sep 17 00:00:00 2001
From: Rahul Joshi <rjoshi at nvidia.com>
Date: Mon, 5 Aug 2024 18:17:24 -0700
Subject: [PATCH 2/4] [TableGen] Emit better error message for duplicate
Subtarget features.
- Keep track of last definition of a feature in a DenseMap and use
it to report a better error message when a duplciate feature is
found.
- Use std::unorderd_map instead of a std::map in
`EmitStageAndOperandCycleData`
- Add a unit test to check if duplicate names are flagged.
---
llvm/test/TableGen/SubtargetFeatureUniqueNames.td | 6 ++++--
llvm/utils/TableGen/SubtargetEmitter.cpp | 14 +++++++++-----
2 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/llvm/test/TableGen/SubtargetFeatureUniqueNames.td b/llvm/test/TableGen/SubtargetFeatureUniqueNames.td
index a482b50b539ed3..87a21d29ec1b13 100644
--- a/llvm/test/TableGen/SubtargetFeatureUniqueNames.td
+++ b/llvm/test/TableGen/SubtargetFeatureUniqueNames.td
@@ -1,10 +1,12 @@
-// RUN: not llvm-tblgen -gen-subtarget -I %p/../../include %s 2>&1 | FileCheck %s
+// 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;
-// CHECK: error: Duplicate key in SubtargetFeatureKV: NameA
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 f02b08bafe2357..7a60e2c1ad7ded 100644
--- a/llvm/utils/TableGen/SubtargetEmitter.cpp
+++ b/llvm/utils/TableGen/SubtargetEmitter.cpp
@@ -14,6 +14,7 @@
#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"
@@ -259,8 +260,8 @@ unsigned SubtargetEmitter::FeatureKeyValues(
llvm::sort(FeatureList, LessRecordFieldName());
- // Check that there are no duplicate keys
- llvm::StringSet<> UniqueKeys;
+ // Check that there are no duplicate keys.
+ DenseMap<StringRef, const Record *> UniqueKeys;
// Begin feature table
OS << "// Sorted (by key) array of values for CPU features.\n"
@@ -291,9 +292,12 @@ unsigned SubtargetEmitter::FeatureKeyValues(
OS << " },\n";
++NumFeatures;
- if (!UniqueKeys.insert(CommandLineName).second)
- PrintFatalError("Duplicate key in SubtargetFeatureKV: " +
- CommandLineName);
+ auto [It, Inserted] = UniqueKeys.insert({CommandLineName, Feature});
+ if (!Inserted) {
+ PrintError(Feature, "Feature `" + CommandLineName + "` already defined.");
+ const Record *Previous = It->second;
+ PrintFatalNote(Previous, "Previous definition here.");
+ }
}
// End feature table
>From ce3e2f480ef9fce84b24cdaa35bf21c3bc964cda Mon Sep 17 00:00:00 2001
From: Rahul Joshi <rjoshi at nvidia.com>
Date: Mon, 5 Aug 2024 18:17:24 -0700
Subject: [PATCH 3/4] [TableGen] Emit better error message for duplicate
Subtarget features.
- Keep track of last definition of a feature in a DenseMap and use
it to report a better error message when a duplciate feature is
found.
- Use std::unorderd_map instead of a std::map in
`EmitStageAndOperandCycleData`
- Add a unit test to check if duplicate names are flagged.
---
llvm/utils/TableGen/SubtargetEmitter.cpp | 1 -
1 file changed, 1 deletion(-)
diff --git a/llvm/utils/TableGen/SubtargetEmitter.cpp b/llvm/utils/TableGen/SubtargetEmitter.cpp
index 7a60e2c1ad7ded..3d57ba77ada8ed 100644
--- a/llvm/utils/TableGen/SubtargetEmitter.cpp
+++ b/llvm/utils/TableGen/SubtargetEmitter.cpp
@@ -19,7 +19,6 @@
#include "llvm/ADT/SmallPtrSet.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringRef.h"
-#include "llvm/ADT/StringSet.h"
#include "llvm/MC/MCInstrItineraries.h"
#include "llvm/MC/MCSchedule.h"
#include "llvm/Support/Debug.h"
>From 944e68d73d15c3484f9cc0c39005b621d663c48b Mon Sep 17 00:00:00 2001
From: Rahul Joshi <rjoshi at nvidia.com>
Date: Mon, 5 Aug 2024 18:17:24 -0700
Subject: [PATCH 4/4] [TableGen] Emit better error message for duplicate
Subtarget features.
- Keep track of last definition of a feature in a DenseMap and use
it to report a better error message when a duplciate feature is
found.
- Use std::unorderd_map instead of a std::map in
`EmitStageAndOperandCycleData`
- Add a unit test to check if duplicate names are flagged.
---
llvm/utils/TableGen/SubtargetEmitter.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/llvm/utils/TableGen/SubtargetEmitter.cpp b/llvm/utils/TableGen/SubtargetEmitter.cpp
index 3d57ba77ada8ed..1bab6e7fdd4c96 100644
--- a/llvm/utils/TableGen/SubtargetEmitter.cpp
+++ b/llvm/utils/TableGen/SubtargetEmitter.cpp
@@ -259,8 +259,8 @@ unsigned SubtargetEmitter::FeatureKeyValues(
llvm::sort(FeatureList, LessRecordFieldName());
- // Check that there are no duplicate keys.
- DenseMap<StringRef, const Record *> 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,7 +291,7 @@ unsigned SubtargetEmitter::FeatureKeyValues(
OS << " },\n";
++NumFeatures;
- auto [It, Inserted] = UniqueKeys.insert({CommandLineName, Feature});
+ auto [It, Inserted] = UniqueFeatures.insert({CommandLineName, Feature});
if (!Inserted) {
PrintError(Feature, "Feature `" + CommandLineName + "` already defined.");
const Record *Previous = It->second;
More information about the llvm-commits
mailing list