Skip to content

Commit b6a4621

Browse files
jurahuljurahul
authored
[NFC][TableGen] Minor code cleanup in SearchableTableEmitter (#147856)
- Add braces around if/else bodies per LLVM coding standards. - Use range for loops and structured bindings. - use auto for variables initialized with `dyn_cast`. - Refactor `compareBy` to also use early return in the comparison loop by extracting the comparison into lambdas.
1 parent 3076794 commit b6a4621

File tree

1 file changed

+87
-71
lines changed

1 file changed

+87
-71
lines changed

‎llvm/utils/TableGen/SearchableTableEmitter.cpp

Lines changed: 87 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,9 @@ using namespace llvm;
3232
#define DEBUG_TYPE "searchable-table-emitter"
3333

3434
static int64_t getAsInt(const Init *B) {
35-
if (const BitsInit *BI = dyn_cast<BitsInit>(B))
35+
if (const auto *BI = dyn_cast<BitsInit>(B))
3636
return *BI->convertInitializerToInt();
37-
if (const IntInit *II = dyn_cast<IntInit>(B))
37+
if (const auto *II = dyn_cast<IntInit>(B))
3838
return II->getValue();
3939
llvm_unreachable("Unexpected initializer");
4040
}
@@ -126,20 +126,21 @@ class SearchableTableEmitter {
126126

127127
std::string primaryRepresentation(SMLoc Loc, const GenericField &Field,
128128
const Init *I) {
129-
if (const StringInit *SI = dyn_cast<StringInit>(I)) {
129+
if (const auto *SI = dyn_cast<StringInit>(I)) {
130130
if (Field.IsCode || SI->hasCodeFormat())
131131
return SI->getValue().str();
132132
else
133133
return SI->getAsString();
134-
} else if (const BitsInit *BI = dyn_cast<BitsInit>(I))
134+
}
135+
if (const auto *BI = dyn_cast<BitsInit>(I))
135136
return "0x" + utohexstr(getAsInt(BI));
136-
else if (const BitInit *BI = dyn_cast<BitInit>(I))
137+
if (const auto *BI = dyn_cast<BitInit>(I))
137138
return BI->getValue() ? "true" : "false";
138-
else if (Field.IsIntrinsic)
139+
if (Field.IsIntrinsic)
139140
return "Intrinsic::" + getIntrinsic(I).EnumName.str();
140-
else if (Field.IsInstruction)
141+
if (Field.IsInstruction)
141142
return I->getAsString();
142-
else if (Field.Enum) {
143+
if (Field.Enum) {
143144
const GenericEnum::Entry *Entry =
144145
Field.Enum->getEntry(cast<DefInit>(I)->getDef());
145146
if (!Entry)
@@ -152,7 +153,7 @@ class SearchableTableEmitter {
152153
}
153154

154155
bool isIntrinsic(const Init *I) {
155-
if (const DefInit *DI = dyn_cast<DefInit>(I))
156+
if (const auto *DI = dyn_cast<DefInit>(I))
156157
return DI->getDef()->isSubClassOf("Intrinsic");
157158
return false;
158159
}
@@ -174,7 +175,8 @@ class SearchableTableEmitter {
174175
if (Ctx == TypeInTempStruct)
175176
return "std::string";
176177
return "StringRef";
177-
} else if (const BitsRecTy *BI = dyn_cast<BitsRecTy>(Field.RecType)) {
178+
}
179+
if (const auto *BI = dyn_cast<BitsRecTy>(Field.RecType)) {
178180
unsigned NumBits = BI->getNumBits();
179181
if (NumBits <= 8)
180182
return "uint8_t";
@@ -188,9 +190,10 @@ class SearchableTableEmitter {
188190
"' lookup method '" + Index.Name +
189191
"', key field '" + Field.Name +
190192
"' of type bits is too large");
191-
} else if (isa<BitRecTy>(Field.RecType)) {
193+
}
194+
if (isa<BitRecTy>(Field.RecType))
192195
return "bool";
193-
} else if (Field.Enum || Field.IsIntrinsic || Field.IsInstruction)
196+
if (Field.Enum || Field.IsIntrinsic || Field.IsInstruction)
194197
return "unsigned";
195198
PrintFatalError(Index.Loc,
196199
Twine("In table '") + Table.Name + "' lookup method '" +
@@ -244,67 +247,81 @@ int64_t SearchableTableEmitter::getNumericKey(const SearchIndex &Index,
244247
/// key of \p Index.
245248
bool SearchableTableEmitter::compareBy(const Record *LHS, const Record *RHS,
246249
const SearchIndex &Index) {
247-
for (const auto &Field : Index.Fields) {
248-
const Init *LHSI = LHS->getValueInit(Field.Name);
249-
const Init *RHSI = RHS->getValueInit(Field.Name);
250+
// Compare two values and return:
251+
// * -1 if LHS < RHS.
252+
// * 1 if LHS > RHS.
253+
// * 0 if LHS == RHS.
254+
auto CmpLTValue = [](const auto &LHS, const auto &RHS) -> int {
255+
if (LHS < RHS)
256+
return -1;
257+
if (LHS > RHS)
258+
return 1;
259+
return 0;
260+
};
261+
262+
// Specialized form of `CmpLTValue` for string-like types that uses compare()
263+
// to do the comparison of the 2 strings once (instead if 2 comparisons if we
264+
// use `CmpLTValue`).
265+
auto CmpLTString = [](StringRef LHS, StringRef RHS) -> int {
266+
return LHS.compare(RHS);
267+
};
250268

269+
// Compare two fields and returns:
270+
// - true if LHS < RHS.
271+
// - false if LHS > RHS.
272+
// - std::nullopt if LHS == RHS.
273+
auto CmpLTField = [this, &Index, &CmpLTValue,
274+
&CmpLTString](const Init *LHSI, const Init *RHSI,
275+
const GenericField &Field) -> int {
251276
if (isa<BitsRecTy>(Field.RecType) || isa<IntRecTy>(Field.RecType)) {
252277
int64_t LHSi = getAsInt(LHSI);
253278
int64_t RHSi = getAsInt(RHSI);
254-
if (LHSi < RHSi)
255-
return true;
256-
if (LHSi > RHSi)
257-
return false;
258-
} else if (Field.IsIntrinsic) {
279+
return CmpLTValue(LHSi, RHSi);
280+
}
281+
282+
if (Field.IsIntrinsic) {
259283
const CodeGenIntrinsic &LHSi = getIntrinsic(LHSI);
260284
const CodeGenIntrinsic &RHSi = getIntrinsic(RHSI);
261-
if (std::tie(LHSi.TargetPrefix, LHSi.Name) <
262-
std::tie(RHSi.TargetPrefix, RHSi.Name))
263-
return true;
264-
if (std::tie(LHSi.TargetPrefix, LHSi.Name) >
265-
std::tie(RHSi.TargetPrefix, RHSi.Name))
266-
return false;
267-
} else if (Field.IsInstruction) {
285+
if (int Cmp = CmpLTString(LHSi.TargetPrefix, RHSi.TargetPrefix))
286+
return Cmp;
287+
return CmpLTString(LHSi.Name, RHSi.Name);
288+
}
289+
290+
if (Field.IsInstruction) {
268291
// This does not correctly compare the predefined instructions!
269292
const Record *LHSr = cast<DefInit>(LHSI)->getDef();
270293
const Record *RHSr = cast<DefInit>(RHSI)->getDef();
271294

272-
bool LHSpseudo = LHSr->getValueAsBit("isPseudo");
273-
bool RHSpseudo = RHSr->getValueAsBit("isPseudo");
274-
if (LHSpseudo && !RHSpseudo)
275-
return true;
276-
if (!LHSpseudo && RHSpseudo)
277-
return false;
295+
// Order pseudo instructions before non-pseudo ones.
296+
bool LHSNotPseudo = !LHSr->getValueAsBit("isPseudo");
297+
bool RHSNotPseudo = !RHSr->getValueAsBit("isPseudo");
298+
if (int Cmp = CmpLTValue(LHSNotPseudo, RHSNotPseudo))
299+
return Cmp;
300+
return CmpLTString(LHSr->getName(), RHSr->getName());
301+
}
278302

279-
int comp = LHSr->getName().compare(RHSr->getName());
280-
if (comp < 0)
281-
return true;
282-
if (comp > 0)
283-
return false;
284-
} else if (Field.Enum) {
285-
auto LHSr = cast<DefInit>(LHSI)->getDef();
286-
auto RHSr = cast<DefInit>(RHSI)->getDef();
303+
if (Field.Enum) {
304+
const Record *LHSr = cast<DefInit>(LHSI)->getDef();
305+
const Record *RHSr = cast<DefInit>(RHSI)->getDef();
287306
int64_t LHSv = Field.Enum->getEntry(LHSr)->Value;
288307
int64_t RHSv = Field.Enum->getEntry(RHSr)->Value;
289-
if (LHSv < RHSv)
290-
return true;
291-
if (LHSv > RHSv)
292-
return false;
293-
} else {
294-
std::string LHSs = primaryRepresentation(Index.Loc, Field, LHSI);
295-
std::string RHSs = primaryRepresentation(Index.Loc, Field, RHSI);
296-
297-
if (isa<StringRecTy>(Field.RecType)) {
298-
LHSs = StringRef(LHSs).upper();
299-
RHSs = StringRef(RHSs).upper();
300-
}
308+
return CmpLTValue(LHSv, RHSv);
309+
}
301310

302-
int comp = LHSs.compare(RHSs);
303-
if (comp < 0)
304-
return true;
305-
if (comp > 0)
306-
return false;
311+
std::string LHSs = primaryRepresentation(Index.Loc, Field, LHSI);
312+
std::string RHSs = primaryRepresentation(Index.Loc, Field, RHSI);
313+
if (isa<StringRecTy>(Field.RecType)) {
314+
LHSs = StringRef(LHSs).upper();
315+
RHSs = StringRef(RHSs).upper();
307316
}
317+
return CmpLTString(LHSs, RHSs);
318+
};
319+
320+
for (const GenericField &Field : Index.Fields) {
321+
const Init *LHSI = LHS->getValueInit(Field.Name);
322+
const Init *RHSI = RHS->getValueInit(Field.Name);
323+
if (int Cmp = CmpLTField(LHSI, RHSI, Field))
324+
return Cmp < 0;
308325
}
309326
return false;
310327
}
@@ -359,8 +376,8 @@ void SearchableTableEmitter::emitLookupFunction(const GenericTable &Table,
359376

360377
std::vector<std::pair<const Record *, unsigned>> Entries;
361378
Entries.reserve(Table.Entries.size());
362-
for (unsigned i = 0; i < Table.Entries.size(); ++i)
363-
Entries.emplace_back(Table.Entries[i], i);
379+
for (auto [Idx, TblEntry] : enumerate(Table.Entries))
380+
Entries.emplace_back(TblEntry, Idx);
364381

365382
llvm::stable_sort(Entries,
366383
[&](const std::pair<const Record *, unsigned> &LHS,
@@ -369,19 +386,19 @@ void SearchableTableEmitter::emitLookupFunction(const GenericTable &Table,
369386
});
370387

371388
IndexRowsStorage.reserve(Entries.size());
372-
for (const auto &Entry : Entries) {
373-
IndexRowsStorage.push_back(Entry.first);
389+
for (const auto &[EntryRec, EntryIndex] : Entries) {
390+
IndexRowsStorage.push_back(EntryRec);
374391

375392
OS << " { ";
376393
ListSeparator LS;
377394
for (const auto &Field : Index.Fields) {
378395
std::string Repr = primaryRepresentation(
379-
Index.Loc, Field, Entry.first->getValueInit(Field.Name));
396+
Index.Loc, Field, EntryRec->getValueInit(Field.Name));
380397
if (isa<StringRecTy>(Field.RecType))
381398
Repr = StringRef(Repr).upper();
382399
OS << LS << Repr;
383400
}
384-
OS << ", " << Entry.second << " },\n";
401+
OS << ", " << EntryIndex << " },\n";
385402
}
386403

387404
OS << " };\n\n";
@@ -398,8 +415,8 @@ void SearchableTableEmitter::emitLookupFunction(const GenericTable &Table,
398415
Index.Fields[0].IsInstruction)) {
399416
int64_t FirstKeyVal = getNumericKey(Index, IndexRows[0]);
400417
IsContiguous = true;
401-
for (unsigned i = 0; i < IndexRows.size(); ++i) {
402-
if (getNumericKey(Index, IndexRows[i]) != (FirstKeyVal + i)) {
418+
for (const auto &[Idx, IndexRow] : enumerate(IndexRows)) {
419+
if (getNumericKey(Index, IndexRow) != FirstKeyVal + (int64_t)Idx) {
403420
IsContiguous = false;
404421
break;
405422
}
@@ -509,9 +526,9 @@ void SearchableTableEmitter::emitLookupFunction(const GenericTable &Table,
509526
OS << " ||\n Key." << Field.Name << " != Idx->" << Field.Name;
510527
}
511528

512-
if (ShouldReturnRange)
529+
if (ShouldReturnRange) {
513530
OS << " return llvm::make_range(It.first, It.second);\n";
514-
else if (IsPrimary) {
531+
} else if (IsPrimary) {
515532
OS << ")\n return nullptr;\n\n";
516533
OS << " return &*Idx;\n";
517534
} else {
@@ -557,8 +574,7 @@ void SearchableTableEmitter::emitGenericTable(const GenericTable &Table,
557574

558575
// The primary data table contains all the fields defined for this map.
559576
OS << "constexpr " << Table.CppTypeName << " " << Table.Name << "[] = {\n";
560-
for (unsigned i = 0; i < Table.Entries.size(); ++i) {
561-
const Record *Entry = Table.Entries[i];
577+
for (const auto &[Idx, Entry] : enumerate(Table.Entries)) {
562578
OS << " { ";
563579

564580
ListSeparator LS;
@@ -567,7 +583,7 @@ void SearchableTableEmitter::emitGenericTable(const GenericTable &Table,
567583
<< primaryRepresentation(Table.Locs[0], Field,
568584
Entry->getValueInit(Field.Name));
569585

570-
OS << " }, // " << i << "\n";
586+
OS << " }, // " << Idx << "\n";
571587
}
572588
OS << " };\n";
573589

0 commit comments

Comments
 (0)