[WebAssembly] Fix assembler parsing of br_table.

[aardappel]

Summary

We use variable_ops in the tablegen defs to denote the list of
branch targets in br_table, but unlike other uses of variable_ops
(e.g. call) the these branch targets need to actually be encoded in the
instruction. The existing tables for variable_ops cause not operands
to be accepted by the assembly matcher.

Following the example of ARM:
https://github.com/llvm-mirror/llvm/blob/2cc0a7da876c1d8c32775b0119e1e15aaa759b9e/lib/Target/ARM/ARMInstrInfo.td#L550-L555
we introduce a new operand type to capture this list, and we use the
same {} syntax as ARM as well to differentiate them from regular
integer operands.

[aardappel]

Hey reviewers, consider this WiP since I don't know if this is the most elegant way to do it. Your feedback very welcome!

[aardappel]

lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCCodeEmitter.cpp

97 ↗

(On Diff #177086)

This is pretty yucky. I must I don't quite follow what these flags are used for, so not sure if we should remove these flags from br_table in the tablegen defs instead?

[dschuff]

lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCCodeEmitter.cpp

97 ↗

(On Diff #177086)

I think we can remove the flags from br_table in tablegen because it doesn't use variable_ops anymore. I think they are only used for determining how the variable_ops should be printed.

[aardappel]

lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCCodeEmitter.cpp

97 ↗

(On Diff #177086)

variable_ops are still being used in call, but that doesn't print these either.

Want me to remove these flags as part of this CL or a next one?

[dschuff]

lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCCodeEmitter.cpp

97 ↗

(On Diff #177086)

Might be a bit cleaner to be in this one but I don't have a strong opinion.

[aheejin]

• Could you add "Fixes PR39384." to the CL/commit description?
• Please run clang-format (and maybe also clang-tidy) on the patch

lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp

82 ↗

(On Diff #177086)

Do we need BrL()?

85 ↗

(On Diff #177086)

Do we need this? struct BrLOp only contains a vector of integers.

126 ↗

(On Diff #177086)

Where is this method used? I also can't find where addImmOperands and addRegOperands are used either.

127 ↗

(On Diff #177086)

What is N here? If it is always 1, do we need that argument?

128 ↗

(On Diff #177086)

clang-format

146 ↗

(On Diff #177086)

Does only printing the size provide sufficient info? Can we print the contents too?

366 ↗

(On Diff #177086)

Please clang-format this block

lib/Target/WebAssembly/InstPrinter/WebAssemblyInstPrinter.cpp

253 ↗

(On Diff #177086)

Nit: LLVM coding standard says variable names should start with an upper case letter. I know, it's weird... :(

lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCCodeEmitter.cpp

97 ↗

(On Diff #177086)

I think it should be fine to be removed within this CL, because we are switching from variable_ops to something else.

lib/Target/WebAssembly/WebAssemblyInstrControl.td

61 ↗

(On Diff #177086)

Nit: Maybe these [], "true" can fit the line above?

75 ↗

(On Diff #177086)

Here too

test/MC/WebAssembly/basic-assembly.s

120 ↗

(On Diff #177086)

• If we are gonna check these down to labelN part here, I guess it's better to add the same labels in the end_blocks below.
• Do we not print down to labelN comment for the default case?

[aheejin]

Oh, and just FYI: the ARM code Github link in your CL description wouldn't work once they make changes to the code. You can take a permalink by clicking three dots in the left side of the code and click "Copy permalink", which will get you a link of the specified line in the latest commit. It looks like this: https://github.com/llvm-mirror/llvm/blob/2cc0a7da876c1d8c32775b0119e1e15aaa759b9e/lib/Target/ARM/ARMInstrInfo.td#L550
Also you can do that for for a block of code to by selecting a block by clicking shift and create a permalink for that, like
https://github.com/llvm-mirror/llvm/blob/2cc0a7da876c1d8c32775b0119e1e15aaa759b9e/lib/Target/ARM/ARMInstrInfo.td#L550-L555

[aardappel]

lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp

82 ↗

(On Diff #177086)

It's in a union, so how else would it initialize the vector?

85 ↗

(On Diff #177086)

It's in a union, so only should be destructed if it is off this type. What's a better way to do this?

126 ↗

(On Diff #177086)

Methods in this file are called by the tablegen generated code in "WebAssemblyGenAsmMatcher.inc"

127 ↗

(On Diff #177086)

This is called from generated code, I do not control that argument.

146 ↗

(On Diff #177086)

This print function is for showing operands when you are debugging the assembly matcher itself. It is not used for anything user facing, so does not need to be pretty.

lib/Target/WebAssembly/InstPrinter/WebAssemblyInstPrinter.cpp

253 ↗

(On Diff #177086)

I usually get it right.. this piece of code was copied/adapted from the ARM code. There's a lot of non-conform code in LLVM it appears!

test/MC/WebAssembly/basic-assembly.s

120 ↗

(On Diff #177086)

I was wanting to not have these in the tests at all, but that would require a weird looking test for an empty like (which doesn't work), so I was forced to put these in.

[aardappel]

Code review fixes.

[aardappel]

ARM link updated as permalink, thanks.

[aheejin]

lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp

82 ↗

(On Diff #177086)

Oh right. Sorry I missed this was a union.

85 ↗

(On Diff #177086)

Sorry, nevermind. :)

146 ↗

(On Diff #177086)

But wouldn't you want to know what's inside when you're debugging too?

lib/Target/WebAssembly/InstPrinter/WebAssemblyInstPrinter.cpp

139 ↗

(On Diff #177633)

Missing 'is' after 'this'?

140 ↗

(On Diff #177633)

It's not related to changes in this CL, but this if statement is a bit hard to understand. Could you possibly break it down to two ifs with continues with each of them, maybe with one-line comment for each as well?

227 ↗

(On Diff #177633)

Why was this deleted too?

lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCCodeEmitter.cpp

135 ↗

(On Diff #177633)

Should this be deleted too?

test/MC/WebAssembly/basic-assembly.s

120 ↗

(On Diff #177086)

Yeah but we are checking branch label comments in other instructions within this test too, so it wouldn't hurt to denote where label3 and label4 is anyway, which is also good for readability.

[aardappel]

lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp

146 ↗

(On Diff #177086)

Yes. Just trying to say this is very special purpose though, I don't think it is worth writing until you need it.

lib/Target/WebAssembly/InstPrinter/WebAssemblyInstPrinter.cpp

140 ↗

(On Diff #177633)

Added more explicit comments.

227 ↗

(On Diff #177633)

Since we're basically not using variable_ops in the same way as before anymore (not annotated by TSFlags, not used in call in stack mode, and not used in br_table in the same way), keeping these checks didn't make much sense to me.

lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCCodeEmitter.cpp

135 ↗

(On Diff #177633)

See above.

test/MC/WebAssembly/basic-assembly.s

120 ↗

(On Diff #177086)

Ok, added labels back in.

[aardappel]

More code review fixes.

[aheejin]

lib/Target/WebAssembly/InstPrinter/WebAssemblyInstPrinter.cpp

227 ↗

(On Diff #177633)

Oh what I asked is about

assert(OpNo < Desc.getNumOperands() &&
       "Unexpected floating-point immediate as a non-fixed operand");

Is this related to TSFlags or variable_ops too?

143 ↗

(On Diff #178135)

variable ops_operand -> variable_ops operand ?

lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCCodeEmitter.cpp

135 ↗

(On Diff #177633)

Oh what I asked is about

assert(OpNo < Desc.getNumOperands() &&
       "Unexpected floating-point immediate as a non-fixed operand");

Is this related to TSFlags or variable_ops too?

[aardappel]

lib/Target/WebAssembly/InstPrinter/WebAssemblyInstPrinter.cpp

227 ↗

(On Diff #177633)

Yes, that is what I was claiming. We're not using TSFlags at all anymore.

[aardappel]

Fixed spelling error.

[aheejin]

Probably I don't have the context of what that floating point related assertion means and how it is related to TSFlags. Just curious, could you explain? Otherwise LGTM!

lib/Target/WebAssembly/InstPrinter/WebAssemblyInstPrinter.cpp

227 ↗

(On Diff #177633)

Sorry, but I don't understand, maybe I don't have the context of this assertion. How is

assert(OpNo < Desc.getNumOperands() &&
       "Unexpected floating-point immediate as a non-fixed operand");

related to TSFlags?

[aardappel]

@aheejin they were protecting against a floating point operand showing up as part of variable_ops. I don't think think this was a particularly crucial assert to start with.

llvm/trunk/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp

 
 #define DEBUG_TYPE "wasm-asm-parser"
 
 namespace {
 
 /// WebAssemblyOperand - Instances of this class represent the operands in a
 /// parsed WASM machine instruction.
 struct WebAssemblyOperand : public MCParsedAsmOperand {
-  enum KindTy { Token, Integer, Float, Symbol } Kind;
+  enum KindTy { Token, Integer, Float, Symbol, BrList } Kind;
 
   SMLoc StartLoc, EndLoc;
 
   struct TokOp {
     StringRef Tok;
   };
 
   struct IntOp {
     int64_t Val;
   };
 
   struct FltOp {
     double Val;
   };
 
   struct SymOp {
     const MCExpr *Exp;
   };
 
+  struct BrLOp {
+    std::vector<unsigned> List;
+  };
+
   union {
     struct TokOp Tok;
     struct IntOp Int;
     struct FltOp Flt;
     struct SymOp Sym;
+    struct BrLOp BrL;
   };
 
   WebAssemblyOperand(KindTy K, SMLoc Start, SMLoc End, TokOp T)
       : Kind(K), StartLoc(Start), EndLoc(End), Tok(T) {}
   WebAssemblyOperand(KindTy K, SMLoc Start, SMLoc End, IntOp I)
       : Kind(K), StartLoc(Start), EndLoc(End), Int(I) {}
   WebAssemblyOperand(KindTy K, SMLoc Start, SMLoc End, FltOp F)
       : Kind(K), StartLoc(Start), EndLoc(End), Flt(F) {}
   WebAssemblyOperand(KindTy K, SMLoc Start, SMLoc End, SymOp S)
       : Kind(K), StartLoc(Start), EndLoc(End), Sym(S) {}
+  WebAssemblyOperand(KindTy K, SMLoc Start, SMLoc End)
+      : Kind(K), StartLoc(Start), EndLoc(End), BrL() {}
+
+  ~WebAssemblyOperand() {
+    if (isBrList())
+      BrL.~BrLOp();
+  }
 
   bool isToken() const override { return Kind == Token; }
   bool isImm() const override {
     return Kind == Integer || Kind == Float || Kind == Symbol;
   }
   bool isMem() const override { return false; }
   bool isReg() const override { return false; }
+  bool isBrList() const { return Kind == BrList; }
 
   unsigned getReg() const override {
     llvm_unreachable("Assembly inspects a register operand");
     return 0;
   }
 
   StringRef getToken() const {
     assert(isToken());
void addImmOperands(MCInst &Inst, unsigned N) const {
     else if (Kind == Float)
       Inst.addOperand(MCOperand::createFPImm(Flt.Val));
     else if (Kind == Symbol)
       Inst.addOperand(MCOperand::createExpr(Sym.Exp));
     else
       llvm_unreachable("Should be immediate or symbol!");
   }
 
+  void addBrListOperands(MCInst &Inst, unsigned N) const {
+    assert(N == 1 && isBrList() && "Invalid BrList!");
+    for (auto Br : BrL.List)
+      Inst.addOperand(MCOperand::createImm(Br));
+  }
+
   void print(raw_ostream &OS) const override {
     switch (Kind) {
     case Token:
       OS << "Tok:" << Tok.Tok;
       break;
     case Integer:
       OS << "Int:" << Int.Val;
       break;
     case Float:
       OS << "Flt:" << Flt.Val;
       break;
     case Symbol:
       OS << "Sym:" << Sym.Exp;
       break;
+    case BrList:
+      OS << "BrList:" << BrL.List.size();
+      break;
     }
   }
 };
 
 class WebAssemblyAsmParser final : public MCTargetAsmParser {
   MCAsmParser &Parser;
   MCAsmLexer &Lexer;
 
while (Lexer.isNot(AsmToken::EndOfStatement)) {
         if (Tok.getString().getAsDouble(Val, false))
           return error("Cannot parse real: ", Tok);
         Operands.push_back(make_unique<WebAssemblyOperand>(
             WebAssemblyOperand::Float, Tok.getLoc(), Tok.getEndLoc(),
             WebAssemblyOperand::FltOp{Val}));
         Parser.Lex();
         break;
       }
+      case AsmToken::LCurly: {
+        Parser.Lex();
+        auto Op = make_unique<WebAssemblyOperand>(
+            WebAssemblyOperand::BrList, Tok.getLoc(), Tok.getEndLoc());
+        if (!Lexer.is(AsmToken::RCurly))
+          for (;;) {
+            Op->BrL.List.push_back(Lexer.getTok().getIntVal());
+            expect(AsmToken::Integer, "integer");
+            if (!isNext(AsmToken::Comma))
+              break;
+          }
+        expect(AsmToken::RCurly, "}");
+        Operands.push_back(std::move(Op));
+        break;
+      }
       default:
         return error("Unexpected token in operand: ", Tok);
       }
       if (Lexer.isNot(AsmToken::EndOfStatement)) {
         if (expect(AsmToken::Comma, ","))
           return true;
       }
     }

llvm/trunk/lib/Target/WebAssembly/InstPrinter/WebAssemblyInstPrinter.h

WebAssemblyInstPrinter(const MCAsmInfo &MAI, const MCInstrInfo &MII,
                          const MCRegisterInfo &MRI);
 
   void printRegName(raw_ostream &OS, unsigned RegNo) const override;
   void printInst(const MCInst *MI, raw_ostream &OS, StringRef Annot,
                  const MCSubtargetInfo &STI) override;
 
   // Used by tblegen code.
   void printOperand(const MCInst *MI, unsigned OpNo, raw_ostream &O);
+  void printBrList(const MCInst *MI, unsigned OpNo, raw_ostream &O);
   void printWebAssemblyP2AlignOperand(const MCInst *MI, unsigned OpNo,
                                       raw_ostream &O);
   void printWebAssemblySignatureOperand(const MCInst *MI, unsigned OpNo,
                                         raw_ostream &O);
 
   // Autogenerated by tblgen.
   void printInstruction(const MCInst *MI, raw_ostream &O);
   static const char *getRegisterName(unsigned RegNo);

llvm/trunk/lib/Target/WebAssembly/InstPrinter/WebAssemblyInstPrinter.cpp

case WebAssembly::CATCH_ALL_S:
       LastSeenEHInst = CATCH;
       break;
     }
 
     // Annotate any control flow label references.
     unsigned NumFixedOperands = Desc.NumOperands;
     SmallSet<uint64_t, 8> Printed;
     for (unsigned i = 0, e = MI->getNumOperands(); i < e; ++i) {
-      if (!(i < NumFixedOperands
-                ? (Desc.OpInfo[i].OperandType ==
-                   WebAssembly::OPERAND_BASIC_BLOCK)
-                : (Desc.TSFlags & WebAssemblyII::VariableOpImmediateIsLabel)))
+      // See if this operand denotes a basic block target.
+      if (i < NumFixedOperands) {
+        // A non-variable_ops operand, check its type.
+        if (Desc.OpInfo[i].OperandType != WebAssembly::OPERAND_BASIC_BLOCK)
           continue;
+      } else {
+        // A variable_ops operand, which currently can be immediates (used in
+        // br_table) which are basic block targets, or for call instructions
+        // when using -wasm-keep-registers (in which case they are registers,
+        // and should not be processed).
+        if (!MI->getOperand(i).isImm())
+          continue;
+      }
       uint64_t Depth = MI->getOperand(i).getImm();
       if (!Printed.insert(Depth).second)
         continue;
 
       if (Opc == WebAssembly::RETHROW || Opc == WebAssembly::RETHROW_S) {
         assert(Depth <= EHPadStack.size() && "Invalid depth argument!");
         if (Depth == EHPadStack.size()) {
           // This can happen when rethrow instruction breaks out of all nests
static std::string toString(const APFloat &FP) {
   assert(Written < BufBytes);
   return buf;
 }
 
 void WebAssemblyInstPrinter::printOperand(const MCInst *MI, unsigned OpNo,
                                           raw_ostream &O) {
   const MCOperand &Op = MI->getOperand(OpNo);
   if (Op.isReg()) {
-    assert((OpNo < MII.get(MI->getOpcode()).getNumOperands() ||
-            MII.get(MI->getOpcode()).TSFlags == 0) &&
-           "WebAssembly variable_ops register ops don't use TSFlags");
     unsigned WAReg = Op.getReg();
     if (int(WAReg) >= 0)
       printRegName(O, WAReg);
     else if (OpNo >= MII.get(MI->getOpcode()).getNumDefs())
       O << "$pop" << WebAssemblyFunctionInfo::getWARegStackId(WAReg);
     else if (WAReg != WebAssemblyFunctionInfo::UnusedReg)
       O << "$push" << WebAssemblyFunctionInfo::getWARegStackId(WAReg);
     else
       O << "$drop";
     // Add a '=' suffix if this is a def.
     if (OpNo < MII.get(MI->getOpcode()).getNumDefs())
       O << '=';
   } else if (Op.isImm()) {
-    const MCInstrDesc &Desc = MII.get(MI->getOpcode());
-    assert((OpNo < Desc.getNumOperands() ||
-            (Desc.TSFlags & WebAssemblyII::VariableOpIsImmediate)) &&
-           "WebAssemblyII::VariableOpIsImmediate should be set for "
-           "variable_ops immediate ops");
-    (void)Desc;
-    // TODO: (MII.get(MI->getOpcode()).TSFlags &
-    //        WebAssemblyII::VariableOpImmediateIsLabel)
-    // can tell us whether this is an immediate referencing a label in the
-    // control flow stack, and it may be nice to pretty-print.
     O << Op.getImm();
   } else if (Op.isFPImm()) {
     const MCInstrDesc &Desc = MII.get(MI->getOpcode());
-    assert(OpNo < Desc.getNumOperands() &&
-           "Unexpected floating-point immediate as a non-fixed operand");
-    assert(Desc.TSFlags == 0 &&
-           "WebAssembly variable_ops floating point ops don't use TSFlags");
     const MCOperandInfo &Info = Desc.OpInfo[OpNo];
     if (Info.OperandType == WebAssembly::OPERAND_F32IMM) {
       // TODO: MC converts all floating point immediate operands to double.
       // This is fine for numeric values, but may cause NaNs to change bits.
       O << ::toString(APFloat(float(Op.getFPImm())));
     } else {
       assert(Info.OperandType == WebAssembly::OPERAND_F64IMM);
       O << ::toString(APFloat(Op.getFPImm()));
     }
   } else {
-    assert((OpNo < MII.get(MI->getOpcode()).getNumOperands() ||
-            (MII.get(MI->getOpcode()).TSFlags &
-             WebAssemblyII::VariableOpIsImmediate)) &&
-           "WebAssemblyII::VariableOpIsImmediate should be set for "
-           "variable_ops expr ops");
     assert(Op.isExpr() && "unknown operand kind in printOperand");
     Op.getExpr()->print(O, &MAI);
   }
 }
 
+void WebAssemblyInstPrinter::printBrList(const MCInst *MI, unsigned OpNo,
+                                         raw_ostream &O) {
+  O << "{";
+  for (unsigned I = OpNo, E = MI->getNumOperands(); I != E; ++I) {
+    if (I != OpNo)
+      O << ", ";
+    O << MI->getOperand(I).getImm();
+  }
+  O << "}";
+}
+
 void WebAssemblyInstPrinter::printWebAssemblyP2AlignOperand(const MCInst *MI,
                                                             unsigned OpNo,
                                                             raw_ostream &O) {
   int64_t Imm = MI->getOperand(OpNo).getImm();
   if (Imm == WebAssembly::GetDefaultP2Align(MI->getOpcode()))
     return;
   O << ":p2align=" << Imm;
 }

llvm/trunk/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCCodeEmitter.cpp

void WebAssemblyMCCodeEmitter::encodeInstruction(
   const MCInstrDesc &Desc = MCII.get(MI.getOpcode());
   for (unsigned i = 0, e = MI.getNumOperands(); i < e; ++i) {
     const MCOperand &MO = MI.getOperand(i);
     if (MO.isReg()) {
       /* nothing to encode */
 
     } else if (MO.isImm()) {
       if (i < Desc.getNumOperands()) {
-        assert(Desc.TSFlags == 0 &&
-               "WebAssembly non-variable_ops don't use TSFlags");
         const MCOperandInfo &Info = Desc.OpInfo[i];
         LLVM_DEBUG(dbgs() << "Encoding immediate: type="
                           << int(Info.OperandType) << "\n");
         switch (Info.OperandType) {
         case WebAssembly::OPERAND_I32IMM:
           encodeSLEB128(int32_t(MO.getImm()), OS);
           break;
         case WebAssembly::OPERAND_OFFSET32:
if (MO.isReg()) {
           support::endian::write<uint64_t>(OS, MO.getImm(), support::little);
           break;
         case WebAssembly::OPERAND_GLOBAL:
           llvm_unreachable("wasm globals should only be accessed symbolicly");
         default:
           encodeULEB128(uint64_t(MO.getImm()), OS);
         }
       } else {
-        assert(Desc.TSFlags == (WebAssemblyII::VariableOpIsImmediate |
-                                WebAssemblyII::VariableOpImmediateIsLabel));
         encodeULEB128(uint64_t(MO.getImm()), OS);
       }
 
     } else if (MO.isFPImm()) {
-      assert(i < Desc.getNumOperands() &&
-             "Unexpected floating-point immediate as a non-fixed operand");
-      assert(Desc.TSFlags == 0 &&
-             "WebAssembly variable_ops floating point ops don't use TSFlags");
       const MCOperandInfo &Info = Desc.OpInfo[i];
       if (Info.OperandType == WebAssembly::OPERAND_F32IMM) {
         // TODO: MC converts all floating point immediate operands to double.
         // This is fine for numeric values, but may cause NaNs to change bits.
         float f = float(MO.getFPImm());
         support::endian::write<float>(OS, f, support::little);
       } else {
         assert(Info.OperandType == WebAssembly::OPERAND_F64IMM);

llvm/trunk/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCTargetDesc.h

enum OperandType {
   /// type signature immediate for call_indirect.
   OPERAND_TYPEINDEX,
   /// Event index.
   OPERAND_EVENT,
 };
 } // end namespace WebAssembly
 
 namespace WebAssemblyII {
-enum {
-  // For variadic instructions, this flag indicates whether an operand
-  // in the variable_ops range is an immediate value.
-  VariableOpIsImmediate = (1 << 0),
-  // For immediate values in the variable_ops range, this flag indicates
-  // whether the value represents a control-flow label.
-  VariableOpImmediateIsLabel = (1 << 1)
-};
 
 /// Target Operand Flag enum.
 enum TOF {
   MO_NO_FLAG = 0,
 
   // Flags to indicate the type of the symbol being referenced
   MO_SYMBOL_FUNCTION = 0x1,
   MO_SYMBOL_GLOBAL = 0x2,

llvm/trunk/lib/Target/WebAssembly/WebAssemblyInstrControl.td

 } // isBarrier = 1
 } // isBranch = 1, isTerminator = 1, hasCtrlDep = 1
 
 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"; }
+def brlist : Operand<i32> {
+  let ParserMatchClass = BrListAsmOperand;
+  let PrintMethod = "printBrList";
+}
+
 // 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.
-// Set TSFlags{0} to 1 to indicate that the variable_ops are immediates.
-// Set TSFlags{1} to 1 to indicate that the immediates represent labels.
 // 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
 def BR_TABLE_I32 : NI<(outs), (ins I32:$index, variable_ops),
                       [(WebAssemblybr_table I32:$index)], "false",
                       "br_table \t$index", 0x0e> {
-  let TSFlags{0} = 1;
-  let TSFlags{1} = 1;
 }
 let BaseName = "BR_TABLE_I32" in
-def BR_TABLE_I32_S : NI<(outs), (ins variable_ops),
-                        [], "true",
-                        "br_table \t", 0x0e> {
-  let TSFlags{0} = 1;
-  let TSFlags{1} = 1;
+def BR_TABLE_I32_S : NI<(outs), (ins brlist:$brl), [], "true",
+                        "br_table \t$brl", 0x0e> {
 }
 let isCodeGenOnly = 1 in
 def BR_TABLE_I64 : NI<(outs), (ins I64:$index, variable_ops),
                       [(WebAssemblybr_table I64:$index)], "false",
                       "br_table \t$index"> {
-  let TSFlags{0} = 1;
-  let TSFlags{1} = 1;
 }
 let BaseName = "BR_TABLE_I64" in
-def BR_TABLE_I64_S : NI<(outs), (ins variable_ops),
-                        [], "true",
-                        "br_table \t"> {
-  let TSFlags{0} = 1;
-  let TSFlags{1} = 1;
+def BR_TABLE_I64_S : NI<(outs), (ins brlist:$brl), [], "true",
+                        "br_table \t$brl"> {
 }
 } // isTerminator = 1, hasCtrlDep = 1, isBarrier = 1
 
 // This is technically a control-flow instruction, since all it affects is the
 // IP.
 defm NOP : NRI<(outs), (ins), [], "nop", 0x01>;
 
 // Placemarkers to indicate the start or end of a block or loop scope.

llvm/trunk/test/CodeGen/WebAssembly/stack-insts.ll

 ; RUN: llc < %s -asm-verbose=false -disable-wasm-fallthrough-return-opt -disable-block-placement -verify-machineinstrs | FileCheck %s
 
 target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
 target triple = "wasm32-unknown-unknown"
 
 declare void @foo0()
 declare void @foo1()
 
 ; Tests if br_table is printed correctly with a tab.
 ; CHECK-LABEL: test0:
-; CHECK-NOT: br_table0, 1, 0, 1, 0
-; CHECK: br_table 0, 1, 0, 1, 0
+; CHECK: br_table {0, 1, 0, 1, 0}
 define void @test0(i32 %n) {
 entry:
   switch i32 %n, label %sw.epilog [
     i32 0, label %sw.bb
     i32 1, label %sw.bb.1
     i32 2, label %sw.bb
     i32 3, label %sw.bb.1
   ]

llvm/trunk/test/MC/WebAssembly/basic-assembly.s

.LBB0_1:
     get_local   0
     i32.lt_s
     br_if       0        # 0: up to label1
 .LBB0_2:
     end_loop
     end_block                       # label0:
     get_local   4
     get_local   5
+    block
+    block
+    block
+    block
+    br_table {0, 1, 2}   # 2 entries, default
+    end_block            # first entry jumps here.
+    i32.const 1
+    br 2
+    end_block            # second entry jumps here.
+    i32.const 2
+    br 1
+    end_block            # default jumps here.
+    i32.const 3
+    end_block            # "switch" exit.
     f32x4.add
     # Test correct parsing of instructions with / and : in them:
     # TODO: enable once instruction has been added.
     #i32x4.trunc_s/f32x4:sat
     i32.trunc_s/f32
     try
 .LBB0_3:
     i32.catch   0

 # CHECK-NEXT:      get_local   0
 # CHECK-NEXT:      i32.lt_s
 # CHECK-NEXT:      br_if 0            # 0: up to label1
 # CHECK-NEXT:  .LBB0_2:
 # CHECK-NEXT:      end_loop
 # CHECK-NEXT:      end_block                       # label0:
 # CHECK-NEXT:      get_local   4
 # CHECK-NEXT:      get_local   5
+# CHECK-NEXT:      block
+# CHECK-NEXT:      block
+# CHECK-NEXT:      block
+# CHECK-NEXT:      block
+# CHECK-NEXT:      br_table {0, 1, 2}  # 1: down to label4
+# CHECK-NEXT:                          # 2: down to label3
+# CHECK-NEXT:      end_block           # label5:
+# CHECK-NEXT:      i32.const 1
+# CHECK-NEXT:      br 2                # 2: down to label2
+# CHECK-NEXT:      end_block           # label4:
+# CHECK-NEXT:      i32.const 2
+# CHECK-NEXT:      br 1                # 1: down to label2
+# CHECK-NEXT:      end_block           # label3:
+# CHECK-NEXT:      i32.const 3
+# CHECK-NEXT:      end_block           # label2:
 # CHECK-NEXT:      f32x4.add
 # CHECK-NEXT:      i32.trunc_s/f32
 # CHECK-NEXT:      try
 # CHECK-NEXT:  .LBB0_3:
 # CHECK-NEXT:      i32.catch   0
 # CHECK-NEXT:  .LBB0_4:
 # CHECK-NEXT:      catch_all
 # CHECK-NEXT:  .LBB0_5:
 # CHECK-NEXT:      end_try
 # CHECK-NEXT:      get_global  __stack_pointer@GLOBAL
 # CHECK-NEXT:      end_function
 
 # CHECK:           .globaltype  __stack_pointer, i32