[NFC][TableGen] Code cleanup in CodeGenMapTable EmitMapTable
#126157
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- 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.
jurahul
force-pushed
the
codegen_map_table_cleanup
branch
from
0a9b065 to
7a3b588
Compare
|
@llvm/pr-subscribers-tablegen Author: Rahul Joshi (jurahul) Changes
Full diff: https://github.com/llvm/llvm-project/pull/126157.diff 1 Files Affected:
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";
}
|
jurahul
commented
Feb 7, 2025
Icohedron
pushed a commit
to Icohedron/llvm-project
that referenced
this pull request
Feb 11, 2025
…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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Tablesince its declared in the scope of each search function.using namespace XXXin the search function to avoid emitting the Target Inst Namespace prefix in the table entries.TableSize== 0 case (verified in Hexagon target).SetVectorinColFieldValueMapto get automatic deduplication and eliminate manual deduplication code.