Skip to content

Commit 4c97c52

Browse files
authored
[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.
1 parent 28fa83f commit 4c97c52

File tree

2 files changed

+23
-8
lines changed

2 files changed

+23
-8
lines changed
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// RUN: not llvm-tblgen -gen-subtarget -I %p/../../include %s 2>&1 | FileCheck %s -DFILE=%s
2+
// Verify that subtarget features with same names result in an error.
3+
4+
include "llvm/Target/Target.td"
5+
6+
def MyTarget : Target;
7+
8+
def FeatureA : SubtargetFeature<"NameA", "", "", "">;
9+
10+
// CHECK: [[FILE]]:[[@LINE+2]]:5: error: Feature `NameA` already defined.
11+
// CHECK: [[FILE]]:[[@LINE-3]]:5: note: Previous definition here.
12+
def FeatureB : SubtargetFeature<"NameA", "", "", "">;

llvm/utils/TableGen/SubtargetEmitter.cpp

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,11 @@
1414
#include "Common/CodeGenSchedule.h"
1515
#include "Common/CodeGenTarget.h"
1616
#include "Common/PredicateExpander.h"
17+
#include "llvm/ADT/DenseMap.h"
1718
#include "llvm/ADT/STLExtras.h"
1819
#include "llvm/ADT/SmallPtrSet.h"
1920
#include "llvm/ADT/StringExtras.h"
21+
#include "llvm/ADT/StringMap.h"
2022
#include "llvm/ADT/StringRef.h"
2123
#include "llvm/MC/MCInstrItineraries.h"
2224
#include "llvm/MC/MCSchedule.h"
@@ -31,8 +33,6 @@
3133
#include <cassert>
3234
#include <cstdint>
3335
#include <iterator>
34-
#include <map>
35-
#include <set>
3636
#include <string>
3737
#include <vector>
3838

@@ -259,8 +259,8 @@ unsigned SubtargetEmitter::FeatureKeyValues(
259259

260260
llvm::sort(FeatureList, LessRecordFieldName());
261261

262-
// Check that there are no duplicate keys
263-
std::set<StringRef> UniqueKeys;
262+
// Check that there are no duplicate features.
263+
DenseMap<StringRef, const Record *> UniqueFeatures;
264264

265265
// Begin feature table
266266
OS << "// Sorted (by key) array of values for CPU features.\n"
@@ -291,9 +291,12 @@ unsigned SubtargetEmitter::FeatureKeyValues(
291291
OS << " },\n";
292292
++NumFeatures;
293293

294-
if (!UniqueKeys.insert(CommandLineName).second)
295-
PrintFatalError("Duplicate key in SubtargetFeatureKV: " +
296-
CommandLineName);
294+
auto [It, Inserted] = UniqueFeatures.insert({CommandLineName, Feature});
295+
if (!Inserted) {
296+
PrintError(Feature, "Feature `" + CommandLineName + "` already defined.");
297+
const Record *Previous = It->second;
298+
PrintFatalNote(Previous, "Previous definition here.");
299+
}
297300
}
298301

299302
// End feature table
@@ -494,7 +497,7 @@ void SubtargetEmitter::EmitStageAndOperandCycleData(
494497
// operand cycles, and pipeline bypass tables. Then add the new Itinerary
495498
// object with computed offsets to the ProcItinLists result.
496499
unsigned StageCount = 1, OperandCycleCount = 1;
497-
std::map<std::string, unsigned> ItinStageMap, ItinOperandMap;
500+
StringMap<unsigned> ItinStageMap, ItinOperandMap;
498501
for (const CodeGenProcModel &ProcModel : SchedModels.procModels()) {
499502
// Add process itinerary to the list.
500503
std::vector<InstrItinerary> &ItinList = ProcItinLists.emplace_back();

0 commit comments

Comments
 (0)