[llvm] [NFC][TableGen] Minor code cleanup in SearchableTableEmitter (PR #147856)
Rahul Joshi via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 9 22:15:45 PDT 2025
https://github.com/jurahul updated https://github.com/llvm/llvm-project/pull/147856
>From 3f12ecdc27419089a69083a42b347b3d3c6faa03 Mon Sep 17 00:00:00 2001
From: Rahul Joshi <rjoshi at nvidia.com>
Date: Wed, 9 Jul 2025 16:21:10 -0700
Subject: [PATCH 1/2] [NFC][TableGen] Minor code cleanup in
SearchableTableEmitter
- Add braces around if/else bodies per LLVM coding standards.
- Use range for loops.
- use auto for variables initialized with `dyn_cast`.
---
.../utils/TableGen/SearchableTableEmitter.cpp | 48 +++++++++----------
1 file changed, 24 insertions(+), 24 deletions(-)
diff --git a/llvm/utils/TableGen/SearchableTableEmitter.cpp b/llvm/utils/TableGen/SearchableTableEmitter.cpp
index 38fc1ee5e4020..e47e683794029 100644
--- a/llvm/utils/TableGen/SearchableTableEmitter.cpp
+++ b/llvm/utils/TableGen/SearchableTableEmitter.cpp
@@ -31,9 +31,9 @@ using namespace llvm;
#define DEBUG_TYPE "searchable-table-emitter"
static int64_t getAsInt(const Init *B) {
- if (const BitsInit *BI = dyn_cast<BitsInit>(B))
+ if (const auto *BI = dyn_cast<BitsInit>(B))
return *BI->convertInitializerToInt();
- if (const IntInit *II = dyn_cast<IntInit>(B))
+ if (const auto *II = dyn_cast<IntInit>(B))
return II->getValue();
llvm_unreachable("Unexpected initializer");
}
@@ -115,20 +115,20 @@ class SearchableTableEmitter {
std::string primaryRepresentation(SMLoc Loc, const GenericField &Field,
const Init *I) {
- if (const StringInit *SI = dyn_cast<StringInit>(I)) {
+ if (const auto *SI = dyn_cast<StringInit>(I)) {
if (Field.IsCode || SI->hasCodeFormat())
return SI->getValue().str();
else
return SI->getAsString();
- } else if (const BitsInit *BI = dyn_cast<BitsInit>(I))
+ } else if (const auto *BI = dyn_cast<BitsInit>(I)) {
return "0x" + utohexstr(getAsInt(BI));
- else if (const BitInit *BI = dyn_cast<BitInit>(I))
+ } else if (const auto *BI = dyn_cast<BitInit>(I)) {
return BI->getValue() ? "true" : "false";
- else if (Field.IsIntrinsic)
+ } else if (Field.IsIntrinsic) {
return "Intrinsic::" + getIntrinsic(I).EnumName.str();
- else if (Field.IsInstruction)
+ } else if (Field.IsInstruction) {
return I->getAsString();
- else if (Field.Enum) {
+ } else if (Field.Enum) {
auto *Entry = Field.Enum->EntryMap[cast<DefInit>(I)->getDef()];
if (!Entry)
PrintFatalError(Loc,
@@ -140,7 +140,7 @@ class SearchableTableEmitter {
}
bool isIntrinsic(const Init *I) {
- if (const DefInit *DI = dyn_cast<DefInit>(I))
+ if (const auto *DI = dyn_cast<DefInit>(I))
return DI->getDef()->isSubClassOf("Intrinsic");
return false;
}
@@ -162,7 +162,7 @@ class SearchableTableEmitter {
if (Ctx == TypeInTempStruct)
return "std::string";
return "StringRef";
- } else if (const BitsRecTy *BI = dyn_cast<BitsRecTy>(Field.RecType)) {
+ } else if (const auto *BI = dyn_cast<BitsRecTy>(Field.RecType)) {
unsigned NumBits = BI->getNumBits();
if (NumBits <= 8)
return "uint8_t";
@@ -178,8 +178,9 @@ class SearchableTableEmitter {
"' of type bits is too large");
} else if (isa<BitRecTy>(Field.RecType)) {
return "bool";
- } else if (Field.Enum || Field.IsIntrinsic || Field.IsInstruction)
+ } else if (Field.Enum || Field.IsIntrinsic || Field.IsInstruction) {
return "unsigned";
+ }
PrintFatalError(Index.Loc,
Twine("In table '") + Table.Name + "' lookup method '" +
Index.Name + "', key field '" + Field.Name +
@@ -346,8 +347,8 @@ void SearchableTableEmitter::emitLookupFunction(const GenericTable &Table,
std::vector<std::pair<const Record *, unsigned>> Entries;
Entries.reserve(Table.Entries.size());
- for (unsigned i = 0; i < Table.Entries.size(); ++i)
- Entries.emplace_back(Table.Entries[i], i);
+ for (auto [Idx, TblEntry] : enumerate(Table.Entries))
+ Entries.emplace_back(TblEntry, Idx);
llvm::stable_sort(Entries,
[&](const std::pair<const Record *, unsigned> &LHS,
@@ -356,19 +357,19 @@ void SearchableTableEmitter::emitLookupFunction(const GenericTable &Table,
});
IndexRowsStorage.reserve(Entries.size());
- for (const auto &Entry : Entries) {
- IndexRowsStorage.push_back(Entry.first);
+ for (const auto &[EntryRec, EntryIndex] : Entries) {
+ IndexRowsStorage.push_back(EntryRec);
OS << " { ";
ListSeparator LS;
for (const auto &Field : Index.Fields) {
std::string Repr = primaryRepresentation(
- Index.Loc, Field, Entry.first->getValueInit(Field.Name));
+ Index.Loc, Field, EntryRec->getValueInit(Field.Name));
if (isa<StringRecTy>(Field.RecType))
Repr = StringRef(Repr).upper();
OS << LS << Repr;
}
- OS << ", " << Entry.second << " },\n";
+ OS << ", " << EntryIndex << " },\n";
}
OS << " };\n\n";
@@ -385,8 +386,8 @@ void SearchableTableEmitter::emitLookupFunction(const GenericTable &Table,
Index.Fields[0].IsInstruction)) {
int64_t FirstKeyVal = getNumericKey(Index, IndexRows[0]);
IsContiguous = true;
- for (unsigned i = 0; i < IndexRows.size(); ++i) {
- if (getNumericKey(Index, IndexRows[i]) != (FirstKeyVal + i)) {
+ for (const auto &[Idx, IndexRow] : enumerate(IndexRows)) {
+ if (getNumericKey(Index, IndexRow) != FirstKeyVal + (int64_t)Idx) {
IsContiguous = false;
break;
}
@@ -496,9 +497,9 @@ void SearchableTableEmitter::emitLookupFunction(const GenericTable &Table,
OS << " ||\n Key." << Field.Name << " != Idx->" << Field.Name;
}
- if (ShouldReturnRange)
+ if (ShouldReturnRange) {
OS << " return llvm::make_range(It.first, It.second);\n";
- else if (IsPrimary) {
+ } else if (IsPrimary) {
OS << ")\n return nullptr;\n\n";
OS << " return &*Idx;\n";
} else {
@@ -544,8 +545,7 @@ void SearchableTableEmitter::emitGenericTable(const GenericTable &Table,
// The primary data table contains all the fields defined for this map.
OS << "constexpr " << Table.CppTypeName << " " << Table.Name << "[] = {\n";
- for (unsigned i = 0; i < Table.Entries.size(); ++i) {
- const Record *Entry = Table.Entries[i];
+ for (const auto &[Idx, Entry] : enumerate(Table.Entries)) {
OS << " { ";
ListSeparator LS;
@@ -554,7 +554,7 @@ void SearchableTableEmitter::emitGenericTable(const GenericTable &Table,
<< primaryRepresentation(Table.Locs[0], Field,
Entry->getValueInit(Field.Name));
- OS << " }, // " << i << "\n";
+ OS << " }, // " << Idx << "\n";
}
OS << " };\n";
>From 27ed81d7953591fe123b851eae7c4958a92cfd98 Mon Sep 17 00:00:00 2001
From: Rahul Joshi <rjoshi at nvidia.com>
Date: Wed, 9 Jul 2025 22:14:28 -0700
Subject: [PATCH 2/2] Review feedback
---
.../utils/TableGen/SearchableTableEmitter.cpp | 117 ++++++++++--------
1 file changed, 63 insertions(+), 54 deletions(-)
diff --git a/llvm/utils/TableGen/SearchableTableEmitter.cpp b/llvm/utils/TableGen/SearchableTableEmitter.cpp
index e47e683794029..20ec5b4707e74 100644
--- a/llvm/utils/TableGen/SearchableTableEmitter.cpp
+++ b/llvm/utils/TableGen/SearchableTableEmitter.cpp
@@ -120,15 +120,16 @@ class SearchableTableEmitter {
return SI->getValue().str();
else
return SI->getAsString();
- } else if (const auto *BI = dyn_cast<BitsInit>(I)) {
+ }
+ if (const auto *BI = dyn_cast<BitsInit>(I))
return "0x" + utohexstr(getAsInt(BI));
- } else if (const auto *BI = dyn_cast<BitInit>(I)) {
+ if (const auto *BI = dyn_cast<BitInit>(I))
return BI->getValue() ? "true" : "false";
- } else if (Field.IsIntrinsic) {
+ if (Field.IsIntrinsic)
return "Intrinsic::" + getIntrinsic(I).EnumName.str();
- } else if (Field.IsInstruction) {
+ if (Field.IsInstruction)
return I->getAsString();
- } else if (Field.Enum) {
+ if (Field.Enum) {
auto *Entry = Field.Enum->EntryMap[cast<DefInit>(I)->getDef()];
if (!Entry)
PrintFatalError(Loc,
@@ -162,7 +163,8 @@ class SearchableTableEmitter {
if (Ctx == TypeInTempStruct)
return "std::string";
return "StringRef";
- } else if (const auto *BI = dyn_cast<BitsRecTy>(Field.RecType)) {
+ }
+ if (const auto *BI = dyn_cast<BitsRecTy>(Field.RecType)) {
unsigned NumBits = BI->getNumBits();
if (NumBits <= 8)
return "uint8_t";
@@ -176,11 +178,11 @@ class SearchableTableEmitter {
"' lookup method '" + Index.Name +
"', key field '" + Field.Name +
"' of type bits is too large");
- } else if (isa<BitRecTy>(Field.RecType)) {
+ }
+ if (isa<BitRecTy>(Field.RecType))
return "bool";
- } else if (Field.Enum || Field.IsIntrinsic || Field.IsInstruction) {
+ if (Field.Enum || Field.IsIntrinsic || Field.IsInstruction)
return "unsigned";
- }
PrintFatalError(Index.Loc,
Twine("In table '") + Table.Name + "' lookup method '" +
Index.Name + "', key field '" + Field.Name +
@@ -233,67 +235,74 @@ int64_t SearchableTableEmitter::getNumericKey(const SearchIndex &Index,
/// key of \p Index.
bool SearchableTableEmitter::compareBy(const Record *LHS, const Record *RHS,
const SearchIndex &Index) {
- for (const auto &Field : Index.Fields) {
- const Init *LHSI = LHS->getValueInit(Field.Name);
- const Init *RHSI = RHS->getValueInit(Field.Name);
+ // Compare two values and return:
+ // true if LHS < RHS
+ // false if LHS > RHS
+ // std::nullopt if LHS == RHS
+ auto CmpLTValue = [](const auto &LHS,
+ const auto &RHS) -> std::optional<bool> {
+ if (LHS < RHS)
+ return true;
+ if (LHS > RHS)
+ return false;
+ return std::nullopt;
+ };
+ // Compare two fields and returns:
+ // true if LHS < RHS
+ // false if LHS > RHS
+ // std::nullopt if LHS == RHS
+ auto CmpLTField = [this, &Index, CmpLTValue](
+ const Init *LHSI, const Init *RHSI,
+ const GenericField &Field) -> std::optional<bool> {
if (isa<BitsRecTy>(Field.RecType) || isa<IntRecTy>(Field.RecType)) {
int64_t LHSi = getAsInt(LHSI);
int64_t RHSi = getAsInt(RHSI);
- if (LHSi < RHSi)
- return true;
- if (LHSi > RHSi)
- return false;
- } else if (Field.IsIntrinsic) {
+ return CmpLTValue(LHSi, RHSi);
+ }
+
+ if (Field.IsIntrinsic) {
const CodeGenIntrinsic &LHSi = getIntrinsic(LHSI);
const CodeGenIntrinsic &RHSi = getIntrinsic(RHSI);
- if (std::tie(LHSi.TargetPrefix, LHSi.Name) <
- std::tie(RHSi.TargetPrefix, RHSi.Name))
- return true;
- if (std::tie(LHSi.TargetPrefix, LHSi.Name) >
- std::tie(RHSi.TargetPrefix, RHSi.Name))
- return false;
- } else if (Field.IsInstruction) {
+ return CmpLTValue(std::tie(LHSi.TargetPrefix, LHSi.Name),
+ std::tie(RHSi.TargetPrefix, RHSi.Name));
+ }
+
+ if (Field.IsInstruction) {
// This does not correctly compare the predefined instructions!
const Record *LHSr = cast<DefInit>(LHSI)->getDef();
const Record *RHSr = cast<DefInit>(RHSI)->getDef();
- bool LHSpseudo = LHSr->getValueAsBit("isPseudo");
- bool RHSpseudo = RHSr->getValueAsBit("isPseudo");
- if (LHSpseudo && !RHSpseudo)
- return true;
- if (!LHSpseudo && RHSpseudo)
- return false;
+ // Order pseudo instructions before non-pseudo ones.
+ bool LHSNotPseudo = !LHSr->getValueAsBit("isPseudo");
+ bool RHSNotPseudo = !RHSr->getValueAsBit("isPseudo");
+ return CmpLTValue(std::tuple(LHSNotPseudo, LHSr->getName()),
+ std::tuple(RHSNotPseudo, RHSr->getName()));
+ }
- int comp = LHSr->getName().compare(RHSr->getName());
- if (comp < 0)
- return true;
- if (comp > 0)
- return false;
- } else if (Field.Enum) {
- auto LHSr = cast<DefInit>(LHSI)->getDef();
- auto RHSr = cast<DefInit>(RHSI)->getDef();
+ if (Field.Enum) {
+ const Record *LHSr = cast<DefInit>(LHSI)->getDef();
+ const Record *RHSr = cast<DefInit>(RHSI)->getDef();
int64_t LHSv = Field.Enum->EntryMap[LHSr]->second;
int64_t RHSv = Field.Enum->EntryMap[RHSr]->second;
- if (LHSv < RHSv)
- return true;
- if (LHSv > RHSv)
- return false;
- } else {
- std::string LHSs = primaryRepresentation(Index.Loc, Field, LHSI);
- std::string RHSs = primaryRepresentation(Index.Loc, Field, RHSI);
+ return CmpLTValue(LHSv, RHSv);
+ }
- if (isa<StringRecTy>(Field.RecType)) {
- LHSs = StringRef(LHSs).upper();
- RHSs = StringRef(RHSs).upper();
- }
+ std::string LHSs = primaryRepresentation(Index.Loc, Field, LHSI);
+ std::string RHSs = primaryRepresentation(Index.Loc, Field, RHSI);
- int comp = LHSs.compare(RHSs);
- if (comp < 0)
- return true;
- if (comp > 0)
- return false;
+ if (isa<StringRecTy>(Field.RecType)) {
+ LHSs = StringRef(LHSs).upper();
+ RHSs = StringRef(RHSs).upper();
}
+ return CmpLTValue(LHSs, RHSs);
+ };
+
+ for (const GenericField &Field : Index.Fields) {
+ const Init *LHSI = LHS->getValueInit(Field.Name);
+ const Init *RHSI = RHS->getValueInit(Field.Name);
+ if (std::optional<bool> Cmp = CmpLTField(LHSI, RHSI, Field))
+ return *Cmp;
}
return false;
}
More information about the llvm-commits
mailing list