[WebAssembly] Added WasmAsmParser.

[aardappel]

Summary

This is to replace the ELFAsmParser that WebAssembly was using, which
so far was a stub that didn't do anything, and couldn't work correctly
with wasm.

This new class is there to implement generic directives related to
wasm as a binary format. Wasm target specific directives are still
parsed in WebAssemblyAsmParser as before. The two classes now
cooperate more correctly too.

Also implemented .result which was missing. Any unknown directives
will now result in errors.

[aardappel]

Forgot to add new file.

[aardappel]

Fixed title.

[dschuff]

otherwise lgtm

lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp

328–329

can we now remove this override?

[sbc100]

lib/MC/MCParser/WasmAsmParser.cpp

17

I think the common pattern is to join the file comment header to the copyright header and add another horizontal bar after.

[aardappel]

Addressed minor code review changes.

lib/MC/MCParser/AsmParser.cpp

 
 } // end anonymous namespace
 
 namespace llvm {
 
 extern MCAsmParserExtension *createDarwinAsmParser();
 extern MCAsmParserExtension *createELFAsmParser();
 extern MCAsmParserExtension *createCOFFAsmParser();
+extern MCAsmParserExtension *createWasmAsmParser();
 
 } // end namespace llvm
 
 enum { DEFAULT_ADDRSPACE = 0 };
 
 AsmParser::AsmParser(SourceMgr &SM, MCContext &Ctx, MCStreamer &Out,
                      const MCAsmInfo &MAI, unsigned CB = 0)
     : Lexer(MAI), Ctx(Ctx), Out(Out), MAI(MAI), SrcMgr(SM),
AsmParser::AsmParser(SourceMgr &SM, MCContext &Ctx, MCStreamer &Out,
   case MCObjectFileInfo::IsMachO:
     PlatformParser.reset(createDarwinAsmParser());
     IsDarwin = true;
     break;
   case MCObjectFileInfo::IsELF:
     PlatformParser.reset(createELFAsmParser());
     break;
   case MCObjectFileInfo::IsWasm:
-    // TODO: WASM will need its own MCAsmParserExtension implementation, but
-    // for now we can re-use the ELF one, since the directives can be the
-    // same for now.
-    PlatformParser.reset(createELFAsmParser());
+    PlatformParser.reset(createWasmAsmParser());
     break;
   }
 
   PlatformParser->Initialize(*this);
   initializeDirectiveKindMap();
 
   NumOfMacroInstantiations = 0;
 }

 bool AsmParser::parseDirectiveCFIStartProc() {
   StringRef Simple;
   if (!parseOptionalToken(AsmToken::EndOfStatement)) {
     if (check(parseIdentifier(Simple) || Simple != "simple",
               "unexpected token") ||
         parseToken(AsmToken::EndOfStatement))
       return addErrorSuffix(" in '.cfi_startproc' directive");
   }
-  
   // TODO(kristina): Deal with a corner case of incorrect diagnostic context
   // being produced if this directive is emitted as part of preprocessor macro
   // expansion which can *ONLY* happen if Clang's cc1as is the API consumer.
   // Tools like llvm-mc on the other hand are not affected by it, and report
   // correct context information.
   getStreamer().EmitCFIStartProc(!Simple.empty(), Lexer.getLoc());
   return false;
 }

lib/MC/MCParser/CMakeLists.txt

 add_llvm_library(LLVMMCParser
   AsmLexer.cpp
   AsmParser.cpp
   COFFAsmParser.cpp
   DarwinAsmParser.cpp
   ELFAsmParser.cpp
   MCAsmLexer.cpp
   MCAsmParser.cpp
   MCAsmParserExtension.cpp
   MCTargetAsmParser.cpp
+  WasmAsmParser.cpp
 
   ADDITIONAL_HEADER_DIRS
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/MC/MCParser
   )

lib/MC/MCParser/WasmAsmParser.cpp

+//===- WasmAsmParser.cpp - Wasm Assembly Parser -----------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+// --
+//
+// Note, this is for wasm, the binary format (analogous to ELF), not wasm,
+// the instruction set (analogous to x86), for which parsing code lives in
+// WebAssemblyAsmParser.
+//
+// This file contains processing for generic directives implemented using
+// MCTargetStreamer, the ones that depend on WebAssemblyTargetStreamer are in
+// WebAssemblyAsmParser.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/BinaryFormat/Wasm.h"
+#include "llvm/MC/MCContext.h"
+#include "llvm/MC/MCParser/MCAsmLexer.h"
+#include "llvm/MC/MCParser/MCAsmParser.h"
+#include "llvm/MC/MCParser/MCAsmParserExtension.h"
+#include "llvm/MC/MCStreamer.h"
+#include "llvm/MC/MCSymbol.h"
+#include "llvm/MC/MCSymbolWasm.h"
+#include "llvm/Support/MachineValueType.h"
+
+using namespace llvm;
+
+namespace {
+
+class WasmAsmParser : public MCAsmParserExtension {
+  MCAsmParser *Parser;
+  MCAsmLexer *Lexer;
+
+  template<bool (WasmAsmParser::*HandlerMethod)(StringRef, SMLoc)>
+  void addDirectiveHandler(StringRef Directive) {
+    MCAsmParser::ExtensionDirectiveHandler Handler = std::make_pair(
+        this, HandleDirective<WasmAsmParser, HandlerMethod>);
+
+    getParser().addDirectiveHandler(Directive, Handler);
+  }
+
+public:
+  WasmAsmParser() : Parser(nullptr), Lexer(nullptr) {
+    BracketExpressionsSupported = true;
+  }
+
+  void Initialize(MCAsmParser &P) override {
+    Parser = &P;
+    Lexer = &Parser->getLexer();
+    // Call the base implementation.
+    this->MCAsmParserExtension::Initialize(*Parser);
+
+    addDirectiveHandler<&WasmAsmParser::parseSectionDirectiveText>(".text");
+    addDirectiveHandler<&WasmAsmParser::parseSectionDirective>(".section");
+    addDirectiveHandler<&WasmAsmParser::parseDirectiveSize>(".size");
+    addDirectiveHandler<&WasmAsmParser::parseDirectiveType>(".type");
+  }
+
+  bool Error(const StringRef &msg, const AsmToken &tok) {
+    return Parser->Error(tok.getLoc(), msg + tok.getString());
+  }
+
+  bool IsNext(AsmToken::TokenKind Kind) {
+    auto ok = Lexer->is(Kind);
+    if (ok) Lex();
+    return ok;
+  }
+
+  bool Expect(AsmToken::TokenKind Kind, const char *KindName) {
+    if (!IsNext(Kind))
+      return Error(std::string("Expected ") + KindName + ", instead got: ",
+                   Lexer->getTok());
+    return false;
+  }
+
+  bool parseSectionDirectiveText(StringRef, SMLoc) {
+    // FIXME: .text currently no-op.
+    return false;
+  }
+
+  bool parseSectionDirective(StringRef, SMLoc) {
+    // FIXME: .section currently no-op.
+    while (Lexer->isNot(AsmToken::EndOfStatement)) Parser->Lex();
+    return false;
+  }
+
+  // TODO: This function is almost the same as ELFAsmParser::ParseDirectiveSize
+  // so maybe could be shared somehow.
+  bool parseDirectiveSize(StringRef, SMLoc) {
+    StringRef Name;
+    if (Parser->parseIdentifier(Name))
+      return TokError("expected identifier in directive");
+    auto Sym = getContext().getOrCreateSymbol(Name);
+    if (Lexer->isNot(AsmToken::Comma))
+      return TokError("unexpected token in directive");
+    Lex();
+    const MCExpr *Expr;
+    if (Parser->parseExpression(Expr))
+      return true;
+    if (Lexer->isNot(AsmToken::EndOfStatement))
+      return TokError("unexpected token in directive");
+    Lex();
+    // MCWasmStreamer implements this.
+    getStreamer().emitELFSize(Sym, Expr);
+    return false;
+  }
+
+  bool parseDirectiveType(StringRef, SMLoc) {
+    // This could be the start of a function, check if followed by
+    // "label,@function"
+    if (!Lexer->is(AsmToken::Identifier))
+      return Error("Expected label after .type directive, got: ",
+                   Lexer->getTok());
+    auto WasmSym = cast<MCSymbolWasm>(
+                     getStreamer().getContext().getOrCreateSymbol(
+                       Lexer->getTok().getString()));
+    Lex();
+    if (!(IsNext(AsmToken::Comma) && IsNext(AsmToken::At) &&
+          Lexer->is(AsmToken::Identifier)))
+      return Error("Expected label,@type declaration, got: ", Lexer->getTok());
+    auto TypeName = Lexer->getTok().getString();
+    if (TypeName == "function")
+      WasmSym->setType(wasm::WASM_SYMBOL_TYPE_FUNCTION);
+    else if (TypeName == "global")
+      WasmSym->setType(wasm::WASM_SYMBOL_TYPE_GLOBAL);
+    else
+      return Error("Unknown WASM symbol type: ", Lexer->getTok());
+    Lex();
+    return Expect(AsmToken::EndOfStatement, "EOL");
+  }
+};
+
+} // end anonymous namespace
+
+namespace llvm {
+
+MCAsmParserExtension *createWasmAsmParser() {
+  return new WasmAsmParser;
+}
+
+} // end namespace llvm

lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp

case Symbol:
       break;
     }
   }
 };
 
 class WebAssemblyAsmParser final : public MCTargetAsmParser {
   MCAsmParser &Parser;
   MCAsmLexer &Lexer;
-  MCSymbolWasm *LastSymbol;
 
 public:
   WebAssemblyAsmParser(const MCSubtargetInfo &STI, MCAsmParser &Parser,
                        const MCInstrInfo &MII, const MCTargetOptions &Options)
       : MCTargetAsmParser(Options, STI, MII), Parser(Parser),
-        Lexer(Parser.getLexer()), LastSymbol(nullptr) {
+        Lexer(Parser.getLexer()) {
     setAvailableFeatures(ComputeAvailableFeatures(STI.getFeatureBits()));
   }
 
 #define GET_ASSEMBLER_HEADER
 #include "WebAssemblyGenAsmMatcher.inc"
 
   // TODO: This is required to be implemented, but appears unused.
   bool ParseRegister(unsigned & /*RegNo*/, SMLoc & /*StartLoc*/,
return StringSwitch<std::pair<MVT::SimpleValueType, unsigned>>(RegType)
         .Case("f64x2", {MVT::v2f64, wasm::WASM_TYPE_V128})
         // arbitrarily chosen vector type to associate with "v128"
         // FIXME: should these be EVTs to avoid this arbitrary hack? Do we want
         // to accept more specific SIMD register types?
         .Case("v128", {MVT::v16i8, wasm::WASM_TYPE_V128})
         .Default({MVT::INVALID_SIMPLE_VALUE_TYPE, wasm::WASM_TYPE_NORESULT});
   }
 
+  bool ParseRegTypeList(std::vector<MVT> &Types) {
+    while (Lexer.is(AsmToken::Identifier)) {
+      auto RegType = ParseRegType(Lexer.getTok().getString()).first;
+      if (RegType == MVT::INVALID_SIMPLE_VALUE_TYPE)
+        return true;
+      Types.push_back(RegType);
+      Parser.Lex();
+      if (!IsNext(AsmToken::Comma))
+        break;
+    }
+    return Expect(AsmToken::EndOfStatement, "EOL");
+  }
+
   void ParseSingleInteger(bool IsNegative, OperandVector &Operands) {
     auto &Int = Lexer.getTok();
     int64_t Val = Int.getIntVal();
     if (IsNegative)
       Val = -Val;
     Operands.push_back(make_unique<WebAssemblyOperand>(
         WebAssemblyOperand::Integer, Int.getLoc(), Int.getEndLoc(),
         WebAssemblyOperand::IntOp{Val}));
bool ParseInstruction(ParseInstructionInfo & /*Info*/, StringRef Name,
     // the wasm module is generated).
     if (BaseName == "block" || BaseName == "loop" || BaseName == "try") {
       Operands.push_back(make_unique<WebAssemblyOperand>(
           WebAssemblyOperand::Integer, NameLoc, NameLoc,
           WebAssemblyOperand::IntOp{-1}));
     }
     return false;
   }
 
-  void onLabelParsed(MCSymbol *Symbol) override {
-    LastSymbol = cast<MCSymbolWasm>(Symbol);
-  }
-
+  // This function processes wasm-specific directives streamed to
+  // WebAssemblyTargetStreamer, all others go to the generic parser
+  // (see WasmAsmParser).
   bool ParseDirective(AsmToken DirectiveID) override {
     // This function has a really weird return value behavior that is different
     // from all the other parsing functions:
     // - return true && no tokens consumed -> don't know this directive / let
     //   the generic parser handle it.
     // - return true && tokens consumed -> a parsing error occurred.
     // - return false -> processed this directive successfully.
     assert(DirectiveID.getKind() == AsmToken::Identifier);
     auto &Out = getStreamer();
     auto &TOut =
         reinterpret_cast<WebAssemblyTargetStreamer &>(*Out.getTargetStreamer());
     // TODO: any time we return an error, at least one token must have been
     // consumed, otherwise this will not signal an error to the caller.
-    if (DirectiveID.getString() == ".type") {
-      // This could be the start of a function, check if followed by
-      // "label,@function"
-      if (!Lexer.is(AsmToken::Identifier))
-        return Error("Expected label after .type directive, got: ",
-                     Lexer.getTok());
-      auto WasmSym = cast<MCSymbolWasm>(
-                       TOut.getStreamer().getContext().getOrCreateSymbol(
-                         Lexer.getTok().getString()));
-      Parser.Lex();
-      if (!(IsNext(AsmToken::Comma) && IsNext(AsmToken::At) &&
-            Lexer.is(AsmToken::Identifier)))
-        return Error("Expected label,@type declaration, got: ", Lexer.getTok());
-      auto TypeName = Lexer.getTok().getString();
-      if (TypeName == "function")
-        WasmSym->setType(wasm::WASM_SYMBOL_TYPE_FUNCTION);
-      else if (TypeName == "global")
-        WasmSym->setType(wasm::WASM_SYMBOL_TYPE_GLOBAL);
-      else
-        return Error("Unknown WASM symbol type: ", Lexer.getTok());
-      Parser.Lex();
-      return Expect(AsmToken::EndOfStatement, "EOL");
-    } else if (DirectiveID.getString() == ".size") {
-      if (!Lexer.is(AsmToken::Identifier))
-        return Error("Expected label after .size directive, got: ",
-                     Lexer.getTok());
-      auto WasmSym = cast<MCSymbolWasm>(
-                       TOut.getStreamer().getContext().getOrCreateSymbol(
-                         Lexer.getTok().getString()));
-      Parser.Lex();
-      if (!IsNext(AsmToken::Comma))
-        return Error("Expected `,`, got: ", Lexer.getTok());
-      const MCExpr *Exp;
-      if (Parser.parseExpression(Exp))
-        return Error("Cannot parse .size expression: ", Lexer.getTok());
-      WasmSym->setSize(Exp);
-      return Expect(AsmToken::EndOfStatement, "EOL");
-    } else if (DirectiveID.getString() == ".globaltype") {
+    if (DirectiveID.getString() == ".globaltype") {
       if (!Lexer.is(AsmToken::Identifier))
         return Error("Expected symbol name after .globaltype directive, got: ",
                      Lexer.getTok());
       auto Name = Lexer.getTok().getString();
       Parser.Lex();
       if (!IsNext(AsmToken::Comma))
         return Error("Expected `,`, got: ", Lexer.getTok());
       if (!Lexer.is(AsmToken::Identifier))
         return Error("Expected type in .globaltype directive, got: ",
                      Lexer.getTok());
       auto Type = ParseRegType(Lexer.getTok().getString()).second;
       if (Type == wasm::WASM_TYPE_NORESULT)
         return Error("Unknown type in .globaltype directive: ",
                      Lexer.getTok());
       Parser.Lex();
       // Now set this symbol with the correct type.
       auto WasmSym = cast<MCSymbolWasm>(
                        TOut.getStreamer().getContext().getOrCreateSymbol(Name));
       WasmSym->setType(wasm::WASM_SYMBOL_TYPE_GLOBAL);
       WasmSym->setGlobalType(wasm::WasmGlobalType{uint8_t(Type), true});
       // And emit the directive again.
       TOut.emitGlobalType(WasmSym);
       return Expect(AsmToken::EndOfStatement, "EOL");
-    } else if (DirectiveID.getString() == ".param" ||
-               DirectiveID.getString() == ".local") {
-      // Track the number of locals, needed for correct virtual register
-      // assignment elsewhere.
-      // Also output a directive to the streamer.
+    } else if (DirectiveID.getString() == ".param") {
       std::vector<MVT> Params;
+      if (ParseRegTypeList(Params)) return true;
+      TOut.emitParam(nullptr /* unused */, Params);
+      return false;
+    } else if (DirectiveID.getString() == ".result") {
+      std::vector<MVT> Results;
+      if (ParseRegTypeList(Results)) return true;
+      TOut.emitResult(nullptr /* unused */, Results);
+      return false;
+    } else if (DirectiveID.getString() == ".local") {
       std::vector<MVT> Locals;
-      while (Lexer.is(AsmToken::Identifier)) {
-        auto RegType = ParseRegType(Lexer.getTok().getString()).first;
-        if (RegType == MVT::INVALID_SIMPLE_VALUE_TYPE)
-          return true;
-        if (DirectiveID.getString() == ".param") {
-          Params.push_back(RegType);
-        } else {
-          Locals.push_back(RegType);
-        }
-        Parser.Lex();
-        if (!IsNext(AsmToken::Comma))
-          break;
-      }
-      assert(LastSymbol);
-      // TODO: LastSymbol isn't even used by emitParam, so could be removed.
-      TOut.emitParam(LastSymbol, Params);
+      if (ParseRegTypeList(Locals)) return true;
       TOut.emitLocal(Locals);
-      return Expect(AsmToken::EndOfStatement, "EOL");
-    } else {
-      // TODO: remove.
-      while (Lexer.isNot(AsmToken::EndOfStatement))
-        Parser.Lex();
-      return Expect(AsmToken::EndOfStatement, "EOL");
-    }
-    // TODO: current ELF directive parsing is broken, fix this is a followup.
-    //return true;  // We didn't process this directive.
       return false;
     }
+    return true;  // We didn't process this directive.
+  }
 
   bool MatchAndEmitInstruction(SMLoc IDLoc, unsigned & /*Opcode*/,
                                OperandVector &Operands, MCStreamer &Out,
                                uint64_t &ErrorInfo,
                                bool MatchingInlineAsm) override {
     MCInst Inst;
     unsigned MatchResult =
         MatchInstructionImpl(Operands, Inst, ErrorInfo, MatchingInlineAsm);

test/CodeGen/WebAssembly/inline-asm-roundtrip.ll

 ;   return dst != 3;
 ; }
 
 target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
 target triple = "wasm32-unknown-unknown"
 
 ; CHECK-LABEL: main:
 ; CHECK-NEXT: .param    i32, i32
+; CHECK-NEXT: .result   i32
 ; CHECK-NEXT: .local    i32
 ; CHECK-NEXT: i32.const 1
 ; CHECK-NEXT: set_local [[SRC:[0-9]+]]
 ; CHECK-NEXT: i32.const 2
 ; CHECK-NEXT: get_local [[SRC]]
 ; CHECK-NEXT: i32.add
 ; CHECK-NEXT: set_local [[DST:[0-9]+]]
 ; CHECK-NEXT: get_local [[DST]]

test/MC/WebAssembly/basic-assembly.s

 # RUN: llvm-mc -triple=wasm32-unknown-unknown -mattr=+simd128,+nontrapping-fptoint,+exception-handling < %s | FileCheck %s
 # this one is just here to see if it converts to .o without errors, but doesn't check any output:
 # RUN: llvm-mc -triple=wasm32-unknown-unknown -filetype=obj -mattr=+simd128,+nontrapping-fptoint,+exception-handling < %s
 
     .text
+    .section .text.main,"",@
     .type    test0,@function
 test0:
     # Test all types:
     .param      i32, i64
+    .result     i32
     .local      f32, f64, v128, v128
     # Explicit getlocal/setlocal:
     get_local   2
     set_local   2
     # Immediates:
     i32.const   -1
     f64.const   0x1.999999999999ap1
     v128.const  0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15
.LBB0_5:
     end_function
 .Lfunc_end0:
   .size test0, .Lfunc_end0-test0
     .globaltype __stack_pointer, i32
 
 # CHECK:           .text
 # CHECK-LABEL: test0:
 # CHECK-NEXT:      .param      i32, i64
+# CHECK-NEXT:      .result     i32
 # CHECK-NEXT:      .local      f32, f64
 # CHECK-NEXT:      get_local   2
 # CHECK-NEXT:      set_local   2
 # CHECK-NEXT:      i32.const   -1
 # CHECK-NEXT:      f64.const   0x1.999999999999ap1
 # CHECK-NEXT:      v128.const  0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15
 # CHECK-NEXT:      v128.const  0, 1, 2, 3, 4, 5, 6, 7
 # CHECK-NEXT:      get_local   0