[WebAssembly] Swap operand order of call_indirect in text format
authorAndy Wingo <wingo@igalia.com>
Tue, 2 Mar 2021 11:18:16 +0000 (12:18 +0100)
committerAndy Wingo <wingo@igalia.com>
Wed, 3 Mar 2021 07:51:21 +0000 (08:51 +0100)
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.

Differential Revision: https://reviews.llvm.org/D97761

llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyInstPrinter.cpp
llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp
llvm/test/CodeGen/WebAssembly/function-pointer64.ll
llvm/test/CodeGen/WebAssembly/multivalue.ll
llvm/test/MC/WebAssembly/basic-assembly.s
llvm/test/MC/WebAssembly/call-indirect-relocs.s
llvm/test/MC/WebAssembly/tail-call-encodings.s
llvm/test/MC/WebAssembly/type-index.s

index cd0f9dc..c170673 100644 (file)
@@ -487,35 +487,38 @@ public:
         WebAssemblyOperand::IntOp{static_cast<int64_t>(BT)}));
   }
 
-  bool addFunctionTableOperand(OperandVector &Operands, MCSymbolWasm *Sym,
-                               SMLoc StartLoc, SMLoc EndLoc) {
-    const auto *Val = MCSymbolRefExpr::create(Sym, getContext());
-    Operands.push_back(std::make_unique<WebAssemblyOperand>(
-        WebAssemblyOperand::Symbol, StartLoc, EndLoc,
-        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) {
+  bool parseFunctionTableOperand(std::unique_ptr<WebAssemblyOperand> *Op) {
     if (STI->checkFeatures("+reference-types")) {
-      return addFunctionTableOperand(Operands, DefaultFunctionTable, StartLoc,
-                                     EndLoc);
+      // 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());
+        *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;
+      }
     } 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;
     }
   }
@@ -556,7 +559,7 @@ public:
     bool ExpectBlockType = false;
     bool ExpectFuncType = false;
     bool ExpectHeapType = false;
-    bool ExpectFunctionTable = false;
+    std::unique_ptr<WebAssemblyOperand> FunctionTable;
     if (Name == "block") {
       push(Block);
       ExpectBlockType = true;
@@ -602,8 +605,12 @@ public:
       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;
     }
@@ -631,16 +638,6 @@ public:
       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)) {
@@ -666,11 +663,6 @@ public:
               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;
@@ -737,12 +729,8 @@ public:
       // 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;
   }
index 11d8f7b..cc58af6 100644 (file)
@@ -48,8 +48,35 @@ void WebAssemblyInstPrinter::printInst(const MCInst *MI, uint64_t Address,
                                        StringRef Annot,
                                        const MCSubtargetInfo &STI,
                                        raw_ostream &OS) {
-  // Print the instruction (this uses the AsmStrings from the .td files).
-  printInstruction(MI, Address, 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());
index d60c162..994baf7 100644 (file)
@@ -869,7 +869,7 @@ bool WebAssemblyFastISel::selectCall(const Instruction *I) {
   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(
index dd739d6..155e480 100644 (file)
@@ -35,8 +35,8 @@ entry:
 ; 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
index a57e2cf..aa2313f 100644 (file)
@@ -58,8 +58,8 @@ define %pair @pair_call_return() {
 ; 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) {
index dcfd9ad..2faddd2 100644 (file)
@@ -156,7 +156,7 @@ empty_fref_table:
 # 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
index 4244fbf..be4202f 100644 (file)
@@ -6,7 +6,7 @@ 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
@@ -19,7 +19,7 @@ empty_fref_table:
 # 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
index 20bfb2e..1b2f85b 100644 (file)
@@ -17,8 +17,8 @@ foo1:
 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)
 
index dd6581b..eef1a10 100644 (file)
@@ -12,7 +12,7 @@ test0:
 # 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