From 10980e6092d46d689bf8b966ace04d426abbe66d Mon Sep 17 00:00:00 2001 From: Rahul Joshi Date: Wed, 9 Jul 2025 07:24:53 -0700 Subject: [PATCH 1/5] [NFC][TableGen] Remove small heap allocations in `SearchableTableEmitter` Change `GenericEnum` to not heap allocate its entries. Instead stash them directly in the `Entries` vector. Change `EntryMap` to hold an index as opposed to a pointer to the entry (the original reason why they were unique_ptr). --- .../utils/TableGen/SearchableTableEmitter.cpp | 71 +++++++++++-------- 1 file changed, 42 insertions(+), 29 deletions(-) diff --git a/llvm/utils/TableGen/SearchableTableEmitter.cpp b/llvm/utils/TableGen/SearchableTableEmitter.cpp index 38fc1ee5e4020..89015c66283a0 100644 --- a/llvm/utils/TableGen/SearchableTableEmitter.cpp +++ b/llvm/utils/TableGen/SearchableTableEmitter.cpp @@ -44,13 +44,27 @@ static int64_t getInt(const Record *R, StringRef Field) { namespace { struct GenericEnum { - using Entry = std::pair; + struct Entry { + StringRef Name; + int64_t Value; + const Record *Def; + Entry(StringRef N, int64_t V, const Record *D) + : Name(N), Value(V), Def(D) {} + }; std::string Name; const Record *Class = nullptr; std::string PreprocessorGuard; - std::vector> Entries; - DenseMap EntryMap; + std::vector Entries; + // Map from a Record to an index into the `Entries` vector. + DenseMap EntryMap; + + const Entry *getEntry(const Record *Def) const { + auto II = EntryMap.find(Def); + if (II == EntryMap.end()) + return nullptr; + return &Entries[II->second]; + } }; struct GenericField { @@ -129,11 +143,12 @@ class SearchableTableEmitter { else if (Field.IsInstruction) return I->getAsString(); else if (Field.Enum) { - auto *Entry = Field.Enum->EntryMap[cast(I)->getDef()]; + const GenericEnum::Entry *Entry = + Field.Enum->getEntry(cast(I)->getDef()); if (!Entry) PrintFatalError(Loc, Twine("Entry for field '") + Field.Name + "' is null"); - return Entry->first.str(); + return Entry->Name.str(); } PrintFatalError(Loc, Twine("invalid field type for field '") + Field.Name + "'; expected: bit, bits, string, or code"); @@ -221,7 +236,7 @@ int64_t SearchableTableEmitter::getNumericKey(const SearchIndex &Index, } if (Field.Enum) { const Record *EnumEntry = Rec->getValueAsDef(Field.Name); - return Field.Enum->EntryMap[EnumEntry]->second; + return Field.Enum->getEntry(EnumEntry)->Value; } assert(isa(Field.RecType) && "unexpected field type"); @@ -272,8 +287,8 @@ bool SearchableTableEmitter::compareBy(const Record *LHS, const Record *RHS, } else if (Field.Enum) { auto LHSr = cast(LHSI)->getDef(); auto RHSr = cast(RHSI)->getDef(); - int64_t LHSv = Field.Enum->EntryMap[LHSr]->second; - int64_t RHSv = Field.Enum->EntryMap[RHSr]->second; + int64_t LHSv = Field.Enum->getEntry(LHSr)->Value; + int64_t RHSv = Field.Enum->getEntry(RHSr)->Value; if (LHSv < RHSv) return true; if (LHSv > RHSv) @@ -308,8 +323,8 @@ void SearchableTableEmitter::emitGenericEnum(const GenericEnum &Enum, emitIfdef((Twine("GET_") + Enum.PreprocessorGuard + "_DECL").str(), OS); OS << "enum " << Enum.Name << " {\n"; - for (const auto &Entry : Enum.Entries) - OS << " " << Entry->first << " = " << Entry->second << ",\n"; + for (const auto &[Name, Value, _] : Enum.Entries) + OS << " " << Name << " = " << Value << ",\n"; OS << "};\n"; OS << "#endif\n\n"; @@ -625,31 +640,29 @@ std::unique_ptr SearchableTableEmitter::parseSearchIndex( void SearchableTableEmitter::collectEnumEntries( GenericEnum &Enum, StringRef NameField, StringRef ValueField, ArrayRef Items) { + Enum.Entries.reserve(Items.size()); for (const Record *EntryRec : Items) { - StringRef Name; - if (NameField.empty()) - Name = EntryRec->getName(); - else - Name = EntryRec->getValueAsString(NameField); - - int64_t Value = 0; - if (!ValueField.empty()) - Value = getInt(EntryRec, ValueField); - - Enum.Entries.push_back(std::make_unique(Name, Value)); - Enum.EntryMap.try_emplace(EntryRec, Enum.Entries.back().get()); + StringRef Name = NameField.empty() ? EntryRec->getName() + : EntryRec->getValueAsString(NameField); + int64_t Value = ValueField.empty() ? 0 : getInt(EntryRec, ValueField); + Enum.Entries.emplace_back(Name, Value, EntryRec); } + // If no values are provided for enums, assign values in the order of sorted + // enum names. if (ValueField.empty()) { - llvm::stable_sort(Enum.Entries, - [](const std::unique_ptr &LHS, - const std::unique_ptr &RHS) { - return LHS->first < RHS->first; - }); + llvm::stable_sort(Enum.Entries, [](const GenericEnum::Entry &LHS, + const GenericEnum::Entry &RHS) { + return LHS.Name < RHS.Name; + }); - for (size_t i = 0; i < Enum.Entries.size(); ++i) - Enum.Entries[i]->second = i; + for (auto [Idx, Entry] : enumerate(Enum.Entries)) + Entry.Value = Idx; } + + // Populate the entry map after the `Entries` vector is finalized. + for (auto [Idx, Entry] : enumerate(Enum.Entries)) + Enum.EntryMap.try_emplace(Entry.Def, Idx); } void SearchableTableEmitter::collectTableEntries( From f5720ef53d31aa3dfbe2beabb143b63ef3512b8a Mon Sep 17 00:00:00 2001 From: Rahul Joshi Date: Wed, 9 Jul 2025 16:47:21 -0700 Subject: [PATCH 2/5] Use MapVector --- .../utils/TableGen/SearchableTableEmitter.cpp | 44 +++++++++---------- 1 file changed, 21 insertions(+), 23 deletions(-) diff --git a/llvm/utils/TableGen/SearchableTableEmitter.cpp b/llvm/utils/TableGen/SearchableTableEmitter.cpp index 89015c66283a0..b06d38fe105f6 100644 --- a/llvm/utils/TableGen/SearchableTableEmitter.cpp +++ b/llvm/utils/TableGen/SearchableTableEmitter.cpp @@ -17,6 +17,7 @@ #include "Common/CodeGenTarget.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/MapVector.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/StringExtras.h" #include "llvm/TableGen/Error.h" @@ -47,23 +48,19 @@ struct GenericEnum { struct Entry { StringRef Name; int64_t Value; - const Record *Def; - Entry(StringRef N, int64_t V, const Record *D) - : Name(N), Value(V), Def(D) {} + Entry(StringRef N, int64_t V) : Name(N), Value(V) {} }; std::string Name; const Record *Class = nullptr; std::string PreprocessorGuard; - std::vector Entries; - // Map from a Record to an index into the `Entries` vector. - DenseMap EntryMap; + MapVector Entries; const Entry *getEntry(const Record *Def) const { - auto II = EntryMap.find(Def); - if (II == EntryMap.end()) + auto II = Entries.find(Def); + if (II == Entries.end()) return nullptr; - return &Entries[II->second]; + return &II->second; } }; @@ -323,8 +320,8 @@ void SearchableTableEmitter::emitGenericEnum(const GenericEnum &Enum, emitIfdef((Twine("GET_") + Enum.PreprocessorGuard + "_DECL").str(), OS); OS << "enum " << Enum.Name << " {\n"; - for (const auto &[Name, Value, _] : Enum.Entries) - OS << " " << Name << " = " << Value << ",\n"; + for (const auto &[_, Entry] : Enum.Entries.getArrayRef()) + OS << " " << Entry.Name << " = " << Entry.Value << ",\n"; OS << "};\n"; OS << "#endif\n\n"; @@ -645,24 +642,25 @@ void SearchableTableEmitter::collectEnumEntries( StringRef Name = NameField.empty() ? EntryRec->getName() : EntryRec->getValueAsString(NameField); int64_t Value = ValueField.empty() ? 0 : getInt(EntryRec, ValueField); - Enum.Entries.emplace_back(Name, Value, EntryRec); + Enum.Entries.try_emplace(EntryRec, Name, Value); } // If no values are provided for enums, assign values in the order of sorted // enum names. if (ValueField.empty()) { - llvm::stable_sort(Enum.Entries, [](const GenericEnum::Entry &LHS, - const GenericEnum::Entry &RHS) { - return LHS.Name < RHS.Name; - }); - - for (auto [Idx, Entry] : enumerate(Enum.Entries)) - Entry.Value = Idx; + // Copy the map entries for sorting and clear the map. + auto SavedEntries = Enum.Entries.takeVector(); + llvm::stable_sort( + SavedEntries, + [](const std::pair &LHS, + const std::pair &RHS) { + return LHS.second.Name < RHS.second.Name; + }); + + // Repopulate entries using the new sorted order. + for (auto [Idx, Entry] : enumerate(SavedEntries)) + Enum.Entries.try_emplace(Entry.first, Entry.second.Name, Idx); } - - // Populate the entry map after the `Entries` vector is finalized. - for (auto [Idx, Entry] : enumerate(Enum.Entries)) - Enum.EntryMap.try_emplace(Entry.Def, Idx); } void SearchableTableEmitter::collectTableEntries( From d3cab55431c00d2cf01e13c29a8073ebad44383d Mon Sep 17 00:00:00 2001 From: Rahul Joshi Date: Wed, 9 Jul 2025 17:00:12 -0700 Subject: [PATCH 3/5] Use make_second_range to skip over keys --- llvm/utils/TableGen/SearchableTableEmitter.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/llvm/utils/TableGen/SearchableTableEmitter.cpp b/llvm/utils/TableGen/SearchableTableEmitter.cpp index b06d38fe105f6..4dd651affa874 100644 --- a/llvm/utils/TableGen/SearchableTableEmitter.cpp +++ b/llvm/utils/TableGen/SearchableTableEmitter.cpp @@ -320,8 +320,9 @@ void SearchableTableEmitter::emitGenericEnum(const GenericEnum &Enum, emitIfdef((Twine("GET_") + Enum.PreprocessorGuard + "_DECL").str(), OS); OS << "enum " << Enum.Name << " {\n"; - for (const auto &[_, Entry] : Enum.Entries.getArrayRef()) - OS << " " << Entry.Name << " = " << Entry.Value << ",\n"; + for (const auto &[Name, Value] : + make_second_range(Enum.Entries.getArrayRef())) + OS << " " << Name << " = " << Value << ",\n"; OS << "};\n"; OS << "#endif\n\n"; From d29c1055f8a4b9a53aa8934a61d3c1a133530498 Mon Sep 17 00:00:00 2001 From: Rahul Joshi Date: Wed, 9 Jul 2025 19:10:26 -0700 Subject: [PATCH 4/5] Use auto for sort comparator --- llvm/utils/TableGen/SearchableTableEmitter.cpp | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/llvm/utils/TableGen/SearchableTableEmitter.cpp b/llvm/utils/TableGen/SearchableTableEmitter.cpp index 4dd651affa874..c1b175fb05aab 100644 --- a/llvm/utils/TableGen/SearchableTableEmitter.cpp +++ b/llvm/utils/TableGen/SearchableTableEmitter.cpp @@ -651,12 +651,9 @@ void SearchableTableEmitter::collectEnumEntries( if (ValueField.empty()) { // Copy the map entries for sorting and clear the map. auto SavedEntries = Enum.Entries.takeVector(); - llvm::stable_sort( - SavedEntries, - [](const std::pair &LHS, - const std::pair &RHS) { - return LHS.second.Name < RHS.second.Name; - }); + llvm::stable_sort(SavedEntries, [](const auto &LHS, const auto &RHS) { + return LHS.second.Name < RHS.second.Name; + }); // Repopulate entries using the new sorted order. for (auto [Idx, Entry] : enumerate(SavedEntries)) From 688a4ee99e23c4dd49cab11f551db9ab1d1f1152 Mon Sep 17 00:00:00 2001 From: Rahul Joshi Date: Thu, 10 Jul 2025 06:39:42 -0700 Subject: [PATCH 5/5] Revert use of auto in comparator --- llvm/utils/TableGen/SearchableTableEmitter.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/llvm/utils/TableGen/SearchableTableEmitter.cpp b/llvm/utils/TableGen/SearchableTableEmitter.cpp index c1b175fb05aab..7d57297eb7c0b 100644 --- a/llvm/utils/TableGen/SearchableTableEmitter.cpp +++ b/llvm/utils/TableGen/SearchableTableEmitter.cpp @@ -651,7 +651,9 @@ void SearchableTableEmitter::collectEnumEntries( if (ValueField.empty()) { // Copy the map entries for sorting and clear the map. auto SavedEntries = Enum.Entries.takeVector(); - llvm::stable_sort(SavedEntries, [](const auto &LHS, const auto &RHS) { + using MapVectorEntryTy = std::pair; + llvm::stable_sort(SavedEntries, [](const MapVectorEntryTy &LHS, + const MapVectorEntryTy &RHS) { return LHS.second.Name < RHS.second.Name; });