Skip to content

Conversation

@jurahul
Copy link
Contributor

@jurahul jurahul commented Jul 9, 2025

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).

…ter`

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).
@jurahul jurahul force-pushed the searchable_table_emitter_cleanup branch from 6b65d8a to 10980e6 Compare July 9, 2025 22:46
@jurahul jurahul marked this pull request as ready for review July 10, 2025 01:05
@llvmbot
Copy link
Member

llvmbot commented Jul 10, 2025

@llvm/pr-subscribers-tablegen

Author: Rahul Joshi (jurahul)

Changes

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).


Full diff: https://github.com/llvm/llvm-project/pull/147845.diff

1 Files Affected:

  • (modified) llvm/utils/TableGen/SearchableTableEmitter.cpp (+42-30)
diff --git a/llvm/utils/TableGen/SearchableTableEmitter.cpp b/llvm/utils/TableGen/SearchableTableEmitter.cpp
index 38fc1ee5e4020..4dd651affa874 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"
@@ -44,13 +45,23 @@ static int64_t getInt(const Record *R, StringRef Field) {
 
 namespace {
 struct GenericEnum {
-  using Entry = std::pair<StringRef, int64_t>;
+  struct Entry {
+    StringRef Name;
+    int64_t Value;
+    Entry(StringRef N, int64_t V) : Name(N), Value(V) {}
+  };
 
   std::string Name;
   const Record *Class = nullptr;
   std::string PreprocessorGuard;
-  std::vector<std::unique_ptr<Entry>> Entries;
-  DenseMap<const Record *, Entry *> EntryMap;
+  MapVector<const Record *, Entry> Entries;
+
+  const Entry *getEntry(const Record *Def) const {
+    auto II = Entries.find(Def);
+    if (II == Entries.end())
+      return nullptr;
+    return &II->second;
+  }
 };
 
 struct GenericField {
@@ -129,11 +140,12 @@ class SearchableTableEmitter {
     else if (Field.IsInstruction)
       return I->getAsString();
     else if (Field.Enum) {
-      auto *Entry = Field.Enum->EntryMap[cast<DefInit>(I)->getDef()];
+      const GenericEnum::Entry *Entry =
+          Field.Enum->getEntry(cast<DefInit>(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 +233,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<BitsRecTy>(Field.RecType) && "unexpected field type");
 
@@ -272,8 +284,8 @@ bool SearchableTableEmitter::compareBy(const Record *LHS, const Record *RHS,
     } else if (Field.Enum) {
       auto LHSr = cast<DefInit>(LHSI)->getDef();
       auto RHSr = cast<DefInit>(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 +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)
-    OS << "  " << Entry->first << " = " << Entry->second << ",\n";
+  for (const auto &[Name, Value] :
+       make_second_range(Enum.Entries.getArrayRef()))
+    OS << "  " << Name << " = " << Value << ",\n";
   OS << "};\n";
 
   OS << "#endif\n\n";
@@ -625,30 +638,29 @@ std::unique_ptr<SearchIndex> SearchableTableEmitter::parseSearchIndex(
 void SearchableTableEmitter::collectEnumEntries(
     GenericEnum &Enum, StringRef NameField, StringRef ValueField,
     ArrayRef<const Record *> 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<GenericEnum::Entry>(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.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 std::unique_ptr<GenericEnum::Entry> &LHS,
-                         const std::unique_ptr<GenericEnum::Entry> &RHS) {
-                        return LHS->first < RHS->first;
-                      });
-
-    for (size_t i = 0; i < Enum.Entries.size(); ++i)
-      Enum.Entries[i]->second = i;
+    // Copy the map entries for sorting and clear the map.
+    auto SavedEntries = Enum.Entries.takeVector();
+    llvm::stable_sort(
+        SavedEntries,
+        [](const std::pair<const Record *, GenericEnum::Entry> &LHS,
+           const std::pair<const Record *, GenericEnum::Entry> &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);
   }
 }
 

@jurahul jurahul requested review from arsenm, mshockwave and topperc July 10, 2025 01:06
@jurahul jurahul requested a review from mshockwave July 10, 2025 02:11
@jurahul jurahul merged commit 0354867 into llvm:main Jul 10, 2025
9 checks passed
@jurahul jurahul deleted the searchable_table_emitter_cleanup branch July 10, 2025 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants