[WebAssembly] Fixed disassembler not knowing about new brlist operand

[aardappel]

Summary

The previously introduced new operand type for br_table didn't have
a disassembler implementation, causing an assert.

[aheejin]

LGTM with nits

lib/Target/WebAssembly/WebAssemblyInstrControl.td

45

Nit: double space

utils/TableGen/WebAssemblyDisassemblerEmitter.cpp

95

How is this related?

[aardappel]

Nit

utils/TableGen/WebAssemblyDisassemblerEmitter.cpp

95

The original problem was triggered by the disassembler asserting on OPERAND_UNKNOWN when trying to disassemble br_table. There shouldn't really ever be any OPERAND_UNKNOWN in the operand table. If this assert had been here before, I would have caught this problem at build time rather than at runtime.

lib/Target/WebAssembly/Disassembler/WebAssemblyDisassembler.cpp

MCDisassembler::DecodeStatus WebAssemblyDisassembler::getInstruction(
   }
   if (WasmInst->ET == ET_Unused)
     return MCDisassembler::Fail;
   // At this point we must have a valid instruction to decode.
   assert(WasmInst->ET == ET_Instruction);
   MI.setOpcode(WasmInst->Opcode);
   // Parse any operands.
   for (uint8_t OPI = 0; OPI < WasmInst->NumOperands; OPI++) {
-    switch (OperandTable[WasmInst->OperandStart + OPI]) {
+    auto OT = OperandTable[WasmInst->OperandStart + OPI];
+    switch (OT) {
     // ULEB operands:
     case WebAssembly::OPERAND_BASIC_BLOCK:
     case WebAssembly::OPERAND_LOCAL:
     case WebAssembly::OPERAND_GLOBAL:
     case WebAssembly::OPERAND_FUNCTION32:
     case WebAssembly::OPERAND_OFFSET32:
     case WebAssembly::OPERAND_P2ALIGN:
     case WebAssembly::OPERAND_TYPEINDEX:
case WebAssembly::OPERAND_VEC_I32IMM: {
         return MCDisassembler::Fail;
       break;
     }
     case WebAssembly::OPERAND_VEC_I64IMM: {
       if (!parseImmediate<uint64_t>(MI, Size, Bytes))
         return MCDisassembler::Fail;
       break;
     }
+    case WebAssembly::OPERAND_BRLIST: {
+      int64_t TargetTableLen;
+      if (!nextLEB(TargetTableLen, Bytes, Size, false))
+        return MCDisassembler::Fail;
+      for (int64_t I = 0; I < TargetTableLen; I++) {
+        if (!parseLEBImmediate(MI, Size, Bytes, false))
+          return MCDisassembler::Fail;
+      }
+      // Default case.
+      if (!parseLEBImmediate(MI, Size, Bytes, false))
+        return MCDisassembler::Fail;
+      break;
+    }
     case MCOI::OPERAND_REGISTER:
       // The tablegen header currently does not have any register operands since
       // we use only the stack (_S) instructions.
       // If you hit this that probably means a bad instruction definition in
       // tablegen.
       llvm_unreachable("Register operand in WebAssemblyDisassembler");
     default:
       llvm_unreachable("Unknown operand type in WebAssemblyDisassembler");
     }
   }
   return MCDisassembler::Success;
 }

lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCTargetDesc.h

enum OperandType {
   /// p2align immediate for load and store address alignment.
   OPERAND_P2ALIGN,
   /// signature immediate for block/loop.
   OPERAND_SIGNATURE,
   /// type signature immediate for call_indirect.
   OPERAND_TYPEINDEX,
   /// Event index.
   OPERAND_EVENT,
+  /// A list of branch targets for br_list.
+  OPERAND_BRLIST,
 };
 } // end namespace WebAssembly
 
 namespace WebAssemblyII {
 
 /// Target Operand Flag enum.
 enum TOF {
   MO_NO_FLAG = 0,

lib/Target/WebAssembly/WebAssemblyInstrControl.td

 def : Pat<(brcond (i32 (setne I32:$cond, 0)), bb:$dst),
           (BR_IF bb_op:$dst, I32:$cond)>;
 def : Pat<(brcond (i32 (seteq I32:$cond, 0)), bb:$dst),
           (BR_UNLESS bb_op:$dst, I32:$cond)>;
 
 // A list of branch targets enclosed in {} and separated by comma.
 // Used by br_table only.
 def BrListAsmOperand : AsmOperandClass { let Name = "BrList"; }
+let OperandNamespace = "WebAssembly" in {
+let OperandType = "OPERAND_BRLIST" in {
 def brlist : Operand<i32> {
   let ParserMatchClass = BrListAsmOperand;
   let PrintMethod = "printBrList";
 }
+}  // OPERAND_BRLIST
+}  // OperandNamespace = "WebAssembly"
 
 // TODO: SelectionDAG's lowering insists on using a pointer as the index for
 // jump tables, so in practice we don't ever use BR_TABLE_I64 in wasm32 mode
 // currently.
 // FIXME: this can't inherit from I<> since there is no way to inherit from a
 // multiclass and still have the let statements.
 let isTerminator = 1, hasCtrlDep = 1, isBarrier = 1 in {
 let isCodeGenOnly = 1 in

test/MC/Disassembler/WebAssembly/wasm.txt

 # Check LEB128 encoding of SIMD instructions
 # CHECK: i64x2.all_true
 0xFD 0x86 0x01
 
 # Including non-canonical LEB128 encodings
 # CHECK: i64x2.any_true
 # CHECK-NOT: i64.div_u
 0xFD 0x85 0x81 0x80 0x80 0x80 0x80 0x00
+
+# Check br_table, which has its own operand type.
+# CHECK: br_table {0, 1, 2}
+0x0E 0x02 0x00 0x01 0x02

utils/TableGen/WebAssemblyDisassemblerEmitter.cpp

for (unsigned I = 0; I < WebAssemblyInstructionTableSize; I++) {
         OS << "  // 0x";
         OS.write_hex(static_cast<unsigned long long>(I));
         OS << ": " << CGI.AsmString << "\n";
         OS << "  { " << InstIt->second.first << ", ET_Instruction, ";
         OS << CGI.Operands.OperandList.size() << ", ";
         // Collect operand types for storage in a shared list.
         CurOperandList.clear();
         for (auto &Op : CGI.Operands.OperandList) {
+          assert(Op.OperandType != "MCOI::OPERAND_UNKNOWN");
           CurOperandList.push_back(Op.OperandType);
         }
         // See if we already have stored this sequence before. This is not
         // strictly necessary but makes the table really small.
         size_t OperandStart = OperandTable.size();
         if (CurOperandList.size() <= OperandTable.size()) {
           for (size_t J = 0; J <= OperandTable.size() - CurOperandList.size();
                ++J) {