[llvm] 4307069 - [WebAssembly] Swap operand order of call_indirect in text format

Andy Wingo via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 2 23:52:49 PST 2021


Author: Andy Wingo
Date: 2021-03-03T08:51:21+01:00
New Revision: 4307069df442007e6ceb5a914031bb95a455386d

URL: https://github.com/llvm/llvm-project/commit/4307069df442007e6ceb5a914031bb95a455386d
DIFF: https://github.com/llvm/llvm-project/commit/4307069df442007e6ceb5a914031bb95a455386d.diff

LOG: [WebAssembly] Swap operand order of call_indirect in text format

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

Added: 
    

Modified: 
    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

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp b/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
index cd0f9dc32ec4..c17067351154 100644
--- a/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
+++ b/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
@@ -487,35 +487,38 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
         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 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
     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 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
       if (pop(Name, Function) || ensureEmptyNestingStack())
         return true;
     } else if (Name == "call_indirect" || Name == "return_call_indirect") {
+      // These instructions have 
diff ering 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 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
       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 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
               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 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
       // 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;
   }

diff  --git a/llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyInstPrinter.cpp b/llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyInstPrinter.cpp
index 11d8f7bf40d1..cc58af6a8f87 100644
--- a/llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyInstPrinter.cpp
+++ b/llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyInstPrinter.cpp
@@ -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());

diff  --git a/llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp
index d60c1624ff16..994baf797c7c 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp
@@ -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(

diff  --git a/llvm/test/CodeGen/WebAssembly/function-pointer64.ll b/llvm/test/CodeGen/WebAssembly/function-pointer64.ll
index dd739d67e14c..155e4807d297 100644
--- a/llvm/test/CodeGen/WebAssembly/function-pointer64.ll
+++ b/llvm/test/CodeGen/WebAssembly/function-pointer64.ll
@@ -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

diff  --git a/llvm/test/CodeGen/WebAssembly/multivalue.ll b/llvm/test/CodeGen/WebAssembly/multivalue.ll
index a57e2cf7fddf..aa2313f9ec09 100644
--- a/llvm/test/CodeGen/WebAssembly/multivalue.ll
+++ b/llvm/test/CodeGen/WebAssembly/multivalue.ll
@@ -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) {

diff  --git a/llvm/test/MC/WebAssembly/basic-assembly.s b/llvm/test/MC/WebAssembly/basic-assembly.s
index dcfd9addb061..2faddd215433 100644
--- a/llvm/test/MC/WebAssembly/basic-assembly.s
+++ b/llvm/test/MC/WebAssembly/basic-assembly.s
@@ -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

diff  --git a/llvm/test/MC/WebAssembly/call-indirect-relocs.s b/llvm/test/MC/WebAssembly/call-indirect-relocs.s
index 4244fbf3abea..be4202f7d494 100644
--- a/llvm/test/MC/WebAssembly/call-indirect-relocs.s
+++ b/llvm/test/MC/WebAssembly/call-indirect-relocs.s
@@ -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

diff  --git a/llvm/test/MC/WebAssembly/tail-call-encodings.s b/llvm/test/MC/WebAssembly/tail-call-encodings.s
index 20bfb2e84a24..1b2f85b43686 100644
--- a/llvm/test/MC/WebAssembly/tail-call-encodings.s
+++ b/llvm/test/MC/WebAssembly/tail-call-encodings.s
@@ -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 at TYPEINDEX, kind: fixup_uleb128_i32
     return_call_indirect (i32) -> (i32)
 

diff  --git a/llvm/test/MC/WebAssembly/type-index.s b/llvm/test/MC/WebAssembly/type-index.s
index dd6581b826f5..eef1a1012466 100644
--- a/llvm/test/MC/WebAssembly/type-index.s
+++ b/llvm/test/MC/WebAssembly/type-index.s
@@ -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


        


More information about the llvm-commits mailing list