[TableGen] Emit instruction name in INSTRINFO_OPERAND_TYPE

[Amir]

Summary

Make Offsets and OpcodeOperandTypes tables human-readable by printing the
instruction name before the operand list.

In effect, this makes debugging generated getOperandType possible.

[Amir]

Rebase

[Amir]

clang-format

[craig.topper]

llvm/utils/TableGen/InstrInfoEmitter.cpp

392

This is only storing the name of the last instruction that uses the offset. An instruction with no operands won't increment CurrentOffset.

417

I think I here is the instruction number. Can't we just look it up via NumberedInstructions[I]?

432

I think CurOffset here is the instrution number. Can we look up NumberedInstructions[CurOffset]?

435

Should we print the names of instructions with no operands here?

[Amir]

Address Craig's comments

[Amir]

llvm/utils/TableGen/InstrInfoEmitter.cpp

392

You're right, thank you so much! I was initially keeping something else in this map, but in a later iterations this reduced only to the opcode which can be referenced from NumberedInstructions

[craig.topper]

LGTM

[paulkirth]

Hi, we're seeing a failure in Fuchsia's Clang CI for windows.

The failing test is LLVM :: TableGen/get-operand-type.td

You can find the failing bot here: https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-windows-x64/b8810810347109726705/overview

If this will be hard to address, would you mind reverting until a fix is ready?

[Amir]

In D127931#3599455, @paulkirth wrote:

Hi, we're seeing a failure in Fuchsia's Clang CI for windows.

The failing test is LLVM :: TableGen/get-operand-type.td

You can find the failing bot here: https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-windows-x64/b8810810347109726705/overview

If this will be hard to address, would you mind reverting until a fix is ready?

Hi @paulkirth,
I’ve addressed this test failure in the 2nd commit (the first one is reverted): https://reviews.llvm.org/rG31e2bba155986b1416c70655c9f5f9e77c2abd46
I couldn’t find a commit hash in the build page, can you please confirm which one is it?

[paulkirth]

Sorry, our CI batches builds so sometimes it's non-obvious. You can find them all under the blame heading. I think they're also somewhere else on that page, but that is always where I go.

It should be 4cd416193cc126355a22b2c9e5c1df3a49b59e50.

[paulkirth]

Sorry, you can ignore my spam here. There's a newer failure that is still ongoing and I've mis-attributed it to this change. I think you're fine as is.

Again, appologies.

[Amir]

In D127931#3599532, @paulkirth wrote:

Sorry, you can ignore my spam here. There's a newer failure that is still ongoing and I've mis-attributed it to this change. I think you're fine as is.

Again, appologies.

No problem, thank you for vigilance!

llvm/test/TableGen/get-operand-type.td

def InstC : Instruction {
   let OutOperandList = (outs RegClass:$d);
   let InOperandList = (ins RegOp:$x);
   field bits<8> Inst;
   field bits<8> SoftFail = 0;
   let Namespace = "MyNamespace";
 }
 
 // CHECK: #ifdef GET_INSTRINFO_OPERAND_TYPE
-// CHECK:        OpTypes::OpA, OpTypes::OpB, OpTypes::i32imm,
-// CHECK-NEXT:   OpTypes::i32imm, -1,
-// CHECK-NEXT:   OpTypes::RegClass, OpTypes::RegOp,
+// CHECK:        /* InstA */
+// CHECK-NEXT:   OpA, OpB, i32imm,
+// CHECK-NEXT:   /* InstB */
+// CHECK-NEXT:   i32imm, -1,
+// CHECK-NEXT:   /* InstC */
+// CHECK-NEXT:   RegClass, RegOp,
 // CHECK: #endif // GET_INSTRINFO_OPERAND_TYPE

llvm/utils/TableGen/InstrInfoEmitter.cpp

void InstrInfoEmitter::emitOperandTypeMappings(
   OS << "#endif // GET_INSTRINFO_OPERAND_TYPES_ENUM\n\n";
 
   OS << "#ifdef GET_INSTRINFO_OPERAND_TYPE\n";
   OS << "#undef GET_INSTRINFO_OPERAND_TYPE\n";
   OS << "namespace llvm {\n";
   OS << "namespace " << Namespace << " {\n";
   OS << "LLVM_READONLY\n";
   OS << "static int getOperandType(uint16_t Opcode, uint16_t OpIdx) {\n";
+  auto getInstrName = [&](int I) -> StringRef {
+    return NumberedInstructions[I]->TheDef->getName();
+  };
   // TODO: Factor out duplicate operand lists to compress the tables.
   if (!NumberedInstructions.empty()) {
     std::vector<int> OperandOffsets;
     std::vector<Record *> OperandRecords;
     int CurrentOffset = 0;
     for (const CodeGenInstruction *Inst : NumberedInstructions) {
       OperandOffsets.push_back(CurrentOffset);
       for (const auto &Op : Inst->Operands) {
         const DagInit *MIOI = Op.MIOperandInfo;
         if (!MIOI || MIOI->getNumArgs() == 0) {
           // Single, anonymous, operand.
           OperandRecords.push_back(Op.Rec);
           ++CurrentOffset;
         } else {
           for (Init *Arg : MIOI->getArgs()) {
             OperandRecords.push_back(cast<DefInit>(Arg)->getDef());
             ++CurrentOffset;
           }
         }
       }
     }
 
     // Emit the table of offsets (indexes) into the operand type table.
     // Size the unsigned integer offset to save space.
     assert(OperandRecords.size() <= UINT32_MAX &&
            "Too many operands for offset table");
     OS << ((OperandRecords.size() <= UINT16_MAX) ? "  const uint16_t"
                                                  : "  const uint32_t");
     OS << " Offsets[] = {\n";
-    for (int I = 0, E = OperandOffsets.size(); I != E; ++I)
+    for (int I = 0, E = OperandOffsets.size(); I != E; ++I) {
+      OS << "    /* " << getInstrName(I) << " */\n";
       OS << "    " << OperandOffsets[I] << ",\n";
+    }
     OS << "  };\n";
 
     // Add an entry for the end so that we don't need to special case it below.
     OperandOffsets.push_back(OperandRecords.size());
 
     // Emit the actual operand types in a flat table.
     // Size the signed integer operand type to save space.
     assert(EnumVal <= INT16_MAX &&
            "Too many operand types for operand types table");
+    OS << "\n  using namespace OpTypes;\n";
     OS << ((EnumVal <= INT8_MAX) ? "  const int8_t" : "  const int16_t");
     OS << " OpcodeOperandTypes[] = {\n    ";
-    for (int I = 0, E = OperandRecords.size(), CurOffset = 1; I != E; ++I) {
+    for (int I = 0, E = OperandRecords.size(), CurOffset = 0; I != E; ++I) {
       // We print each Opcode's operands in its own row.
       if (I == OperandOffsets[CurOffset]) {
-        OS << "\n    ";
-        // If there are empty rows, mark them with an empty comment.
+        OS << "\n    /* " << getInstrName(CurOffset) << " */\n    ";
         while (OperandOffsets[++CurOffset] == I)
-          OS << "/**/\n    ";
+          OS << "/* " << getInstrName(CurOffset) << " */\n    ";
       }
       Record *OpR = OperandRecords[I];
       if ((OpR->isSubClassOf("Operand") ||
            OpR->isSubClassOf("RegisterOperand") ||
            OpR->isSubClassOf("RegisterClass")) &&
           !OpR->isAnonymous())
-        OS << "OpTypes::" << OpR->getName();
+        OS << OpR->getName();
       else
         OS << -1;
       OS << ", ";
     }
     OS << "\n  };\n";
 
     OS << "  return OpcodeOperandTypes[Offsets[Opcode] + OpIdx];\n";
   } else {