[llvm] d705e7e - [NFC][TableGen] Code cleanup in CodeGenMapTable `EmitMapTable` (#126157)

via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 7 11:47:52 PST 2025


Author: Rahul Joshi
Date: 2025-02-07T11:47:49-08:00
New Revision: d705e7e9eb5e6d3c791935f191225118b88ab574

URL: https://github.com/llvm/llvm-project/commit/d705e7e9eb5e6d3c791935f191225118b88ab574
DIFF: https://github.com/llvm/llvm-project/commit/d705e7e9eb5e6d3c791935f191225118b88ab574.diff

LOG: [NFC][TableGen] Code cleanup in CodeGenMapTable `EmitMapTable` (#126157)

- Emit C++17 nested namespaces.
- Shorten the binary search table name to just `Table` since its
declared in the scope of each search function.
- Use `using namespace XXX` in the search function to avoid emitting the
Target Inst Namespace prefix in the table entries.
- Add short-cut handling of `TableSize` == 0 case (verified in Hexagon
target).
- Use `SetVector` in `ColFieldValueMap` to get automatic deduplication
and eliminate manual deduplication code.
- Use range for loops.

Added: 
    

Modified: 
    llvm/utils/TableGen/CodeGenMapTable.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/utils/TableGen/CodeGenMapTable.cpp b/llvm/utils/TableGen/CodeGenMapTable.cpp
index 8d22c0013dda88..2641e713c0c85a 100644
--- a/llvm/utils/TableGen/CodeGenMapTable.cpp
+++ b/llvm/utils/TableGen/CodeGenMapTable.cpp
@@ -78,6 +78,7 @@
 #include "Common/CodeGenInstruction.h"
 #include "Common/CodeGenTarget.h"
 #include "TableGenBackends.h"
+#include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/TableGen/Error.h"
 #include "llvm/TableGen/Record.h"
@@ -361,45 +362,38 @@ unsigned MapTableEmitter::emitBinSearchTable(raw_ostream &OS) {
   StringRef Namespace = Target.getInstNamespace();
   ArrayRef<const ListInit *> ValueCols = InstrMapDesc.getValueCols();
   unsigned NumCol = ValueCols.size();
-  unsigned TotalNumInstr = NumberedInstructions.size();
   unsigned TableSize = 0;
 
-  OS << "static const uint16_t " << InstrMapDesc.getName();
+  OS << "  using namespace " << Namespace << ";\n";
   // Number of columns in the table are NumCol+1 because key instructions are
   // emitted as first column.
-  OS << "Table[][" << NumCol + 1 << "] = {\n";
-  for (unsigned I = 0; I < TotalNumInstr; I++) {
-    const Record *CurInstr = NumberedInstructions[I]->TheDef;
+  for (const CodeGenInstruction *Inst : NumberedInstructions) {
+    const Record *CurInstr = Inst->TheDef;
     ArrayRef<const Record *> ColInstrs = MapTable[CurInstr];
+    if (ColInstrs.empty())
+      continue;
     std::string OutStr;
-    unsigned RelExists = 0;
-    if (!ColInstrs.empty()) {
-      for (unsigned J = 0; J < NumCol; J++) {
-        if (ColInstrs[J] != nullptr) {
-          RelExists = 1;
-          OutStr += ", ";
-          OutStr += Namespace;
-          OutStr += "::";
-          OutStr += ColInstrs[J]->getName();
-        } else {
-          OutStr += ", (uint16_t)-1U";
-        }
+    bool RelExists = false;
+    for (const Record *ColInstr : ColInstrs) {
+      if (ColInstr) {
+        RelExists = true;
+        OutStr += ", ";
+        OutStr += ColInstr->getName();
+      } else {
+        OutStr += ", (uint16_t)-1U";
       }
+    }
 
-      if (RelExists) {
-        OS << "  { " << Namespace << "::" << CurInstr->getName();
-        OS << OutStr << " },\n";
-        TableSize++;
-      }
+    if (RelExists) {
+      if (TableSize == 0)
+        OS << "  static constexpr uint16_t Table[][" << NumCol + 1 << "] = {\n";
+      OS << "    { " << CurInstr->getName() << OutStr << " },\n";
+      ++TableSize;
     }
   }
-  if (!TableSize) {
-    OS << "  { " << Namespace << "::"
-       << "INSTRUCTION_LIST_END, ";
-    OS << Namespace << "::"
-       << "INSTRUCTION_LIST_END }";
-  }
-  OS << "}; // End of " << InstrMapDesc.getName() << "Table\n\n";
+
+  if (TableSize != 0)
+    OS << "  }; // End of Table\n\n";
   return TableSize;
 }
 
@@ -409,15 +403,19 @@ unsigned MapTableEmitter::emitBinSearchTable(raw_ostream &OS) {
 //===----------------------------------------------------------------------===//
 
 void MapTableEmitter::emitBinSearch(raw_ostream &OS, unsigned TableSize) {
+  if (TableSize == 0) {
+    OS << "  return -1;\n";
+    return;
+  }
+
   OS << "  unsigned mid;\n";
   OS << "  unsigned start = 0;\n";
   OS << "  unsigned end = " << TableSize << ";\n";
   OS << "  while (start < end) {\n";
   OS << "    mid = start + (end - start) / 2;\n";
-  OS << "    if (Opcode == " << InstrMapDesc.getName() << "Table[mid][0]) {\n";
+  OS << "    if (Opcode == Table[mid][0]) \n";
   OS << "      break;\n";
-  OS << "    }\n";
-  OS << "    if (Opcode < " << InstrMapDesc.getName() << "Table[mid][0])\n";
+  OS << "    if (Opcode < Table[mid][0])\n";
   OS << "      end = mid;\n";
   OS << "    else\n";
   OS << "      start = mid + 1;\n";
@@ -431,7 +429,6 @@ void MapTableEmitter::emitBinSearch(raw_ostream &OS, unsigned TableSize) {
 //===----------------------------------------------------------------------===//
 
 void MapTableEmitter::emitMapFuncBody(raw_ostream &OS, unsigned TableSize) {
-
   const ListInit *ColFields = InstrMapDesc.getColFields();
   ArrayRef<const ListInit *> ValueCols = InstrMapDesc.getValueCols();
 
@@ -439,6 +436,8 @@ void MapTableEmitter::emitMapFuncBody(raw_ostream &OS, unsigned TableSize) {
   // relation table. If found, return opcode value from the appropriate column
   // of the table.
   emitBinSearch(OS, TableSize);
+  if (TableSize == 0)
+    return;
 
   if (ValueCols.size() > 1) {
     for (unsigned I = 0, E = ValueCols.size(); I < E; I++) {
@@ -453,14 +452,12 @@ void MapTableEmitter::emitMapFuncBody(raw_ostream &OS, unsigned TableSize) {
           OS << " && ";
       }
       OS << ")\n";
-      OS << "    return " << InstrMapDesc.getName();
-      OS << "Table[mid][" << I + 1 << "];\n";
+      OS << "    return Table[mid][" << I + 1 << "];\n";
     }
     OS << "  return -1;";
-  } else
-    OS << "  return " << InstrMapDesc.getName() << "Table[mid][1];\n";
-
-  OS << "}\n\n";
+  } else {
+    OS << "  return Table[mid][1];\n";
+  }
 }
 
 //===----------------------------------------------------------------------===//
@@ -468,7 +465,6 @@ void MapTableEmitter::emitMapFuncBody(raw_ostream &OS, unsigned TableSize) {
 //===----------------------------------------------------------------------===//
 
 void MapTableEmitter::emitTablesWithFunc(raw_ostream &OS) {
-
   // Emit function name and the input parameters : mostly opcode value of the
   // current instruction. However, if a table has multiple columns (more than 2
   // since first column is used for the key instructions), then we also need
@@ -491,6 +487,8 @@ void MapTableEmitter::emitTablesWithFunc(raw_ostream &OS) {
 
   // Emit rest of the function body.
   emitMapFuncBody(OS, TableSize);
+
+  OS << "}\n\n";
 }
 
 //===----------------------------------------------------------------------===//
@@ -498,7 +496,7 @@ void MapTableEmitter::emitTablesWithFunc(raw_ostream &OS) {
 //===----------------------------------------------------------------------===//
 
 static void emitEnums(raw_ostream &OS, const RecordKeeper &Records) {
-  std::map<std::string, std::vector<const Init *>> ColFieldValueMap;
+  std::map<std::string, SetVector<const Init *>> ColFieldValueMap;
 
   // Iterate over all InstrMapping records and create a map between column
   // fields and their possible values across all records.
@@ -507,10 +505,9 @@ static void emitEnums(raw_ostream &OS, const RecordKeeper &Records) {
     const ListInit *ColFields = CurMap->getValueAsListInit("ColFields");
     const ListInit *List = CurMap->getValueAsListInit("ValueCols");
     std::vector<const ListInit *> ValueCols;
-    unsigned ListSize = List->size();
 
-    for (unsigned J = 0; J < ListSize; J++) {
-      const auto *ListJ = cast<ListInit>(List->getElement(J));
+    for (const Init *Elem : *List) {
+      const auto *ListJ = cast<ListInit>(Elem);
 
       if (ListJ->size() != ColFields->size())
         PrintFatalError("Record `" + CurMap->getName() +
@@ -521,37 +518,26 @@ static void emitEnums(raw_ostream &OS, const RecordKeeper &Records) {
     }
 
     for (unsigned J = 0, EndCf = ColFields->size(); J < EndCf; J++) {
-      for (unsigned K = 0; K < ListSize; K++) {
-        std::string ColName = ColFields->getElement(J)->getAsUnquotedString();
-        ColFieldValueMap[ColName].push_back((ValueCols[K])->getElement(J));
-      }
+      std::string ColName = ColFields->getElement(J)->getAsUnquotedString();
+      auto &MapEntry = ColFieldValueMap[ColName];
+      for (const ListInit *List : ValueCols)
+        MapEntry.insert(List->getElement(J));
     }
   }
 
   for (auto &[EnumName, FieldValues] : ColFieldValueMap) {
-    // Delete duplicate entries from ColFieldValueMap
-    for (unsigned i = 0; i < FieldValues.size() - 1; i++) {
-      const Init *CurVal = FieldValues[i];
-      for (unsigned j = i + 1; j < FieldValues.size(); j++) {
-        if (CurVal == FieldValues[j]) {
-          FieldValues.erase(FieldValues.begin() + j);
-          --j;
-        }
-      }
-    }
-
     // Emit enumerated values for the column fields.
     OS << "enum " << EnumName << " {\n";
     ListSeparator LS(",\n");
     for (const Init *Field : FieldValues)
-      OS << LS << "\t" << EnumName << "_" << Field->getAsUnquotedString();
+      OS << LS << "  " << EnumName << "_" << Field->getAsUnquotedString();
     OS << "\n};\n\n";
   }
 }
 
 //===----------------------------------------------------------------------===//
 // Parse 'InstrMapping' records and use the information to form relationship
-// between instructions. These relations are emitted as a tables along with the
+// between instructions. These relations are emitted as tables along with the
 // functions to query them.
 //===----------------------------------------------------------------------===//
 void llvm::EmitMapTable(const RecordKeeper &Records, raw_ostream &OS) {
@@ -565,8 +551,7 @@ void llvm::EmitMapTable(const RecordKeeper &Records, raw_ostream &OS) {
 
   OS << "#ifdef GET_INSTRMAP_INFO\n";
   OS << "#undef GET_INSTRMAP_INFO\n";
-  OS << "namespace llvm {\n\n";
-  OS << "namespace " << NameSpace << " {\n\n";
+  OS << "namespace llvm::" << NameSpace << " {\n\n";
 
   // Emit coulumn field names and their values as enums.
   emitEnums(OS, Records);
@@ -589,7 +574,6 @@ void llvm::EmitMapTable(const RecordKeeper &Records, raw_ostream &OS) {
     // Emit map tables and the functions to query them.
     IMap.emitTablesWithFunc(OS);
   }
-  OS << "} // end namespace " << NameSpace << "\n";
-  OS << "} // end namespace llvm\n";
+  OS << "} // end namespace llvm::" << NameSpace << '\n';
   OS << "#endif // GET_INSTRMAP_INFO\n\n";
 }


        


More information about the llvm-commits mailing list