[WebAssembly] Assembler/InstPrinter: support call_indirect type index.

[aardappel]

Summary

A TYPE_INDEX operand (as used by call_indirect) used to be represented
by the InstPrinter as a symbol (e.g. .Ltype_index0@TYPE_INDEX) which
was a bit of a mismatch with the WasmObjectWriter which expects an
unnamed symbol, to receive the signature from and then turn into a
reloc.

There was really no good way to round-trip this information. An earlier
version of this patch tried to attach the signature information using
a .functype, but that ran into trouble when the symbol was re-emitted
without a name. Removing the name was a giant hack also.

The current version changes the assembly syntax to have an inline
signature spec for TYPEINDEX operands that is always unnamed, which
is much more elegant both in syntax and in implementation (as now the
assembler is able to follow the same path as the regular backend)

[aheejin]

Why should type_index symbols be unnamed, i.e., why does WasmObjectWriter try to ensure that they are unnamed? Can we possibly remove that constraints?

[sbc100]

I think the problem here is that there is no such thing as a first class "type" symbol, there are only functions symbols that can be referenced with @TYPE_INDEX to find where they live in the type index space.

Do we need a first class "type" symbol? Perhaps we do? Also can call_indirect take a signature literal in the asm syntax. i.e. do we want to allow all of the following:

call_indirect some_function@TYPEINDEX
call_indirect some_type
call_indirect (i32) -> (i32)

Right now it seems like we are only supporting the first one, and this change basically declared a face function symbol to hang that type off of.

It seems clear that we *do* want to support populating the type section and referencing those types without declaring any functions at all. You can imagine a module that does all kind of call_indirect on an imported table without actually declaring any functions at all. So I think I'm arguing that we a first calls type symbol as well as the some_function@TYPEINDEX syntax.

lib/Target/WebAssembly/MCTargetDesc/WebAssemblyInstPrinter.cpp

58 ↗

(On Diff #209900)

This seems like this should be unnecessary.

The only types of symbols that can be referenced with VK_WASM_TYPEINDEX are function symbols. i.e. should be able to assert here that SRE->getSymbol is a function symbo.l. And functions symbols should already if their type specified in the assembly when they are declared.

lib/Target/WebAssembly/MCTargetDesc/WebAssemblyInstPrinter.h

62 ↗

(On Diff #209900)

Seems a little odd to have these two "ToString" functions return std::string but the two above return C strings.

[aardappel]

Reworked to use inline signatures.

[aardappel]

@sbc100 @aheejin and others: code has been reworked and is much more elegant.. no more hacks.

lib/Target/WebAssembly/MCTargetDesc/WebAssemblyInstPrinter.cpp

58 ↗

(On Diff #209900)

I'm not sure what you mean, since not all function symbols are a TYPEINDEX. Either way, this code has been reworked.

lib/Target/WebAssembly/MCTargetDesc/WebAssemblyInstPrinter.h

62 ↗

(On Diff #209900)

The const char * ones are because, well, the strings are compile-time constant, and the std::string ones since they construct a string. I wouldn't want to change the former to std::string also for consistency, as this forces an allocation on the caller they may not need.

[tlively]

This looks great! The code is straightforward and I like the extra information in the text format. We will have to do a little more work to support return_call_indirect, but it looks like doing so should be fairly straightforward on top of this base.

lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp

495 ↗

(On Diff #212028)

We should add return_call_indirect support as well, but probably in a separate CL.

[tlively]

lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp

495 ↗

(On Diff #212028)

Actually, there should be no different work to do for return_call_indirect because the text format should contain the correct return type. So it would be good to add that case to this condition now.

[aardappel]

Added return_call_indirect

[aardappel]

Fixed tail-call-encodings.s test

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

if (Name == "block") {
         return true;
     } else if (Name == "end_block") {
       if (pop(Name, Block))
         return true;
     } else if (Name == "end_function") {
       CurrentState = EndFunction;
       if (pop(Name, Function) || ensureEmptyNestingStack())
         return true;
+    } else if (Name == "call_indirect" || Name == "return_call_indirect") {
+      // 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 = make_unique<wasm::WasmSignature>();
+      if (parseSignature(Signature.get()))
+        return true;
+      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(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;

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

public:
   static const char *getRegisterName(unsigned RegNo);
 };
 
 namespace WebAssembly {
 
 const char *typeToString(wasm::ValType Ty);
 const char *anyTypeToString(unsigned Ty);
 
+std::string typeListToString(ArrayRef<wasm::ValType> List);
+std::string signatureToString(const wasm::WasmSignature *Sig);
+
 } // end namespace WebAssembly
 
 } // end namespace llvm
 
 #endif

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

 /// Print MCInst instructions to wasm format.
 ///
 //===----------------------------------------------------------------------===//
 
 #include "MCTargetDesc/WebAssemblyInstPrinter.h"
 #include "MCTargetDesc/WebAssemblyMCTargetDesc.h"
 #include "WebAssembly.h"
 #include "WebAssemblyMachineFunctionInfo.h"
+#include "WebAssemblyUtilities.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/CodeGen/TargetRegisterInfo.h"
 #include "llvm/MC/MCExpr.h"
 #include "llvm/MC/MCInst.h"
 #include "llvm/MC/MCInstrInfo.h"
 #include "llvm/MC/MCSubtargetInfo.h"
 #include "llvm/MC/MCSymbol.h"
if (Info.OperandType == WebAssembly::OPERAND_F32IMM) {
       // 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(Op.isExpr() && "unknown operand kind in printOperand");
+    // call_indirect instructions have a TYPEINDEX operand that we print
+    // as a signature here, such that the assembler can recover this
+    // information.
+    auto SRE = static_cast<const MCSymbolRefExpr *>(Op.getExpr());
+    if (SRE->getKind() == MCSymbolRefExpr::VK_WASM_TYPEINDEX) {
+      auto &Sym = static_cast<const MCSymbolWasm &>(SRE->getSymbol());
+      O << WebAssembly::signatureToString(Sym.getSignature());
+    } else {
       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();
void WebAssemblyInstPrinter::printWebAssemblySignatureOperand(const MCInst *MI,
                                                               raw_ostream &O) {
   auto Imm = static_cast<unsigned>(MI->getOperand(OpNo).getImm());
   if (Imm != wasm::WASM_TYPE_NORESULT)
     O << WebAssembly::anyTypeToString(Imm);
 }
 
 // We have various enums representing a subset of these types, use this
 // function to convert any of them to text.
-const char *llvm::WebAssembly::anyTypeToString(unsigned Ty) {
+const char *WebAssembly::anyTypeToString(unsigned Ty) {
   switch (Ty) {
   case wasm::WASM_TYPE_I32:
     return "i32";
   case wasm::WASM_TYPE_I64:
     return "i64";
   case wasm::WASM_TYPE_F32:
     return "f32";
   case wasm::WASM_TYPE_F64:
     return "f64";
   case wasm::WASM_TYPE_V128:
     return "v128";
   case wasm::WASM_TYPE_FUNCREF:
     return "funcref";
   case wasm::WASM_TYPE_FUNC:
     return "func";
   case wasm::WASM_TYPE_EXNREF:
     return "exnref";
   case wasm::WASM_TYPE_NORESULT:
     return "void";
   default:
     return "invalid_type";
   }
 }
 
-const char *llvm::WebAssembly::typeToString(wasm::ValType Ty) {
+const char *WebAssembly::typeToString(wasm::ValType Ty) {
   return anyTypeToString(static_cast<unsigned>(Ty));
 }
+
+std::string WebAssembly::typeListToString(ArrayRef<wasm::ValType> List) {
+  std::string S;
+  for (auto &Ty : List) {
+    if (&Ty != &List[0]) S += ", ";
+    S += WebAssembly::typeToString(Ty);
+  }
+  return S;
+}
+
+std::string WebAssembly::signatureToString(const wasm::WasmSignature *Sig) {
+  std::string S("(");
+  S += typeListToString(Sig->Params);
+  S += ") -> (";
+  S += typeListToString(Sig->Returns);
+  S += ")";
+  return S;
+}

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

 
 protected:
   void emitValueType(wasm::ValType Type);
 };
 
 /// This part is for ascii assembly output
 class WebAssemblyTargetAsmStreamer final : public WebAssemblyTargetStreamer {
   formatted_raw_ostream &OS;
-  void emitSignature(const wasm::WasmSignature *Sig);
-  void emitParamList(const wasm::WasmSignature *Sig);
-  void emitReturnList(const wasm::WasmSignature *Sig);
 
 public:
   WebAssemblyTargetAsmStreamer(MCStreamer &S, formatted_raw_ostream &OS);
 
   void emitLocal(ArrayRef<wasm::ValType> Types) override;
   void emitEndFunc() override;
   void emitFunctionType(const MCSymbolWasm *Sym) override;
   void emitIndIdx(const MCExpr *Value) override;

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

void WebAssemblyTargetAsmStreamer::emitLocal(ArrayRef<wasm::ValType> Types) {
   if (!Types.empty()) {
     OS << "\t.local  \t";
     printTypes(OS, Types);
   }
 }
 
 void WebAssemblyTargetAsmStreamer::emitEndFunc() { OS << "\t.endfunc\n"; }
 
-void WebAssemblyTargetAsmStreamer::emitSignature(
-    const wasm::WasmSignature *Sig) {
-  OS << "(";
-  emitParamList(Sig);
-  OS << ") -> (";
-  emitReturnList(Sig);
-  OS << ")";
-}
-
-void WebAssemblyTargetAsmStreamer::emitParamList(
-    const wasm::WasmSignature *Sig) {
-  auto &Params = Sig->Params;
-  for (auto &Ty : Params) {
-    if (&Ty != &Params[0])
-      OS << ", ";
-    OS << WebAssembly::typeToString(Ty);
-  }
-}
-
-void WebAssemblyTargetAsmStreamer::emitReturnList(
-    const wasm::WasmSignature *Sig) {
-  auto &Returns = Sig->Returns;
-  for (auto &Ty : Returns) {
-    if (&Ty != &Returns[0])
-      OS << ", ";
-    OS << WebAssembly::typeToString(Ty);
-  }
-}
-
 void WebAssemblyTargetAsmStreamer::emitFunctionType(const MCSymbolWasm *Sym) {
   assert(Sym->isFunction());
   OS << "\t.functype\t" << Sym->getName() << " ";
-  emitSignature(Sym->getSignature());
+  OS << WebAssembly::signatureToString(Sym->getSignature());
   OS << "\n";
 }
 
 void WebAssemblyTargetAsmStreamer::emitGlobalType(const MCSymbolWasm *Sym) {
   assert(Sym->isGlobal());
   OS << "\t.globaltype\t" << Sym->getName() << ", "
      << WebAssembly::typeToString(
             static_cast<wasm::ValType>(Sym->getGlobalType().Type))
      << '\n';
 }
 
 void WebAssemblyTargetAsmStreamer::emitEventType(const MCSymbolWasm *Sym) {
   assert(Sym->isEvent());
   OS << "\t.eventtype\t" << Sym->getName() << " ";
-  emitParamList(Sym->getSignature());
+  OS << WebAssembly::typeListToString(Sym->getSignature()->Params);
   OS << "\n";
 }
 
 void WebAssemblyTargetAsmStreamer::emitImportModule(const MCSymbolWasm *Sym,
                                                     StringRef ImportModule) {
   OS << "\t.import_module\t" << Sym->getName() << ", "
                              << ImportModule << '\n';
 }

llvm/trunk/test/CodeGen/WebAssembly/reg-stackify.ll

 ; CHECK-NEXT: i32.load  $push[[L0:.+]]=, 0($0)
 ; CHECK-NEXT: i32.load  $push[[L1:.+]]=, 0($pop[[L0]])
 ; CHECK-NEXT: i32.call_indirect $push{{.+}}=, $pop[[L3]], $1, $pop[[L1]]
 ; NOREGS-LABEL: call_indirect_stackify:
 ; NOREGS: i32.load  0
 ; NOREGS-NEXT: local.tee 0
 ; NOREGS:      i32.load  0
 ; NOREGS-NEXT: i32.load  0
-; NOREGS-NEXT: i32.call_indirect
+; NOREGS-NEXT: i32.call_indirect (i32, i32) -> (i32)
 %class.call_indirect = type { i32 (...)** }
 define i32 @call_indirect_stackify(%class.call_indirect** %objptr, i32 %arg) {
   %obj = load %class.call_indirect*, %class.call_indirect** %objptr
   %addr = bitcast %class.call_indirect* %obj to i32(%class.call_indirect*, i32)***
   %vtable = load i32(%class.call_indirect*, i32)**, i32(%class.call_indirect*, i32)*** %addr
   %vfn = getelementptr inbounds i32(%class.call_indirect*, i32)*, i32(%class.call_indirect*, i32)** %vtable, i32 0
   %f = load i32(%class.call_indirect*, i32)*, i32(%class.call_indirect*, i32)** %vfn
   %ret = call i32 %f(%class.call_indirect* %obj, i32 %arg)

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

test0:
     i32.ge_s
     br_if       0        # 0: down to label0
 .LBB0_1:
     loop        i32      # label1:
     call        something1
     i64.const   1234
     i32.call    something2
     i32.const   0
-    call_indirect 0
+    call_indirect (i32, f64) -> ()
     i32.const   1
     i32.add
     local.tee   0
     local.get   0
     i32.lt_s
     br_if       0        # 0: up to label1
 .LBB0_2:
     end_loop

 # CHECK-NEXT:      i32.ge_s
 # CHECK-NEXT:      br_if       0       # 0: down to label0
 # CHECK-NEXT:  .LBB0_1:
 # CHECK-NEXT:      loop        i32     # label1:
 # CHECK-NEXT:      call        something1
 # CHECK-NEXT:      i64.const   1234
 # CHECK-NEXT:      i32.call    something2
 # CHECK-NEXT:      i32.const   0
-# CHECK-NEXT:      call_indirect 0
+# CHECK-NEXT:      call_indirect (i32, f64) -> ()
 # CHECK-NEXT:      i32.const   1
 # CHECK-NEXT:      i32.add
 # CHECK-NEXT:      local.tee   0
 # CHECK-NEXT:      local.get   0
 # CHECK-NEXT:      i32.lt_s
 # CHECK-NEXT:      br_if       0       # 0: up to label1
 # CHECK-NEXT:  .LBB0_2:
 # CHECK-NEXT:      end_loop

llvm/trunk/test/MC/WebAssembly/tail-call-encodings.s

foo1:
     # CHECK-NEXT: fixup A - offset: 1, value: bar1, kind: fixup_uleb128_i32
     return_call bar1
 
     end_function
 
 foo2:
     .functype foo2 () -> ()
 
-    # CHECK: return_call_indirect 0 # encoding: [0x13,0x00,0x00]
-    return_call_indirect 0
+    # CHECK: return_call_indirect (i32) -> (i32) # encoding: [0x13,
+    # CHECK-NEXT: fixup A - offset: 1, value: .Ltypeindex0@TYPEINDEX, kind: fixup_uleb128_i32
+    return_call_indirect (i32) -> (i32)
 
     end_function

llvm/trunk/test/MC/WebAssembly/type-index.s

+# RUN: llvm-mc -triple=wasm32-unknown-unknown -mattr=+unimplemented-simd128,+nontrapping-fptoint,+exception-handling < %s | FileCheck %s
+# Check that it converts to .o without errors:
+# RUN: llvm-mc -triple=wasm32-unknown-unknown -filetype=obj -mattr=+unimplemented-simd128,+nontrapping-fptoint,+exception-handling < %s | obj2yaml | FileCheck -check-prefix=BIN %s
+
+# Minimal test for type indices in call_indirect.
+
+test0:
+    .functype   test0 (i32) -> (i32)
+    call_indirect (f64) -> (f64)
+    end_function
+
+# CHECK:  .text
+# CHECK-LABEL: test0:
+# CHECK-NEXT: .functype test0 (i32) -> (i32)
+# CHECK-NEXT: call_indirect (f64) -> (f64)
+# CHECK-NEXT: end_function
+
+# BIN:      --- !WASM
+# BIN-NEXT: FileHeader:
+# BIN-NEXT:   Version:         0x00000001
+# BIN-NEXT: Sections:
+# BIN-NEXT:   - Type:            TYPE
+# BIN-NEXT:     Signatures:
+# BIN-NEXT:       - Index:           0
+# BIN-NEXT:         ReturnType:      I32
+# BIN-NEXT:         ParamTypes:
+# BIN-NEXT:           - I32
+# BIN-NEXT:       - Index:           1
+# BIN-NEXT:         ReturnType:      F64
+# BIN-NEXT:         ParamTypes:
+# BIN-NEXT:           - F64
+# BIN-NEXT:   - Type:            IMPORT
+# BIN-NEXT:     Imports:
+# BIN-NEXT:       - Module:          env
+# BIN-NEXT:         Field:           __linear_memory
+# BIN-NEXT:         Kind:            MEMORY
+# BIN-NEXT:         Memory:
+# BIN-NEXT:           Initial:         0x00000000
+# BIN-NEXT:       - Module:          env
+# BIN-NEXT:         Field:           __indirect_function_table
+# BIN-NEXT:         Kind:            TABLE
+# BIN-NEXT:         Table:
+# BIN-NEXT:           ElemType:        FUNCREF
+# BIN-NEXT:           Limits:
+# BIN-NEXT:             Initial:         0x00000000
+# BIN-NEXT:   - Type:            FUNCTION
+# BIN-NEXT:     FunctionTypes:   [ 0 ]
+# BIN-NEXT:   - Type:            CODE
+# BIN-NEXT:     Relocations:
+# BIN-NEXT:       - Type:            R_WASM_TYPE_INDEX_LEB
+# BIN-NEXT:         Index:           1
+# BIN-NEXT:         Offset:          0x00000004
+# BIN-NEXT:     Functions:
+# BIN-NEXT:       - Index:           0
+# BIN-NEXT:         Locals:          []
+# BIN-NEXT:         Body:            118180808000000B
+# BIN-NEXT:   - Type:            CUSTOM
+# BIN-NEXT:     Name:            linking
+# BIN-NEXT:     Version:         2
+# BIN-NEXT:     SymbolTable:
+# BIN-NEXT:       - Index:           0
+# BIN-NEXT:         Kind:            FUNCTION
+# BIN-NEXT:         Name:            test0
+# BIN-NEXT:         Flags:           [ BINDING_LOCAL ]
+# BIN-NEXT:         Function:        0
+# BIN-NEXT: ...
+