Skip to content

Commit d705e7e

Browse files
authored
[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.
1 parent 605a9e3 commit d705e7e

File tree

1 file changed

+49
-65
lines changed

1 file changed

+49
-65
lines changed

llvm/utils/TableGen/CodeGenMapTable.cpp

Lines changed: 49 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@
7878
#include "Common/CodeGenInstruction.h"
7979
#include "Common/CodeGenTarget.h"
8080
#include "TableGenBackends.h"
81+
#include "llvm/ADT/SetVector.h"
8182
#include "llvm/ADT/StringExtras.h"
8283
#include "llvm/TableGen/Error.h"
8384
#include "llvm/TableGen/Record.h"
@@ -361,45 +362,38 @@ unsigned MapTableEmitter::emitBinSearchTable(raw_ostream &OS) {
361362
StringRef Namespace = Target.getInstNamespace();
362363
ArrayRef<const ListInit *> ValueCols = InstrMapDesc.getValueCols();
363364
unsigned NumCol = ValueCols.size();
364-
unsigned TotalNumInstr = NumberedInstructions.size();
365365
unsigned TableSize = 0;
366366

367-
OS << "static const uint16_t " << InstrMapDesc.getName();
367+
OS << " using namespace " << Namespace << ";\n";
368368
// Number of columns in the table are NumCol+1 because key instructions are
369369
// emitted as first column.
370-
OS << "Table[][" << NumCol + 1 << "] = {\n";
371-
for (unsigned I = 0; I < TotalNumInstr; I++) {
372-
const Record *CurInstr = NumberedInstructions[I]->TheDef;
370+
for (const CodeGenInstruction *Inst : NumberedInstructions) {
371+
const Record *CurInstr = Inst->TheDef;
373372
ArrayRef<const Record *> ColInstrs = MapTable[CurInstr];
373+
if (ColInstrs.empty())
374+
continue;
374375
std::string OutStr;
375-
unsigned RelExists = 0;
376-
if (!ColInstrs.empty()) {
377-
for (unsigned J = 0; J < NumCol; J++) {
378-
if (ColInstrs[J] != nullptr) {
379-
RelExists = 1;
380-
OutStr += ", ";
381-
OutStr += Namespace;
382-
OutStr += "::";
383-
OutStr += ColInstrs[J]->getName();
384-
} else {
385-
OutStr += ", (uint16_t)-1U";
386-
}
376+
bool RelExists = false;
377+
for (const Record *ColInstr : ColInstrs) {
378+
if (ColInstr) {
379+
RelExists = true;
380+
OutStr += ", ";
381+
OutStr += ColInstr->getName();
382+
} else {
383+
OutStr += ", (uint16_t)-1U";
387384
}
385+
}
388386

389-
if (RelExists) {
390-
OS << " { " << Namespace << "::" << CurInstr->getName();
391-
OS << OutStr << " },\n";
392-
TableSize++;
393-
}
387+
if (RelExists) {
388+
if (TableSize == 0)
389+
OS << " static constexpr uint16_t Table[][" << NumCol + 1 << "] = {\n";
390+
OS << " { " << CurInstr->getName() << OutStr << " },\n";
391+
++TableSize;
394392
}
395393
}
396-
if (!TableSize) {
397-
OS << " { " << Namespace << "::"
398-
<< "INSTRUCTION_LIST_END, ";
399-
OS << Namespace << "::"
400-
<< "INSTRUCTION_LIST_END }";
401-
}
402-
OS << "}; // End of " << InstrMapDesc.getName() << "Table\n\n";
394+
395+
if (TableSize != 0)
396+
OS << " }; // End of Table\n\n";
403397
return TableSize;
404398
}
405399

@@ -409,15 +403,19 @@ unsigned MapTableEmitter::emitBinSearchTable(raw_ostream &OS) {
409403
//===----------------------------------------------------------------------===//
410404

411405
void MapTableEmitter::emitBinSearch(raw_ostream &OS, unsigned TableSize) {
406+
if (TableSize == 0) {
407+
OS << " return -1;\n";
408+
return;
409+
}
410+
412411
OS << " unsigned mid;\n";
413412
OS << " unsigned start = 0;\n";
414413
OS << " unsigned end = " << TableSize << ";\n";
415414
OS << " while (start < end) {\n";
416415
OS << " mid = start + (end - start) / 2;\n";
417-
OS << " if (Opcode == " << InstrMapDesc.getName() << "Table[mid][0]) {\n";
416+
OS << " if (Opcode == Table[mid][0]) \n";
418417
OS << " break;\n";
419-
OS << " }\n";
420-
OS << " if (Opcode < " << InstrMapDesc.getName() << "Table[mid][0])\n";
418+
OS << " if (Opcode < Table[mid][0])\n";
421419
OS << " end = mid;\n";
422420
OS << " else\n";
423421
OS << " start = mid + 1;\n";
@@ -431,14 +429,15 @@ void MapTableEmitter::emitBinSearch(raw_ostream &OS, unsigned TableSize) {
431429
//===----------------------------------------------------------------------===//
432430

433431
void MapTableEmitter::emitMapFuncBody(raw_ostream &OS, unsigned TableSize) {
434-
435432
const ListInit *ColFields = InstrMapDesc.getColFields();
436433
ArrayRef<const ListInit *> ValueCols = InstrMapDesc.getValueCols();
437434

438435
// Emit binary search algorithm to locate instructions in the
439436
// relation table. If found, return opcode value from the appropriate column
440437
// of the table.
441438
emitBinSearch(OS, TableSize);
439+
if (TableSize == 0)
440+
return;
442441

443442
if (ValueCols.size() > 1) {
444443
for (unsigned I = 0, E = ValueCols.size(); I < E; I++) {
@@ -453,22 +452,19 @@ void MapTableEmitter::emitMapFuncBody(raw_ostream &OS, unsigned TableSize) {
453452
OS << " && ";
454453
}
455454
OS << ")\n";
456-
OS << " return " << InstrMapDesc.getName();
457-
OS << "Table[mid][" << I + 1 << "];\n";
455+
OS << " return Table[mid][" << I + 1 << "];\n";
458456
}
459457
OS << " return -1;";
460-
} else
461-
OS << " return " << InstrMapDesc.getName() << "Table[mid][1];\n";
462-
463-
OS << "}\n\n";
458+
} else {
459+
OS << " return Table[mid][1];\n";
460+
}
464461
}
465462

466463
//===----------------------------------------------------------------------===//
467464
// Emit relation tables and the functions to query them.
468465
//===----------------------------------------------------------------------===//
469466

470467
void MapTableEmitter::emitTablesWithFunc(raw_ostream &OS) {
471-
472468
// Emit function name and the input parameters : mostly opcode value of the
473469
// current instruction. However, if a table has multiple columns (more than 2
474470
// since first column is used for the key instructions), then we also need
@@ -491,14 +487,16 @@ void MapTableEmitter::emitTablesWithFunc(raw_ostream &OS) {
491487

492488
// Emit rest of the function body.
493489
emitMapFuncBody(OS, TableSize);
490+
491+
OS << "}\n\n";
494492
}
495493

496494
//===----------------------------------------------------------------------===//
497495
// Emit enums for the column fields across all the instruction maps.
498496
//===----------------------------------------------------------------------===//
499497

500498
static void emitEnums(raw_ostream &OS, const RecordKeeper &Records) {
501-
std::map<std::string, std::vector<const Init *>> ColFieldValueMap;
499+
std::map<std::string, SetVector<const Init *>> ColFieldValueMap;
502500

503501
// Iterate over all InstrMapping records and create a map between column
504502
// fields and their possible values across all records.
@@ -507,10 +505,9 @@ static void emitEnums(raw_ostream &OS, const RecordKeeper &Records) {
507505
const ListInit *ColFields = CurMap->getValueAsListInit("ColFields");
508506
const ListInit *List = CurMap->getValueAsListInit("ValueCols");
509507
std::vector<const ListInit *> ValueCols;
510-
unsigned ListSize = List->size();
511508

512-
for (unsigned J = 0; J < ListSize; J++) {
513-
const auto *ListJ = cast<ListInit>(List->getElement(J));
509+
for (const Init *Elem : *List) {
510+
const auto *ListJ = cast<ListInit>(Elem);
514511

515512
if (ListJ->size() != ColFields->size())
516513
PrintFatalError("Record `" + CurMap->getName() +
@@ -521,37 +518,26 @@ static void emitEnums(raw_ostream &OS, const RecordKeeper &Records) {
521518
}
522519

523520
for (unsigned J = 0, EndCf = ColFields->size(); J < EndCf; J++) {
524-
for (unsigned K = 0; K < ListSize; K++) {
525-
std::string ColName = ColFields->getElement(J)->getAsUnquotedString();
526-
ColFieldValueMap[ColName].push_back((ValueCols[K])->getElement(J));
527-
}
521+
std::string ColName = ColFields->getElement(J)->getAsUnquotedString();
522+
auto &MapEntry = ColFieldValueMap[ColName];
523+
for (const ListInit *List : ValueCols)
524+
MapEntry.insert(List->getElement(J));
528525
}
529526
}
530527

531528
for (auto &[EnumName, FieldValues] : ColFieldValueMap) {
532-
// Delete duplicate entries from ColFieldValueMap
533-
for (unsigned i = 0; i < FieldValues.size() - 1; i++) {
534-
const Init *CurVal = FieldValues[i];
535-
for (unsigned j = i + 1; j < FieldValues.size(); j++) {
536-
if (CurVal == FieldValues[j]) {
537-
FieldValues.erase(FieldValues.begin() + j);
538-
--j;
539-
}
540-
}
541-
}
542-
543529
// Emit enumerated values for the column fields.
544530
OS << "enum " << EnumName << " {\n";
545531
ListSeparator LS(",\n");
546532
for (const Init *Field : FieldValues)
547-
OS << LS << "\t" << EnumName << "_" << Field->getAsUnquotedString();
533+
OS << LS << " " << EnumName << "_" << Field->getAsUnquotedString();
548534
OS << "\n};\n\n";
549535
}
550536
}
551537

552538
//===----------------------------------------------------------------------===//
553539
// Parse 'InstrMapping' records and use the information to form relationship
554-
// between instructions. These relations are emitted as a tables along with the
540+
// between instructions. These relations are emitted as tables along with the
555541
// functions to query them.
556542
//===----------------------------------------------------------------------===//
557543
void llvm::EmitMapTable(const RecordKeeper &Records, raw_ostream &OS) {
@@ -565,8 +551,7 @@ void llvm::EmitMapTable(const RecordKeeper &Records, raw_ostream &OS) {
565551

566552
OS << "#ifdef GET_INSTRMAP_INFO\n";
567553
OS << "#undef GET_INSTRMAP_INFO\n";
568-
OS << "namespace llvm {\n\n";
569-
OS << "namespace " << NameSpace << " {\n\n";
554+
OS << "namespace llvm::" << NameSpace << " {\n\n";
570555

571556
// Emit coulumn field names and their values as enums.
572557
emitEnums(OS, Records);
@@ -589,7 +574,6 @@ void llvm::EmitMapTable(const RecordKeeper &Records, raw_ostream &OS) {
589574
// Emit map tables and the functions to query them.
590575
IMap.emitTablesWithFunc(OS);
591576
}
592-
OS << "} // end namespace " << NameSpace << "\n";
593-
OS << "} // end namespace llvm\n";
577+
OS << "} // end namespace llvm::" << NameSpace << '\n';
594578
OS << "#endif // GET_INSTRMAP_INFO\n\n";
595579
}

0 commit comments

Comments
 (0)