[llvm] [TableGen] Rework error reporting for duplicate Feature/Processor. (PR #102257)
Rahul Joshi via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 6 20:29:08 PDT 2024
https://github.com/jurahul updated https://github.com/llvm/llvm-project/pull/102257
>From d40aa135b1ddb231e6d57f99b8f84f73dbee1d2f Mon Sep 17 00:00:00 2001
From: Rahul Joshi <rjoshi at nvidia.com>
Date: Tue, 6 Aug 2024 17:04:28 -0700
Subject: [PATCH] [TableGen] Rework error reporting for duplicate
Feature/Processor.
- Extract code for duplicate checks into a helper function and
update `collectProcModels` to use the helper.
- Update `FeatureKeyValues` to:
(a) Remove code for duplicate checks and use the helper.
(b) Trim features with empty name explicitly to be able to use
the helper.
---
llvm/test/TableGen/ProcessorUniqueNames.td | 12 ++++++
.../utils/TableGen/Common/CodeGenSchedule.cpp | 32 +++++++++++-----
llvm/utils/TableGen/Common/CodeGenSchedule.h | 5 +++
llvm/utils/TableGen/SubtargetEmitter.cpp | 38 +++++++------------
4 files changed, 54 insertions(+), 33 deletions(-)
create mode 100644 llvm/test/TableGen/ProcessorUniqueNames.td
diff --git a/llvm/test/TableGen/ProcessorUniqueNames.td b/llvm/test/TableGen/ProcessorUniqueNames.td
new file mode 100644
index 0000000000000..b706f96860ba5
--- /dev/null
+++ b/llvm/test/TableGen/ProcessorUniqueNames.td
@@ -0,0 +1,12 @@
+// RUN: not llvm-tblgen -gen-subtarget -I %p/../../include %s 2>&1 | FileCheck %s -DFILE=%s
+// Verify that processors with same names result in an error.
+
+include "llvm/Target/Target.td"
+
+def MyTarget : Target;
+
+def ProcessorA : ProcessorModel<"NameA", NoSchedModel, []>;
+
+// CHECK: [[FILE]]:[[@LINE+2]]:5: error: Processor `NameA` already defined.
+// CHECK: [[FILE]]:[[@LINE-3]]:5: note: Previous definition here.
+def ProcessorB : ProcessorModel<"NameA", NoSchedModel, []>;
diff --git a/llvm/utils/TableGen/Common/CodeGenSchedule.cpp b/llvm/utils/TableGen/Common/CodeGenSchedule.cpp
index 152e68797b991..8e2304e960671 100644
--- a/llvm/utils/TableGen/Common/CodeGenSchedule.cpp
+++ b/llvm/utils/TableGen/Common/CodeGenSchedule.cpp
@@ -535,15 +535,8 @@ void CodeGenSchedModels::collectProcModels() {
RecVec ProcRecords = Records.getAllDerivedDefinitions("Processor");
llvm::sort(ProcRecords, LessRecordFieldName());
- // Check for duplicated names.
- auto I = std::adjacent_find(ProcRecords.begin(), ProcRecords.end(),
- [](const Record *Rec1, const Record *Rec2) {
- return Rec1->getValueAsString("Name") ==
- Rec2->getValueAsString("Name");
- });
- if (I != ProcRecords.end())
- PrintFatalError((*I)->getLoc(), "Duplicate processor name " +
- (*I)->getValueAsString("Name"));
+ // Check duplicate Processor name.
+ checkDuplicateRecords(ProcRecords, "Processor");
// Reserve space because we can. Reallocation would be ok.
ProcModels.reserve(ProcRecords.size() + 1);
@@ -2290,3 +2283,24 @@ void PredTransitions::dump() const {
}
}
#endif // NDEBUG
+
+/// Verifies that there are no duplicate records with the same "Name" field.
+/// If there are, reports a fatal error message (and exits). Assumes that
+/// Records are sorted on the "Name" field.
+void llvm::checkDuplicateRecords(ArrayRef<Record *> Records,
+ StringRef ObjectName) {
+ auto I = std::adjacent_find(Records.begin(), Records.end(),
+ [](const Record *Rec1, const Record *Rec2) {
+ return Rec1->getValueAsString("Name") ==
+ Rec2->getValueAsString("Name");
+ });
+ if (I == Records.end())
+ return;
+
+ // Found a duplicate name.
+ const Record *First = *I;
+ const Record *Second = *(I + 1);
+ StringRef Name = First->getValueAsString("Name");
+ PrintError(Second, ObjectName + " `" + Name + "` already defined.");
+ PrintFatalNote(First, "Previous definition here.");
+}
diff --git a/llvm/utils/TableGen/Common/CodeGenSchedule.h b/llvm/utils/TableGen/Common/CodeGenSchedule.h
index 10ec7f41f56ff..a36cc990c0c38 100644
--- a/llvm/utils/TableGen/Common/CodeGenSchedule.h
+++ b/llvm/utils/TableGen/Common/CodeGenSchedule.h
@@ -649,6 +649,11 @@ class CodeGenSchedModels {
void addReadAdvance(Record *ProcReadAdvanceDef, unsigned PIdx);
};
+/// Verifies that there are no duplicate records with the same "Name" field.
+/// If there are, reports a fatal error message (and exits). Assumes that
+/// Records are sorted on the "Name" field.
+void checkDuplicateRecords(ArrayRef<Record *> Records, StringRef ObjectName);
+
} // namespace llvm
#endif
diff --git a/llvm/utils/TableGen/SubtargetEmitter.cpp b/llvm/utils/TableGen/SubtargetEmitter.cpp
index 163a9dc947060..fbe666a025e8c 100644
--- a/llvm/utils/TableGen/SubtargetEmitter.cpp
+++ b/llvm/utils/TableGen/SubtargetEmitter.cpp
@@ -250,34 +250,33 @@ void SubtargetEmitter::EmitSubtargetInfoMacroCalls(raw_ostream &OS) {
//
unsigned SubtargetEmitter::FeatureKeyValues(
raw_ostream &OS, const DenseMap<Record *, unsigned> &FeatureMap) {
- // Gather and sort all the features
+ // Gather and sort all the features.
std::vector<Record *> FeatureList =
Records.getAllDerivedDefinitions("SubtargetFeature");
+ // Remove features with empty name.
+ llvm::erase_if(FeatureList, [](const Record *Rec) {
+ return Rec->getValueAsString("Name").empty();
+ });
if (FeatureList.empty())
return 0;
- llvm::sort(FeatureList, LessRecordFieldName());
+ llvm::sort(Begin, End, LessRecordFieldName());
- // Check that there are no duplicate features.
- DenseMap<StringRef, const Record *> UniqueFeatures;
+ // Check duplicate Feature name.
+ checkDuplicateRecords(FeatureList, "Feature");
- // Begin feature table
+ // Begin feature table.
OS << "// Sorted (by key) array of values for CPU features.\n"
<< "extern const llvm::SubtargetFeatureKV " << Target
<< "FeatureKV[] = {\n";
- // For each feature
- unsigned NumFeatures = 0;
for (const Record *Feature : FeatureList) {
// Next feature
StringRef Name = Feature->getName();
StringRef CommandLineName = Feature->getValueAsString("Name");
StringRef Desc = Feature->getValueAsString("Desc");
- if (CommandLineName.empty())
- continue;
-
// Emit as { "feature", "description", { featureEnum }, { i1 , i2 , ... , in
// } }
OS << " { "
@@ -289,20 +288,12 @@ unsigned SubtargetEmitter::FeatureKeyValues(
printFeatureMask(OS, ImpliesList, FeatureMap);
OS << " },\n";
- ++NumFeatures;
-
- 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
+ // End feature table.
OS << "};\n";
- return NumFeatures;
+ return FeatureList.size();
}
//
@@ -317,18 +308,17 @@ SubtargetEmitter::CPUKeyValues(raw_ostream &OS,
Records.getAllDerivedDefinitions("Processor");
llvm::sort(ProcessorList, LessRecordFieldName());
- // Begin processor table
+ // Begin processor table.
OS << "// Sorted (by key) array of values for CPU subtype.\n"
<< "extern const llvm::SubtargetSubTypeKV " << Target
<< "SubTypeKV[] = {\n";
- // For each processor
for (Record *Processor : ProcessorList) {
StringRef Name = Processor->getValueAsString("Name");
RecVec FeatureList = Processor->getValueAsListOfDefs("Features");
RecVec TuneFeatureList = Processor->getValueAsListOfDefs("TuneFeatures");
- // Emit as { "cpu", "description", 0, { f1 , f2 , ... fn } },
+ // Emit as "{ "cpu", "description", 0, { f1 , f2 , ... fn } },".
OS << " { "
<< "\"" << Name << "\", ";
@@ -342,7 +332,7 @@ SubtargetEmitter::CPUKeyValues(raw_ostream &OS,
OS << ", &" << ProcModelName << " },\n";
}
- // End processor table
+ // End processor table.
OS << "};\n";
return ProcessorList.size();
More information about the llvm-commits
mailing list