[WebAssembly] Swap operand order of call_indirect in text format

[wingo]

Summary

The WebAssembly text and binary formats have different operand orders
for the "type" and "table" fields of call_indirect (and
return_call_indirect). In LLVM we use the binary order for the MCInstr,
but when we produce or consume the text format we should use the text
order. For compilation units targetting WebAssembly 1.0 (without the
reference types feature), we omit the table operand entirely.

[wingo]

Related to https://reviews.llvm.org/D90948#2572528. Note that this patch parses call_indirect () -> () the same as call_indirect __indirect_function_table, () -> () when reference-types is enabled, i.e. supplying the "missing" operand. Advantage is that you can compile the same input with or without reference types. Disadvantage is that it's slightly magical. Feedback welcome.

[tlively]

LGTM with the current behavior of implicitly inserting the __indirect_function_table operand. Thanks, @wingo!

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

#include "WebAssemblyGenAsmMatcher.inc"
 
   void addBlockTypeOperand(OperandVector &Operands, SMLoc NameLoc,
                            WebAssembly::BlockType BT) {
     Operands.push_back(std::make_unique<WebAssemblyOperand>(
         WebAssemblyOperand::Integer, NameLoc, NameLoc,
         WebAssemblyOperand::IntOp{static_cast<int64_t>(BT)}));
   }
 
-  bool addFunctionTableOperand(OperandVector &Operands, MCSymbolWasm *Sym,
-                               SMLoc StartLoc, SMLoc EndLoc) {
+  bool parseFunctionTableOperand(std::unique_ptr<WebAssemblyOperand> *Op) {
+    if (STI->checkFeatures("+reference-types")) {
+      // If the reference-types feature is enabled, there is an explicit table
+      // operand.  To allow the same assembly to be compiled with or without
+      // reference types, we allow the operand to be omitted, in which case we
+      // default to __indirect_function_table.
+      auto &Tok = Lexer.getTok();
+      if (Tok.is(AsmToken::Identifier)) {
+        auto *Sym =
+            GetOrCreateFunctionTableSymbol(getContext(), Tok.getString());
         const auto *Val = MCSymbolRefExpr::create(Sym, getContext());
-    Operands.push_back(std::make_unique<WebAssemblyOperand>(
-        WebAssemblyOperand::Symbol, StartLoc, EndLoc,
-        WebAssemblyOperand::SymOp{Val}));
+        *Op = std::make_unique<WebAssemblyOperand>(
+            WebAssemblyOperand::Symbol, Tok.getLoc(), Tok.getEndLoc(),
+            WebAssemblyOperand::SymOp{Val});
+        Parser.Lex();
+        return expect(AsmToken::Comma, ",");
+      } else {
+        const auto *Val =
+            MCSymbolRefExpr::create(DefaultFunctionTable, getContext());
+        *Op = std::make_unique<WebAssemblyOperand>(
+            WebAssemblyOperand::Symbol, SMLoc(), SMLoc(),
+            WebAssemblyOperand::SymOp{Val});
         return false;
       }
-
-  bool addFunctionTableOperand(OperandVector &Operands, StringRef TableName,
-                               SMLoc StartLoc, SMLoc EndLoc) {
-    return addFunctionTableOperand(
-        Operands, GetOrCreateFunctionTableSymbol(getContext(), TableName),
-        StartLoc, EndLoc);
-  }
-
-  bool addDefaultFunctionTableOperand(OperandVector &Operands, SMLoc StartLoc,
-                                      SMLoc EndLoc) {
-    if (STI->checkFeatures("+reference-types")) {
-      return addFunctionTableOperand(Operands, DefaultFunctionTable, StartLoc,
-                                     EndLoc);
     } else {
       // For the MVP there is at most one table whose number is 0, but we can't
       // write a table symbol or issue relocations.  Instead we just ensure the
       // table is live and write a zero.
       getStreamer().emitSymbolAttribute(DefaultFunctionTable, MCSA_NoDeadStrip);
-      Operands.push_back(std::make_unique<WebAssemblyOperand>(
-          WebAssemblyOperand::Integer, StartLoc, EndLoc,
-          WebAssemblyOperand::IntOp{0}));
+      *Op = std::make_unique<WebAssemblyOperand>(WebAssemblyOperand::Integer,
+                                                 SMLoc(), SMLoc(),
+                                                 WebAssemblyOperand::IntOp{0});
       return false;
     }
   }
 
   bool ParseInstruction(ParseInstructionInfo & /*Info*/, StringRef Name,
                         SMLoc NameLoc, OperandVector &Operands) override {
     // Note: Name does NOT point into the sourcecode, but to a local, so
     // use NameLoc instead.
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;
     bool ExpectHeapType = false;
-    bool ExpectFunctionTable = false;
+    std::unique_ptr<WebAssemblyOperand> FunctionTable;
     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") {
+      // These instructions have differing operand orders in the text format vs
+      // the binary formats.  The MC instructions follow the binary format, so
+      // here we stash away the operand and append it later.
+      if (parseFunctionTableOperand(&FunctionTable))
+        return true;
       ExpectFuncType = true;
-      ExpectFunctionTable = true;
     } else if (Name == "ref.null") {
       ExpectHeapType = 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
if (ExpectFuncType || (ExpectBlockType && Lexer.is(AsmToken::LParen))) {
       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}));
-
-      // Allow additional operands after the signature, notably for
-      // call_indirect against a named table.
-      if (Lexer.isNot(AsmToken::EndOfStatement)) {
-        if (expect(AsmToken::Comma, ","))
-          return true;
-        if (Lexer.is(AsmToken::EndOfStatement)) {
-          return error("Unexpected trailing comma");
-        }
-      }
     }
 
     while (Lexer.isNot(AsmToken::EndOfStatement)) {
       auto &Tok = Lexer.getTok();
       switch (Tok.getKind()) {
       case AsmToken::Identifier: {
         if (!parseSpecialFloatMaybe(false, Operands))
           break;
while (Lexer.isNot(AsmToken::EndOfStatement)) {
           auto HeapType = parseHeapType(Id.getString());
           if (HeapType == WebAssembly::HeapType::Invalid) {
             return error("Expected a heap type: ", Id);
           }
           Operands.push_back(std::make_unique<WebAssemblyOperand>(
               WebAssemblyOperand::Integer, Id.getLoc(), Id.getEndLoc(),
               WebAssemblyOperand::IntOp{static_cast<int64_t>(HeapType)}));
           Parser.Lex();
-        } else if (ExpectFunctionTable) {
-          if (addFunctionTableOperand(Operands, Id.getString(), Id.getLoc(),
-                                      Id.getEndLoc()))
-            return true;
-          Parser.Lex();
         } else {
           // Assume this identifier is a label.
           const MCExpr *Val;
           SMLoc End;
           if (Parser.parseExpression(Val, End))
             return error("Cannot parse symbol: ", Lexer.getTok());
           Operands.push_back(std::make_unique<WebAssemblyOperand>(
               WebAssemblyOperand::Symbol, Id.getLoc(), Id.getEndLoc(),
while (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::BlockType::Void);
     }
-    if (ExpectFunctionTable && Operands.size() == 2) {
-      // If call_indirect doesn't specify a target table, supply one.
-      if (addDefaultFunctionTableOperand(Operands, NameLoc,
-                                         SMLoc::getFromPointer(Name.end())))
-        return true;
-    }
+    if (FunctionTable)
+      Operands.push_back(std::move(FunctionTable));
     Parser.Lex();
     return false;
   }
 
   bool parseSignature(wasm::WasmSignature *Signature) {
     if (expect(AsmToken::LParen, "("))
       return true;
     if (parseRegTypeList(Signature->Params))

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

void WebAssemblyInstPrinter::printRegName(raw_ostream &OS,
   // Note that there's an implicit local.get/local.set here!
   OS << "$" << RegNo;
 }
 
 void WebAssemblyInstPrinter::printInst(const MCInst *MI, uint64_t Address,
                                        StringRef Annot,
                                        const MCSubtargetInfo &STI,
                                        raw_ostream &OS) {
+  switch (MI->getOpcode()) {
+  case WebAssembly::CALL_INDIRECT_S:
+  case WebAssembly::RET_CALL_INDIRECT_S: {
+    // A special case for call_indirect (and ret_call_indirect), if the table
+    // operand is a symbol: the order of the type and table operands is inverted
+    // in the text format relative to the binary format.  Otherwise if table the
+    // operand isn't a symbol, then we have an MVP compilation unit, and the
+    // table shouldn't appear in the output.
+    OS << "\t";
+    OS << getMnemonic(MI).first;
+    OS << " ";
+
+    assert(MI->getNumOperands() == 2);
+    const unsigned TypeOperand = 0;
+    const unsigned TableOperand = 1;
+    if (MI->getOperand(TableOperand).isExpr()) {
+      printOperand(MI, TableOperand, OS);
+      OS << ", ";
+    } else {
+      assert(MI->getOperand(TableOperand).getImm() == 0);
+    }
+    printOperand(MI, TypeOperand, OS);
+    break;
+  }
+  default:
     // Print the instruction (this uses the AsmStrings from the .td files).
     printInstruction(MI, Address, OS);
+    break;
+  }
 
   // Print any additional variadic operands.
   const MCInstrDesc &Desc = MII.get(MI->getOpcode());
   if (Desc.isVariadic()) {
     if ((Desc.getNumOperands() == 0 && MI->getNumOperands() > 0) ||
         Desc.variadicOpsAreDefs())
       OS << "\t";
     unsigned Start = Desc.getNumOperands();

llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp

bool WebAssemblyFastISel::selectCall(const Instruction *I) {
   auto MIB = BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, TII.get(Opc));
 
   if (!IsVoid)
     MIB.addReg(ResultReg, RegState::Define);
 
   if (IsDirect) {
     MIB.addGlobalAddress(Func);
   } else {
-    // Placehoder for the type index.
+    // Placeholder for the type index.
     MIB.addImm(0);
     // The table into which this call_indirect indexes.
     MCSymbolWasm *Table = WebAssembly::getOrCreateFunctionTableSymbol(
         MF->getMMI().getContext(), Subtarget);
     if (Subtarget->hasReferenceTypes()) {
       MIB.addSym(Table);
     } else {
       // Otherwise for the MVP there is at most one table whose number is 0, but

llvm/test/CodeGen/WebAssembly/function-pointer64.ll

 ; For simplicity (and compatibility with UB C/C++ code) we keep all types
 ; of pointers the same size, so function pointers (which are 32-bit indices
 ; in Wasm) are represented as 64-bit until called.
 
 ; CHECK:      .functype foo (i64) -> ()
 ; CHECK-NEXT: i32.const 1
 ; CHECK-NEXT: local.get 0
 ; CHECK-NEXT: i32.wrap_i64
-; CHECK-NEXT: call_indirect (i32) -> (), 0
-; REF:        call_indirect (i32) -> (), __indirect_function_table
+; CHECK-NEXT: call_indirect (i32) -> ()
+; REF:        call_indirect __indirect_function_table, (i32) -> ()
 
 ; CHECK:      .functype test () -> ()
 ; CHECK-NEXT: i64.const bar
 ; CHECK-NEXT: call foo
 
 
 ; Check we're emitting a 64-bit relocs for the call_indirect, the
 ; `i64.const bar` reference in code, and the global.

llvm/test/CodeGen/WebAssembly/multivalue.ll

 define %pair @pair_call_return() {
   %p = call %pair @pair_const()
   ret %pair %p
 }
 
 ; CHECK-LABEL: pair_call_indirect:
 ; CHECK-NEXT: .functype pair_call_indirect (i32) -> (i32, i64)
 ; CHECK-NEXT: local.get 0{{$}}
-; CHECK-NEXT: call_indirect () -> (i32, i64), 0{{$}}
-; REF:        call_indirect () -> (i32, i64), __indirect_function_table{{$}}
+; CHECK-NEXT: call_indirect () -> (i32, i64){{$}}
+; REF:        call_indirect __indirect_function_table, () -> (i32, i64){{$}}
 ; CHECK-NEXT: end_function{{$}}
 ; REGS: call_indirect $push{{[0-9]+}}=, $push{{[0-9]+}}=, $0{{$}}
 define %pair @pair_call_indirect(%pair()* %f) {
   %p = call %pair %f()
   ret %pair %p
 }
 
 ; CHECK-LABEL: pair_tail_call:

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

 # 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:      call        something2
 # CHECK-NEXT:      i32.const   0
-# CHECK-NEXT:      call_indirect (i32, f64) -> (), __indirect_function_table
+# CHECK-NEXT:      call_indirect __indirect_function_table, (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/test/MC/WebAssembly/call-indirect-relocs.s

 # RUN: llvm-mc -triple=wasm32-unknown-unknown -mattr=+reference-types < %s | FileCheck --check-prefix=CHECK %s
 # RUN: llvm-mc -triple=wasm32-unknown-unknown -mattr=+reference-types -filetype=obj < %s | obj2yaml | FileCheck -check-prefix=BIN %s
 
 test0:
     .functype   test0 () -> ()
     i32.const 42
     f64.const 2.5
     i32.const   0
-    call_indirect (i32, f64) -> (), empty_fref_table
+    call_indirect empty_fref_table, (i32, f64) -> ()
     end_function
 
 .tabletype empty_fref_table, funcref
 empty_fref_table:
 
 
 # CHECK:           .text
 # CHECK-LABEL: test0:
 # CHECK-NEXT:      .functype   test0 () -> ()
 # CHECK-NEXT:      i32.const   42
 # CHECK-NEXT:      f64.const   0x1.4p1
 # CHECK-NEXT:      i32.const   0
-# CHECK-NEXT:      call_indirect (i32, f64) -> (), empty_fref_table
+# CHECK-NEXT:      call_indirect empty_fref_table, (i32, f64) -> ()
 # CHECK-NEXT:      end_function
 
 # CHECK:           .tabletype empty_fref_table, funcref
 # CHECK: empty_fref_table:
 
 # BIN: --- !WASM
 # BIN-NEXT: FileHeader:
 # BIN-NEXT:   Version:         0x1

llvm/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 () -> ()
 
-    # REF: return_call_indirect (i32) -> (i32), __indirect_function_table # encoding: [0x13,
-    # CHECK: return_call_indirect (i32) -> (i32), 0 # encoding: [0x13,
+    # REF: return_call_indirect __indirect_function_table, (i32) -> (i32) # encoding: [0x13,
+    # 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/test/MC/WebAssembly/type-index.s

 # RUN: llvm-mc -triple=wasm32-unknown-unknown -mattr=+reference-types,+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=+reference-types,+unimplemented-simd128,+nontrapping-fptoint,+exception-handling < %s | obj2yaml | FileCheck -check-prefix=BIN %s
 
 # Minimal test for type indices and table references 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: call_indirect __indirect_function_table, (f64) -> (f64)
 # CHECK-NEXT: end_function
 
 # BIN:      --- !WASM
 # BIN-NEXT: FileHeader:
 # BIN-NEXT:   Version:         0x1
 # BIN-NEXT: Sections:
 # BIN-NEXT:   - Type:            TYPE
 # BIN-NEXT:     Signatures: