[WebAssembly] Allow multivalue types in block signature operands

[tlively]

Summary

Renames ExprType to the more apt BlockType and adds a variant for
multivalue blocks. Currently non-void blocks are only generated at the
end of functions where the block return type needs to agree with the
function return type, and that remains true for multivalue
blocks. That invariant means that the actual signature does not need
to be stored in the block signature MachineOperand because it can be
inferred by WebAssemblyMCInstLower from the return type of the
parent function. WebAssemblyMCInstLower continues to lower block
signature operands to immediates when possible but lowers multivalue
signatures to function type symbols. The AsmParser and Disassembler
are updated to handle multivalue block types as well.

[sbc100]

llvm/tools/llvm-mc/llvm-mc.cpp

520

This change to remove the context creation seems seperate. If so can you split it out? That was this change can stay wasm specific.

[tlively]

llvm/tools/llvm-mc/llvm-mc.cpp

520

It's not separate, unfortunately. In order to create a wasm symbol from the disassembler we need a bunch of target information that this target contains that the old one did not have.

[tlively]

llvm/tools/llvm-mc/llvm-mc.cpp

520

I am happy to reconsider the wisdom of creating a symbol in the disassembler, though.

[sbc100]

llvm/tools/llvm-mc/llvm-mc.cpp

520

I was confused because the existing disassembly already needs a context and even creates one saying: // Set up the MCContext for creating symbols and MCExpr's.. So that approach seems justified and consistent with the existing code. I'm not clear why it wasn't done this way to begin with.

[tlively]

llvm/tools/llvm-mc/llvm-mc.cpp

520

Yeah it was surprising that the previous code didn't work. With the previous code, asking the context for a symbol returned a generic MCSymbol. With the new code it returns a MCSymbolWasm, which we need to be able to set the symbol type to wasm::WASM_SYMBOL_TYPE_FUNCTION.

[dschuff]

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

135

The invariant that only fallthrough-return blocks are allowed to be multivalue should probably be restated here.
edit: I guess it's not just fallthrough-return blocks, right? Just the last block, even if it has an explicit return?
We should probably have some more tests for those cases.

[tlively]

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

135

Right, it's the last block and the last block within that block recursively. In fact this only happens with explicit returns inside the blocks (or blocks that are otherwise never exited) because blocks that could be fallthrough return blocks instead set their results to a local then have a local.get in the return position.

I will expand this comment to reiterate this invariant.

I only added one additional test for this case because the algorithm for determining when to set block types was not changed and is already tested in cfg-stackify.ll.

[tlively]

• Explain more about multivalue types in WebAssembly::BlockType

[aheejin]

Nice! Mostly LGTM.

Currently non-void blocks are only generated at the end of functions where the block return type needs to agree with the function return type, and that remains true for multivalue blocks. That invariant means that the actual signature does not need to be stored in the block signature MachineOperand because it can be inferred by WebAssemblyMCInstLower from the return type of the parent function.

I guess this is a tentative state before you implement the rest of the proposal in full, right? If other blocks are able to return multivalue, are we gonna change their operands to also take typeindex?

llvm/lib/Target/WebAssembly/Disassembler/WebAssemblyDisassembler.cpp

224

What are septet values, and when is Val negative? All values of BlockType look unsigned. Maybe I'm missing something..?

227

When does this happen?

239

Disassembler is not going to print multivalue signatures then? Is this tentative or permanent?

llvm/lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp

203

Nit: Do we need llvm:: here?

[aheejin]

llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp

1248

It's preexisting, but I think we should add END_TRY here too. Not sure if I can generate a test case that ends with end_try returning something easily though..

[tlively]

llvm/lib/Target/WebAssembly/Disassembler/WebAssemblyDisassembler.cpp

224

See https://webassembly.github.io/multi-value/core/binary/instructions.html#control-instructions for details on the encoding. By septet value I just mean a group of 7 bits. That's where the & 0x7f comes from.

227

See wasm-error.txt for an example. Basically anytime you have a negative SLEB128 value here that occupies more than one byte, which is invalid according to the spec.

239

I don't think there's any way to access the type section from the disassembler, so this is permanent unless there's a large overhaul. cc @aardappel for the details.

llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp

1248

Sounds good.

[aheejin]

In D68889#1706857, @aheejin wrote:

Nice! Mostly LGTM.

Currently non-void blocks are only generated at the end of functions where the block return type needs to agree with the function return type, and that remains true for multivalue blocks. That invariant means that the actual signature does not need to be stored in the block signature MachineOperand because it can be inferred by WebAssemblyMCInstLower from the return type of the parent function.

I guess this is a tentative state before you implement the rest of the proposal in full, right? If other blocks are able to return multivalue, are we gonna change their operands to also take typeindex?

Ping

[tlively]

In D68889#1708287, @aheejin wrote:

In D68889#1706857, @aheejin wrote:

Nice! Mostly LGTM.

Currently non-void blocks are only generated at the end of functions where the block return type needs to agree with the function return type, and that remains true for multivalue blocks. That invariant means that the actual signature does not need to be stored in the block signature MachineOperand because it can be inferred by WebAssemblyMCInstLower from the return type of the parent function.

I guess this is a tentative state before you implement the rest of the proposal in full, right? If other blocks are able to return multivalue, are we gonna change their operands to also take typeindex?

I don't think we have to change this, or if we do it could be considered a separate project. Up to now we have been ok not using the ability for blocks to produce values, so I don't see a pressing need to change that. When we do work on multivalue blocks, it will be more important to have them in Binaryen first.

[tlively]

• Address comments

[aardappel]

llvm/lib/Target/WebAssembly/Disassembler/WebAssemblyDisassembler.cpp

233

Is this necessary? A disassembler is really just a function that turns a range of bytes into a string, it having a "side effect" of creating symbols seems odd to me.

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

if (Type == "v128" || Type == "i8x16" || Type == "i16x8" ||
         Type == "i32x4" || Type == "i64x2" || Type == "f32x4" ||
         Type == "f64x2")
       return wasm::ValType::V128;
     if (Type == "exnref")
       return wasm::ValType::EXNREF;
     return Optional<wasm::ValType>();
   }
 
-  WebAssembly::ExprType parseBlockType(StringRef ID) {
-    return StringSwitch<WebAssembly::ExprType>(ID)
-        .Case("i32", WebAssembly::ExprType::I32)
-        .Case("i64", WebAssembly::ExprType::I64)
-        .Case("f32", WebAssembly::ExprType::F32)
-        .Case("f64", WebAssembly::ExprType::F64)
-        .Case("v128", WebAssembly::ExprType::V128)
-        .Case("exnref", WebAssembly::ExprType::Exnref)
-        .Case("void", WebAssembly::ExprType::Void)
-        .Default(WebAssembly::ExprType::Invalid);
+  WebAssembly::BlockType parseBlockType(StringRef ID) {
+    // Multivalue block types are handled separately in parseSignature
+    return StringSwitch<WebAssembly::BlockType>(ID)
+        .Case("i32", WebAssembly::BlockType::I32)
+        .Case("i64", WebAssembly::BlockType::I64)
+        .Case("f32", WebAssembly::BlockType::F32)
+        .Case("f64", WebAssembly::BlockType::F64)
+        .Case("v128", WebAssembly::BlockType::V128)
+        .Case("exnref", WebAssembly::BlockType::Exnref)
+        .Case("void", WebAssembly::BlockType::Void)
+        .Default(WebAssembly::BlockType::Invalid);
   }
 
   bool parseRegTypeList(SmallVectorImpl<wasm::ValType> &Types) {
     while (Lexer.is(AsmToken::Identifier)) {
       auto Type = parseType(Lexer.getTok().getString());
       if (!Type)
         return error("unknown type: ", Lexer.getTok());
       Types.push_back(Type.getValue());
if (IsLoadStore || IsAtomic) {
             WebAssemblyOperand::Integer, Tok.getLoc(), Tok.getEndLoc(),
             WebAssemblyOperand::IntOp{-1}));
       }
     }
     return false;
   }
 
   void addBlockTypeOperand(OperandVector &Operands, SMLoc NameLoc,
-                           WebAssembly::ExprType BT) {
+                           WebAssembly::BlockType BT) {
     Operands.push_back(std::make_unique<WebAssemblyOperand>(
         WebAssemblyOperand::Integer, NameLoc, NameLoc,
         WebAssemblyOperand::IntOp{static_cast<int64_t>(BT)}));
   }
 
   bool ParseInstruction(ParseInstructionInfo & /*Info*/, StringRef Name,
                         SMLoc NameLoc, OperandVector &Operands) override {
     // Note: Name does NOT point into the sourcecode, but to a local, so
bool ParseInstruction(ParseInstructionInfo & /*Info*/, StringRef Name,
     // Now construct the name as first operand.
     Operands.push_back(std::make_unique<WebAssemblyOperand>(
         WebAssemblyOperand::Token, NameLoc, SMLoc::getFromPointer(Name.end()),
         WebAssemblyOperand::TokOp{Name}));
 
     // If this instruction is part of a control flow structure, ensure
     // proper nesting.
     bool ExpectBlockType = false;
+    bool ExpectFuncType = false;
     if (Name == "block") {
       push(Block);
       ExpectBlockType = true;
     } else if (Name == "loop") {
       push(Loop);
       ExpectBlockType = true;
     } else if (Name == "try") {
       push(Try);
if (Name == "block") {
       if (pop(Name, Block))
         return true;
     } else if (Name == "end_function") {
       ensureLocals(getStreamer());
       CurrentState = EndFunction;
       if (pop(Name, Function) || ensureEmptyNestingStack())
         return true;
     } else if (Name == "call_indirect" || Name == "return_call_indirect") {
+      ExpectFuncType = true;
+    }
+
+    if (ExpectFuncType || (ExpectBlockType && Lexer.is(AsmToken::LParen))) {
       // This has a special TYPEINDEX operand which in text we
       // represent as a signature, such that we can re-build this signature,
       // attach it to an anonymous symbol, which is what WasmObjectWriter
       // expects to be able to recreate the actual unique-ified type indices.
       auto Loc = Parser.getTok();
       auto Signature = std::make_unique<wasm::WasmSignature>();
       if (parseSignature(Signature.get()))
         return true;
+      // Got signature as block type, don't need more
+      ExpectBlockType = false;
       auto &Ctx = getStreamer().getContext();
       // The "true" here will cause this to be a nameless symbol.
       MCSymbol *Sym = Ctx.createTempSymbol("typeindex", true);
       auto *WasmSym = cast<MCSymbolWasm>(Sym);
       WasmSym->setSignature(Signature.get());
       addSignature(std::move(Signature));
       WasmSym->setType(wasm::WASM_SYMBOL_TYPE_FUNCTION);
       const MCExpr *Expr = MCSymbolRefExpr::create(
           WasmSym, MCSymbolRefExpr::VK_WASM_TYPEINDEX, Ctx);
       Operands.push_back(std::make_unique<WebAssemblyOperand>(
           WebAssemblyOperand::Symbol, Loc.getLoc(), Loc.getEndLoc(),
           WebAssemblyOperand::SymOp{Expr}));
     }
 
     while (Lexer.isNot(AsmToken::EndOfStatement)) {
       auto &Tok = Lexer.getTok();
       switch (Tok.getKind()) {
       case AsmToken::Identifier: {
         if (!parseSpecialFloatMaybe(false, Operands))
           break;
         auto &Id = Lexer.getTok();
         if (ExpectBlockType) {
           // Assume this identifier is a block_type.
           auto BT = parseBlockType(Id.getString());
-          if (BT == WebAssembly::ExprType::Invalid)
+          if (BT == WebAssembly::BlockType::Invalid)
             return error("Unknown block type: ", Id);
           addBlockTypeOperand(Operands, NameLoc, BT);
           Parser.Lex();
         } else {
           // Assume this identifier is a label.
           const MCExpr *Val;
           SMLoc End;
           if (Parser.parseExpression(Val, End))
while (Lexer.isNot(AsmToken::EndOfStatement)) {
       }
       if (Lexer.isNot(AsmToken::EndOfStatement)) {
         if (expect(AsmToken::Comma, ","))
           return true;
       }
     }
     if (ExpectBlockType && Operands.size() == 1) {
       // Support blocks with no operands as default to void.
-      addBlockTypeOperand(Operands, NameLoc, WebAssembly::ExprType::Void);
+      addBlockTypeOperand(Operands, NameLoc, WebAssembly::BlockType::Void);
     }
     Parser.Lex();
     return false;
   }
 
   void onLabelParsed(MCSymbol *Symbol) override {
     LastLabel = Symbol;
     CurrentState = Label;

llvm/lib/Target/WebAssembly/Disassembler/LLVMBuild.txt

 ;   http://llvm.org/docs/LLVMBuild.html
 ;
 ;===------------------------------------------------------------------------===;
 
 [component_0]
 type = Library
 name = WebAssemblyDisassembler
 parent = WebAssembly
-required_libraries = WebAssemblyDesc MCDisassembler WebAssemblyInfo Support
+required_libraries = WebAssemblyDesc MCDisassembler WebAssemblyInfo Support MC
 add_to_library_groups = WebAssembly

llvm/lib/Target/WebAssembly/Disassembler/WebAssemblyDisassembler.cpp

 #include "TargetInfo/WebAssemblyTargetInfo.h"
 #include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCDisassembler/MCDisassembler.h"
 #include "llvm/MC/MCFixedLenDisassembler.h"
 #include "llvm/MC/MCInst.h"
 #include "llvm/MC/MCInstrInfo.h"
 #include "llvm/MC/MCSubtargetInfo.h"
 #include "llvm/MC/MCSymbol.h"
+#include "llvm/MC/MCSymbolWasm.h"
 #include "llvm/Support/Endian.h"
 #include "llvm/Support/LEB128.h"
 #include "llvm/Support/TargetRegistry.h"
 
 using namespace llvm;
 
 #define DEBUG_TYPE "wasm-disassembler"
 
for (uint8_t OPI = 0; OPI < WasmInst->NumOperands; OPI++) {
     }
     // SLEB operands:
     case WebAssembly::OPERAND_I32IMM:
     case WebAssembly::OPERAND_I64IMM: {
       if (!parseLEBImmediate(MI, Size, Bytes, true))
         return MCDisassembler::Fail;
       break;
     }
-    // block_type operands (uint8_t).
+    // block_type operands:
     case WebAssembly::OPERAND_SIGNATURE: {
-      if (!parseImmediate<uint8_t>(MI, Size, Bytes))
+      int64_t Val;
+      uint64_t PrevSize = Size;
+      if (!nextLEB(Val, Bytes, Size, true))
         return MCDisassembler::Fail;
+      if (Val < 0) {
+        // Negative values are single septet value types or empty types
+        if (Size != PrevSize + 1) {
+          MI.addOperand(
+              MCOperand::createImm(int64_t(WebAssembly::BlockType::Invalid)));
+        } else {
+          MI.addOperand(MCOperand::createImm(Val & 0x7f));
+        }
+      } else {
+        // We don't have access to the signature, so create a symbol without one
+        MCSymbol *Sym = getContext().createTempSymbol("typeindex", true);
+        auto *WasmSym = cast<MCSymbolWasm>(Sym);
+        WasmSym->setType(wasm::WASM_SYMBOL_TYPE_FUNCTION);
+        const MCExpr *Expr = MCSymbolRefExpr::create(
+            WasmSym, MCSymbolRefExpr::VK_WASM_TYPEINDEX, getContext());
+        MI.addOperand(MCOperand::createExpr(Expr));
+      }
       break;
     }
     // FP operands.
     case WebAssembly::OPERAND_F32IMM: {
       if (!parseImmediate<float>(MI, Size, Bytes))
         return MCDisassembler::Fail;
       break;
     }

llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyInstPrinter.cpp

void WebAssemblyInstPrinter::printWebAssemblyP2AlignOperand(const MCInst *MI,
   if (Imm == WebAssembly::GetDefaultP2Align(MI->getOpcode()))
     return;
   O << ":p2align=" << Imm;
 }
 
 void WebAssemblyInstPrinter::printWebAssemblySignatureOperand(const MCInst *MI,
                                                               unsigned OpNo,
                                                               raw_ostream &O) {
-  auto Imm = static_cast<unsigned>(MI->getOperand(OpNo).getImm());
+  const MCOperand &Op = MI->getOperand(OpNo);
+  if (Op.isImm()) {
+    auto Imm = static_cast<unsigned>(Op.getImm());
     if (Imm != wasm::WASM_TYPE_NORESULT)
       O << WebAssembly::anyTypeToString(Imm);
+  } else {
+    auto Expr = cast<MCSymbolRefExpr>(Op.getExpr());
+    auto *Sym = cast<MCSymbolWasm>(&Expr->getSymbol());
+    if (Sym->getSignature()) {
+      O << WebAssembly::signatureToString(Sym->getSignature());
+    } else {
+      // Disassembler does not currently produce a signature
+      O << "unknown_type";
+    }
+  }
 }
 
 // We have various enums representing a subset of these types, use this
 // function to convert any of them to text.
 const char *WebAssembly::anyTypeToString(unsigned Ty) {
   switch (Ty) {
   case wasm::WASM_TYPE_I32:
     return "i32";

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

if (MO.isReg()) {
         FixupKind = MCFixupKind(WebAssembly::fixup_sleb128_i32);
         break;
       case WebAssembly::OPERAND_I64IMM:
         FixupKind = MCFixupKind(WebAssembly::fixup_sleb128_i64);
         PaddedSize = 10;
         break;
       case WebAssembly::OPERAND_FUNCTION32:
       case WebAssembly::OPERAND_OFFSET32:
+      case WebAssembly::OPERAND_SIGNATURE:
       case WebAssembly::OPERAND_TYPEINDEX:
       case WebAssembly::OPERAND_GLOBAL:
       case WebAssembly::OPERAND_EVENT:
         FixupKind = MCFixupKind(WebAssembly::fixup_uleb128_i32);
         break;
       default:
         llvm_unreachable("unexpected symbolic operand kind");
       }

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

 // Defines symbolic names for the WebAssembly instructions.
 //
 #define GET_INSTRINFO_ENUM
 #include "WebAssemblyGenInstrInfo.inc"
 
 namespace llvm {
 namespace WebAssembly {
 
-/// This is used to indicate block signatures.
-enum class ExprType : unsigned {
+/// Used as immediate MachineOperands for block signatures
+enum class BlockType : unsigned {
+  Invalid = 0x00,
   Void = 0x40,
-  I32 = 0x7F,
-  I64 = 0x7E,
-  F32 = 0x7D,
-  F64 = 0x7C,
-  V128 = 0x7B,
-  Exnref = 0x68,
-  Invalid = 0x00
+  I32 = unsigned(wasm::ValType::I32),
+  I64 = unsigned(wasm::ValType::I64),
+  F32 = unsigned(wasm::ValType::F32),
+  F64 = unsigned(wasm::ValType::F64),
+  V128 = unsigned(wasm::ValType::V128),
+  Exnref = unsigned(wasm::ValType::EXNREF),
+  // Multivalue blocks (and other non-void blocks) are only emitted when the
+  // blocks will never be exited and are at the ends of functions (see
+  // WebAssemblyCFGStackify::fixEndsAtEndOfFunction). They also are never made
+  // to pop values off the stack, so the exact multivalue signature can always
+  // be inferred from the return type of the parent function in MCInstLower.
+  Multivalue = 0xffff,
 };
 
 /// Instruction opcodes emitted via means other than CodeGen.
 static const unsigned Nop = 0x01;
 static const unsigned End = 0x0b;
 
 wasm::ValType toValType(const MVT &Ty);
 

llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp

#endif
 
   // 'br_on_exn' extracts exnref object and pushes variable number of values
   // depending on its tag. For C++ exception, its a single i32 value, and the
   // generated code will be in the form of:
   // block i32
   //   br_on_exn 0, $__cpp_exception
   //   rethrow
   // end_block
-  WebAssembly::ExprType ReturnType = WebAssembly::ExprType::Void;
+  WebAssembly::BlockType ReturnType = WebAssembly::BlockType::Void;
   if (IsBrOnExn) {
     const char *TagName = BrOnExn->getOperand(1).getSymbolName();
     if (std::strcmp(TagName, "__cpp_exception") != 0)
       llvm_unreachable("Only C++ exception is supported");
-    ReturnType = WebAssembly::ExprType::I32;
+    ReturnType = WebAssembly::BlockType::I32;
   }
 
   auto InsertPos = getLatestInsertPos(Header, BeforeSet, AfterSet);
   MachineInstr *Begin =
       BuildMI(*Header, InsertPos, Header->findDebugLoc(InsertPos),
               TII.get(WebAssembly::BLOCK))
           .addImm(int64_t(ReturnType));
 
else
       AfterSet.insert(&MI);
 #endif
   }
 
   // Mark the beginning of the loop.
   auto InsertPos = getEarliestInsertPos(&MBB, BeforeSet, AfterSet);
   MachineInstr *Begin = BuildMI(MBB, InsertPos, MBB.findDebugLoc(InsertPos),
                                 TII.get(WebAssembly::LOOP))
-                            .addImm(int64_t(WebAssembly::ExprType::Void));
+                            .addImm(int64_t(WebAssembly::BlockType::Void));
 
   // Decide where in Header to put the END_LOOP.
   BeforeSet.clear();
   AfterSet.clear();
 #ifndef NDEBUG
   for (const auto &MI : MBB)
     // Existing END_LOOP markers belong to parent loops of this loop
     if (MI.getOpcode() == WebAssembly::END_LOOP)
else
       break;
   }
 
   // Add the TRY.
   auto InsertPos = getLatestInsertPos(Header, BeforeSet, AfterSet);
   MachineInstr *Begin =
       BuildMI(*Header, InsertPos, Header->findDebugLoc(InsertPos),
               TII.get(WebAssembly::TRY))
-          .addImm(int64_t(WebAssembly::ExprType::Void));
+          .addImm(int64_t(WebAssembly::BlockType::Void));
 
   // Decide where in Header to put the END_TRY.
   BeforeSet.clear();
   AfterSet.clear();
   for (const auto &MI : *Cont) {
 #ifndef NDEBUG
     // END_TRY should precede existing LOOP and BLOCK markers.
     if (MI.getOpcode() == WebAssembly::LOOP ||
for (auto Range : TryRanges) {
           break;
         }
       }
 
       // Create the nested try instruction.
       MachineInstr *NestedTry =
           BuildMI(*MBB, *RangeBegin, RangeBegin->getDebugLoc(),
                   TII.get(WebAssembly::TRY))
-              .addImm(int64_t(WebAssembly::ExprType::Void));
+              .addImm(int64_t(WebAssembly::BlockType::Void));
 
       // Create the nested EH pad and fill instructions in.
       MachineBasicBlock *NestedEHPad = MF.CreateMachineBasicBlock();
       MF.insert(std::next(MBB->getIterator()), NestedEHPad);
       NestedEHPad->setIsEHPad();
       NestedEHPad->setIsEHScopeEntry();
       BuildMI(NestedEHPad, RangeEnd->getDebugLoc(), TII.get(WebAssembly::CATCH),
               ExnReg);

 /// matches the function signature, even though it's unreachable. This function
 /// checks for such cases and fixes up the signatures.
 void WebAssemblyCFGStackify::fixEndsAtEndOfFunction(MachineFunction &MF) {
   const auto &MFI = *MF.getInfo<WebAssemblyFunctionInfo>();
 
   if (MFI.getResults().empty())
     return;
 
-  // TODO: Generalize from value types to function types for multivalue
-  WebAssembly::ExprType RetType;
-  switch (MFI.getResults().front().SimpleTy) {
-  case MVT::i32:
-    RetType = WebAssembly::ExprType::I32;
-    break;
-  case MVT::i64:
-    RetType = WebAssembly::ExprType::I64;
-    break;
-  case MVT::f32:
-    RetType = WebAssembly::ExprType::F32;
-    break;
-  case MVT::f64:
-    RetType = WebAssembly::ExprType::F64;
-    break;
-  case MVT::v16i8:
-  case MVT::v8i16:
-  case MVT::v4i32:
-  case MVT::v2i64:
-  case MVT::v4f32:
-  case MVT::v2f64:
-    RetType = WebAssembly::ExprType::V128;
-    break;
-  case MVT::exnref:
-    RetType = WebAssembly::ExprType::Exnref;
-    break;
-  default:
-    llvm_unreachable("unexpected return type");
-  }
+  // MCInstLower will add the proper types to multivalue signatures based on the
+  // function return type
+  WebAssembly::BlockType RetType =
+      MFI.getResults().size() > 1
+          ? WebAssembly::BlockType::Multivalue
+          : WebAssembly::BlockType(
+                WebAssembly::toValType(MFI.getResults().front()));
 
   for (MachineBasicBlock &MBB : reverse(MF)) {
     for (MachineInstr &MI : reverse(MBB)) {
       if (MI.isPosition() || MI.isDebugInstr())
         continue;
-      if (MI.getOpcode() == WebAssembly::END_BLOCK) {
-        if (MFI.getResults().size() > 1)
-          report_fatal_error("Multivalue block signatures not implemented yet");
-        EndToBegin[&MI]->getOperand(0).setImm(int32_t(RetType));
-        continue;
-      }
-      if (MI.getOpcode() == WebAssembly::END_LOOP) {
-        if (MFI.getResults().size() > 1)
-          report_fatal_error("Multivalue loop signatures not implemented yet");
+      switch (MI.getOpcode()) {
+      case WebAssembly::END_BLOCK:
+      case WebAssembly::END_LOOP:
+      case WebAssembly::END_TRY:
         EndToBegin[&MI]->getOperand(0).setImm(int32_t(RetType));
         continue;
-      }
+      default:
         // Something other than an `end`. We're done.
         return;
       }
     }
   }
+}
 
 // WebAssembly functions end with an end instruction, as if the function body
 // were a block.
 static void appendEndToFunction(MachineFunction &MF,
                                 const WebAssemblyInstrInfo &TII) {
   BuildMI(MF.back(), MF.back().end(),
           MF.back().findPrevDebugLoc(MF.back().end()),
           TII.get(WebAssembly::END_FUNCTION));

llvm/lib/Target/WebAssembly/WebAssemblyMCInstLower.h

 /// This file declares the class to lower WebAssembly MachineInstrs to
 /// their corresponding MCInst records.
 ///
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_LIB_TARGET_WEBASSEMBLY_WEBASSEMBLYMCINSTLOWER_H
 #define LLVM_LIB_TARGET_WEBASSEMBLY_WEBASSEMBLYMCINSTLOWER_H
 
+#include "llvm/BinaryFormat/Wasm.h"
 #include "llvm/MC/MCInst.h"
 #include "llvm/Support/Compiler.h"
 
 namespace llvm {
 class WebAssemblyAsmPrinter;
 class MCContext;
 class MCSymbol;
 class MachineInstr;
 class MachineOperand;
 
 /// This class is used to lower an MachineInstr into an MCInst.
 class LLVM_LIBRARY_VISIBILITY WebAssemblyMCInstLower {
   MCContext &Ctx;
   WebAssemblyAsmPrinter &Printer;
 
   MCSymbol *GetGlobalAddressSymbol(const MachineOperand &MO) const;
   MCSymbol *GetExternalSymbolSymbol(const MachineOperand &MO) const;
   MCOperand lowerSymbolOperand(const MachineOperand &MO, MCSymbol *Sym) const;
+  MCOperand lowerTypeIndexOperand(SmallVector<wasm::ValType, 1> &&,
+                                  SmallVector<wasm::ValType, 4> &&) const;
 
 public:
   WebAssemblyMCInstLower(MCContext &ctx, WebAssemblyAsmPrinter &printer)
       : Ctx(ctx), Printer(printer) {}
   void lower(const MachineInstr *MI, MCInst &OutMI) const;
 };
 } // end namespace llvm
 
 #endif

llvm/lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp

if (MO.getOffset() != 0) {
 
     Expr = MCBinaryExpr::createAdd(
         Expr, MCConstantExpr::create(MO.getOffset(), Ctx), Ctx);
   }
 
   return MCOperand::createExpr(Expr);
 }
 
+MCOperand WebAssemblyMCInstLower::lowerTypeIndexOperand(
+    SmallVector<wasm::ValType, 1> &&Returns,
+    SmallVector<wasm::ValType, 4> &&Params) const {
+  auto Signature = std::make_unique<wasm::WasmSignature>(std::move(Returns),
+                                                         std::move(Params));
+  MCSymbol *Sym = Printer.createTempSymbol("typeindex");
+  auto *WasmSym = cast<MCSymbolWasm>(Sym);
+  WasmSym->setSignature(Signature.get());
+  Printer.addSignature(std::move(Signature));
+  WasmSym->setType(wasm::WASM_SYMBOL_TYPE_FUNCTION);
+  const MCExpr *Expr =
+      MCSymbolRefExpr::create(WasmSym, MCSymbolRefExpr::VK_WASM_TYPEINDEX, Ctx);
+  return MCOperand::createExpr(Expr);
+}
+
 // Return the WebAssembly type associated with the given register class.
 static wasm::ValType getType(const TargetRegisterClass *RC) {
   if (RC == &WebAssembly::I32RegClass)
     return wasm::ValType::I32;
   if (RC == &WebAssembly::I64RegClass)
     return wasm::ValType::I64;
   if (RC == &WebAssembly::F32RegClass)
     return wasm::ValType::F32;
   if (RC == &WebAssembly::F64RegClass)
     return wasm::ValType::F64;
   if (RC == &WebAssembly::V128RegClass)
     return wasm::ValType::V128;
   llvm_unreachable("Unexpected register class");
 }
 
+static void getFunctionReturns(const MachineInstr *MI,
+                               SmallVectorImpl<wasm::ValType> &Returns) {
+  const Function &F = MI->getMF()->getFunction();
+  const TargetMachine &TM = MI->getMF()->getTarget();
+  Type *RetTy = F.getReturnType();
+  SmallVector<MVT, 4> CallerRetTys;
+  computeLegalValueVTs(F, TM, RetTy, CallerRetTys);
+  valTypesFromMVTs(CallerRetTys, Returns);
+}
+
 void WebAssemblyMCInstLower::lower(const MachineInstr *MI,
                                    MCInst &OutMI) const {
   OutMI.setOpcode(MI->getOpcode());
 
   const MCInstrDesc &Desc = MI->getDesc();
   for (unsigned I = 0, E = MI->getNumOperands(); I != E; ++I) {
     const MachineOperand &MO = MI->getOperand(I);
 
case MachineOperand::MO_Register: {
       unsigned WAReg = MFI.getWAReg(MO.getReg());
       MCOp = MCOperand::createReg(WAReg);
       break;
     }
     case MachineOperand::MO_Immediate:
       if (I < Desc.NumOperands) {
         const MCOperandInfo &Info = Desc.OpInfo[I];
         if (Info.OperandType == WebAssembly::OPERAND_TYPEINDEX) {
-          MCSymbol *Sym = Printer.createTempSymbol("typeindex");
-
           SmallVector<wasm::ValType, 4> Returns;
           SmallVector<wasm::ValType, 4> Params;
 
           const MachineRegisterInfo &MRI =
               MI->getParent()->getParent()->getRegInfo();
           for (const MachineOperand &MO : MI->defs())
             Returns.push_back(getType(MRI.getRegClass(MO.getReg())));
           for (const MachineOperand &MO : MI->explicit_uses())
             if (MO.isReg())
               Params.push_back(getType(MRI.getRegClass(MO.getReg())));
 
           // call_indirect instructions have a callee operand at the end which
           // doesn't count as a param.
           if (WebAssembly::isCallIndirect(MI->getOpcode()))
             Params.pop_back();
 
           // return_call_indirect instructions have the return type of the
           // caller
-          if (MI->getOpcode() == WebAssembly::RET_CALL_INDIRECT) {
-            const Function &F = MI->getMF()->getFunction();
-            const TargetMachine &TM = MI->getMF()->getTarget();
-            Type *RetTy = F.getReturnType();
-            SmallVector<MVT, 4> CallerRetTys;
-            computeLegalValueVTs(F, TM, RetTy, CallerRetTys);
-            valTypesFromMVTs(CallerRetTys, Returns);
-          }
+          if (MI->getOpcode() == WebAssembly::RET_CALL_INDIRECT)
+            getFunctionReturns(MI, Returns);
 
-          auto *WasmSym = cast<MCSymbolWasm>(Sym);
-          auto Signature = std::make_unique<wasm::WasmSignature>(std::move(Returns),
-                                                            std::move(Params));
-          WasmSym->setSignature(Signature.get());
-          Printer.addSignature(std::move(Signature));
-          WasmSym->setType(wasm::WASM_SYMBOL_TYPE_FUNCTION);
-
-          const MCExpr *Expr = MCSymbolRefExpr::create(
-              WasmSym, MCSymbolRefExpr::VK_WASM_TYPEINDEX, Ctx);
-          MCOp = MCOperand::createExpr(Expr);
+          MCOp = lowerTypeIndexOperand(std::move(Returns), std::move(Params));
           break;
+        } else if (Info.OperandType == WebAssembly::OPERAND_SIGNATURE) {
+          auto BT = static_cast<WebAssembly::BlockType>(MO.getImm());
+          assert(BT != WebAssembly::BlockType::Invalid);
+          if (BT == WebAssembly::BlockType::Multivalue) {
+            SmallVector<wasm::ValType, 1> Returns;
+            getFunctionReturns(MI, Returns);
+            MCOp = lowerTypeIndexOperand(std::move(Returns),
+                                         SmallVector<wasm::ValType, 4>());
+            break;
+          }
         }
       }
       MCOp = MCOperand::createImm(MO.getImm());
       break;
     case MachineOperand::MO_FPImmediate: {
       // TODO: MC converts all floating point immediate operands to double.
       // This is fine for numeric values, but may cause NaNs to change bits.
       const ConstantFP *Imm = MO.getFPImm();

llvm/test/CodeGen/WebAssembly/multivalue.ll

 ; RUN: llc < %s -asm-verbose=false -verify-machineinstrs -disable-wasm-fallthrough-return-opt -wasm-disable-explicit-locals -wasm-keep-registers -mattr=+multivalue | FileCheck %s
 
-; Test that the multivalue attribute is accepted
-; TODO(tlively): implement multivalue
+; Test that the multivalue returns, function types, and block types
+; work as expected.
 
 target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
 target triple = "wasm32-unknown-unknown"
 
 %pair = type { i32, i32 }
 %packed_pair = type <{ i32, i32 }>
 
 ; CHECK-LABEL: pair_ident:
-; CHECK-NEXT: pair_ident (i32, i32) -> (i32, i32)
+; CHECK-NEXT: .functype pair_ident (i32, i32) -> (i32, i32)
 ; CHECK-NEXT: return $0, $1{{$}}
 define %pair @pair_ident(%pair %p) {
   ret %pair %p
 }
 
 ; CHECK-LABEL: packed_pair_ident:
-; CHECK-NEXT: packed_pair_ident (i32, i32) -> (i32, i32)
-; CHECK-nEXT: return $0, $1{{$}}
+; CHECK-NEXT: .functype packed_pair_ident (i32, i32) -> (i32, i32)
+; CHECK-NEXT: return $0, $1{{$}}
 define %packed_pair @packed_pair_ident(%packed_pair %p) {
   ret %packed_pair %p
 }
 
+; CHECK-LABEL: minimal_loop:
+; CHECK-NEXT: .functype minimal_loop (i32) -> (i32, i64)
+; CHECK-NEXT: .LBB{{[0-9]+}}_1:
+; CHECK-NEXT: loop () -> (i32, i64)
+; CHECK-NEXT: br 0{{$}}
+; CHECK-NEXT: .LBB{{[0-9]+}}_2:
+; CHECK-NEXT: end_loop{{$}}
+define {i32, i64} @minimal_loop(i32* %p) {
+entry:
+  br label %loop
+loop:
+  br label %loop
+}
+
 ; CHECK-LABEL: .section .custom_section.target_features
 ; CHECK-NEXT: .int8 1
 ; CHECK-NEXT: .int8 43
 ; CHECK-NEXT: .int8 10
 ; CHECK-NEXT: .ascii "multivalue"

llvm/test/MC/Disassembler/WebAssembly/wasm-error.txt

 # RUN: llvm-mc --disassemble %s -triple=wasm32-unknown-unknown | FileCheck %s
 
 # CHECK: .text
 
-# CHECK: block invalid_type
+# CHECK: block unknown_type
 0x02 0x00
 
+# CHECK: block invalid_type
+0x02 0xff 0x42
+
 # CHECK: br 16 # Invalid depth argument!
 0x0C 0x10

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

.LBB0_2:
     end_loop
     end_block            # label0:
     local.get   4
     local.get   5
     block       void
     block       i64
     block       f32
     block       f64
+    block       () -> (i32, i32)
+    i32.const   1
+    i32.const   2
+    end_block
+    drop
+    drop
     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.

 # CHECK-NEXT:      end_loop
 # CHECK-NEXT:      end_block           # label0:
 # CHECK-NEXT:      local.get   4
 # CHECK-NEXT:      local.get   5
 # CHECK-NEXT:      block
 # CHECK-NEXT:      block       i64
 # CHECK-NEXT:      block       f32
 # CHECK-NEXT:      block       f64
+# CHECK-NEXT:      block       () -> (i32, i32)
+# CHECK-NEXT:      i32.const   1
+# CHECK-NEXT:      i32.const   2
+# CHECK-NEXT:      end_block
+# CHECK-NEXT:      drop
+# CHECK-NEXT:      drop
 # 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

llvm/tools/llvm-mc/Disassembler.h

 #include <string>
 
 namespace llvm {
 
 class MemoryBuffer;
 class Target;
 class raw_ostream;
 class SourceMgr;
+class MCContext;
 class MCSubtargetInfo;
 class MCStreamer;
 
 class Disassembler {
 public:
-  static int disassemble(const Target &T,
-                         const std::string &Triple,
-                         MCSubtargetInfo &STI,
-                         MCStreamer &Streamer,
-                         MemoryBuffer &Buffer,
-                         SourceMgr &SM,
+  static int disassemble(const Target &T, const std::string &Triple,
+                         MCSubtargetInfo &STI, MCStreamer &Streamer,
+                         MemoryBuffer &Buffer, SourceMgr &SM, MCContext &Ctx,
                          raw_ostream &Out);
 };
 
 } // namespace llvm
 
 #endif

llvm/tools/llvm-mc/Disassembler.cpp

 //===----------------------------------------------------------------------===//
 
 #include "Disassembler.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/MC/MCAsmInfo.h"
 #include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCDisassembler/MCDisassembler.h"
 #include "llvm/MC/MCInst.h"
+#include "llvm/MC/MCObjectFileInfo.h"
 #include "llvm/MC/MCRegisterInfo.h"
 #include "llvm/MC/MCStreamer.h"
 #include "llvm/MC/MCSubtargetInfo.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/raw_ostream.h"
 
while (SkipToToken(Str)) {
     ByteArray.first.push_back(ByteVal);
     ByteArray.second.push_back(Value.data());
     Str = Str.substr(Next);
   }
 
   return false;
 }
 
-int Disassembler::disassemble(const Target &T,
-                              const std::string &Triple,
-                              MCSubtargetInfo &STI,
-                              MCStreamer &Streamer,
-                              MemoryBuffer &Buffer,
-                              SourceMgr &SM,
-                              raw_ostream &Out) {
+int Disassembler::disassemble(const Target &T, const std::string &Triple,
+                              MCSubtargetInfo &STI, MCStreamer &Streamer,
+                              MemoryBuffer &Buffer, SourceMgr &SM,
+                              MCContext &Ctx, raw_ostream &Out) {
 
   std::unique_ptr<const MCRegisterInfo> MRI(T.createMCRegInfo(Triple));
   if (!MRI) {
     errs() << "error: no register info for target " << Triple << "\n";
     return -1;
   }
 
   std::unique_ptr<const MCAsmInfo> MAI(T.createMCAsmInfo(*MRI, Triple));
   if (!MAI) {
     errs() << "error: no assembly info for target " << Triple << "\n";
     return -1;
   }
 
-  // Set up the MCContext for creating symbols and MCExpr's.
-  MCContext Ctx(MAI.get(), MRI.get(), nullptr);
-
   std::unique_ptr<const MCDisassembler> DisAsm(
     T.createMCDisassembler(STI, Ctx));
   if (!DisAsm) {
     errs() << "error: no disassembler for target " << Triple << "\n";
     return -1;
   }
 
   // Set up initial section manually here

llvm/tools/llvm-mc/llvm-mc.cpp

case AC_MDisassemble:
     IP->setUseMarkup(true);
     disassemble = true;
     break;
   case AC_Disassemble:
     disassemble = true;
     break;
   }
   if (disassemble)
-    Res = Disassembler::disassemble(*TheTarget, TripleName, *STI, *Str,
-                                    *Buffer, SrcMgr, Out->os());
+    Res = Disassembler::disassemble(*TheTarget, TripleName, *STI, *Str, *Buffer,
+                                    SrcMgr, Ctx, Out->os());
 
   // Keep output if no errors.
   if (Res == 0) {
     Out->keep();
     if (DwoOut)
       DwoOut->keep();
   }
   return Res;
 }