[RISCV 6/10] Add basic RISCVAsmParser

[asb]

Summary

This doesn't yet support parsing things like %pcrel_hi(foo), but will handle basic instructions with register or immediate operands.

Note: this patch is reliant on D23496

[t.p.northover]

lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

299

I think this should be MatchOperand_NoMatch. ParseFail normally means you've already lexed something so backtracking is impossible.

[reames]

lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

176

style: shouldn't this be createToken?

204

This seems odd. Is this a standard convention? Or can we lift this check into the caller and assert the Expr is not null?

347

This is a reasonable amount of parsing code without any associated tests. Can we write even trivial tests for this code at this point?

[asb]

Address review comments. Note that the way we defined AsmOperandClass has changed, which will be built upon in the next patch.

lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

204

Some targets define a whole bunch of AsmOperandClass, which means they'll have a lot of addFooOperands which eventually call addExpr. Checking !Expr in addExpr seems to be what the other backends I've checked do. That said, none of my testcases result in a null Expr so it may be one of those things that has been copied blindly by every backend author. I've added an assert to addExpr.

347

The next patch (D23564) adds tests and not much else (a very trivial instruction printer).

[t.p.northover]

Just spotted one nit on my last pass through the code:

lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

361–362

Isn't the first check redundant here?

[jyknight]

lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

31–34

Why are these here? Looks like MCTargetAsmParser already has a Parser member and these accessors.

35

Similarly, already stored in MCTargetAsmParser, accessible via getSTI().

[asb]

Address review comments from @jyknight and @t.p.northover. Also use # rather than !strconcat in RISCVInstrInfo.td

[asb]

Refresh to follow changes made in rL283702, putting TheRISCV32Target and TheRISCV64Target behind accessors. Additionally, drop the unused getEndLoc after rL283691 removed it from MCParsedAsmOperand

[grosser]

Hi Alex,

I silently followed the RISC-V patches being upstreamed and did not see an update here for a little while. Is this patch blocked on something or did you just not get to this one yet?

Best,
Tobias

[majnemer]

lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

79–82

Isn't this just a StringRef?

[asb]

In D23563#639241, @grosser wrote:

Hi Alex,

I silently followed the RISC-V patches being upstreamed and did not see an update here for a little while. Is this patch blocked on something or did you just not get to this one yet?

Due to other responsibilities and my ongoing attempts to find further funding I'm a little behind where I wanted to be with the next set of patches. There were some concerns that aspects of this patchset might need adjusting once codegen was added, which is why I've held back on merging this part of the patchset. In retrospect, this probably wasn't necessary.

[asb]

Update to address review comment (just use StringRef rather than introducing Token struct).

lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

79–82

Yes, I've replaced it with a StringRef now - thanks.

[alekseyshl]

http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux/builds/3300 is still unhappy about RISCVAsmParser.cpp

[asb]

@alekseyshl: I've just committed rL310375 which should fix it.

[asb]

RISCV should never have been included in LLVM_ALL_TARGETS when first committed - that was a mistake in the initial 'stub backend' patch. Removing RISCV from LLVM_ALL_TARGETS is the correct thing to do and shouldn't affect end-users, but I'll write to llvm-dev about this tomorrow for wider feedback.

lib/Target/RISCV/AsmParser/CMakeLists.txt

+add_llvm_library(LLVMRISCVAsmParser
+  RISCVAsmParser.cpp
+  )

lib/Target/RISCV/AsmParser/LLVMBuild.txt

+;===- ./lib/Target/RISCV/AsmParser/LLVMBuild.txt ---------------*- Conf -*--===;
+;
+;                     The LLVM Compiler Infrastructure
+;
+; This file is distributed under the University of Illinois Open Source
+; License. See LICENSE.TXT for details.
+;
+;===------------------------------------------------------------------------===;
+;
+; This is an LLVMBuild description file for the components in this subdirectory.
+;
+; For more information on the LLVMBuild system, please see:
+;
+;   http://llvm.org/docs/LLVMBuild.html
+;
+;===------------------------------------------------------------------------===;
+
+[component_0]
+type = Library
+name = RISCVAsmParser
+parent = RISCV
+required_libraries = MC MCParser RISCVDesc RISCVInfo Support
+add_to_library_groups = RISCV

lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

+//===-- RISCVAsmParser.cpp - Parse RISCV assembly to MCInst instructions --===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "MCTargetDesc/RISCVMCTargetDesc.h"
+#include "llvm/MC/MCParser/MCAsmLexer.h"
+#include "llvm/MC/MCParser/MCParsedAsmOperand.h"
+#include "llvm/MC/MCParser/MCTargetAsmParser.h"
+#include "llvm/MC/MCContext.h"
+#include "llvm/MC/MCRegisterInfo.h"
+#include "llvm/MC/MCStreamer.h"
+#include "llvm/MC/MCSubtargetInfo.h"
+#include "llvm/MC/MCExpr.h"
+#include "llvm/MC/MCInst.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringSwitch.h"
+#include "llvm/Support/TargetRegistry.h"
+
+using namespace llvm;
+
+namespace {
+struct RISCVOperand;
+
+class RISCVAsmParser : public MCTargetAsmParser {
+  SMLoc getLoc() const { return getParser().getTok().getLoc(); }
+
+  bool MatchAndEmitInstruction(SMLoc IDLoc, unsigned &Opcode,
+                               OperandVector &Operands, MCStreamer &Out,
+                               uint64_t &ErrorInfo,
+                               bool MatchingInlineAsm) override;
+
+  bool ParseRegister(unsigned &RegNo, SMLoc &StartLoc, SMLoc &EndLoc) override;
+
+  bool ParseInstruction(ParseInstructionInfo &Info, StringRef Name,
+                        SMLoc NameLoc, OperandVector &Operands) override;
+
+  bool ParseDirective(AsmToken DirectiveID) override;
+
+// Auto-generated instruction matching functions
+#define GET_ASSEMBLER_HEADER
+#include "RISCVGenAsmMatcher.inc"
+
+  OperandMatchResultTy parseImmediate(OperandVector &Operands);
+  OperandMatchResultTy parseRegister(OperandVector &Operands);
+
+  bool parseOperand(OperandVector &Operands);
+
+public:
+  enum RISCVMatchResultTy {
+    Match_Dummy = FIRST_TARGET_MATCH_RESULT_TY,
+#define GET_OPERAND_DIAGNOSTIC_TYPES
+#include "RISCVGenAsmMatcher.inc"
+#undef GET_OPERAND_DIAGNOSTIC_TYPES
+  };
+
+  RISCVAsmParser(const MCSubtargetInfo &STI, MCAsmParser &Parser,
+                 const MCInstrInfo &MII, const MCTargetOptions &Options)
+      : MCTargetAsmParser(Options, STI) {
+    setAvailableFeatures(ComputeAvailableFeatures(STI.getFeatureBits()));
+  }
+};
+
+/// RISCVOperand - Instances of this class represent a parsed machine
+/// instruction
+struct RISCVOperand : public MCParsedAsmOperand {
+
+  enum KindTy {
+    Token,
+    Register,
+    Immediate,
+  } Kind;
+
+  struct TokOp {
+    const char *Data;
+    unsigned Length;
+  };
+
+  struct RegOp {
+    unsigned RegNum;
+  };
+
+  struct ImmOp {
+    const MCExpr *Val;
+  };
+
+  SMLoc StartLoc, EndLoc;
+  union {
+    TokOp Tok;
+    RegOp Reg;
+    ImmOp Imm;
+  };
+
+  RISCVOperand(KindTy K) : MCParsedAsmOperand(), Kind(K) {}
+
+public:
+  RISCVOperand(const RISCVOperand &o) : MCParsedAsmOperand() {
+    Kind = o.Kind;
+    StartLoc = o.StartLoc;
+    EndLoc = o.EndLoc;
+    switch (Kind) {
+    case Register:
+      Reg = o.Reg;
+      break;
+    case Immediate:
+      Imm = o.Imm;
+      break;
+    case Token:
+      Tok = o.Tok;
+      break;
+    }
+  }
+
+  bool isToken() const override { return Kind == Token; }
+  bool isReg() const override { return Kind == Register; }
+  bool isImm() const override { return Kind == Immediate; }
+  bool isMem() const override { return false; }
+
+  bool isConstantImm() const {
+    return isImm() && dyn_cast<MCConstantExpr>(getImm());
+  }
+
+  int64_t getConstantImm() const {
+    const MCExpr *Val = getImm();
+    return static_cast<const MCConstantExpr *>(Val)->getValue();
+  }
+
+  bool isSImm12() const {
+    return (isConstantImm() && isInt<12>(getConstantImm()));
+  }
+
+  /// getStartLoc - Gets location of the first token of this operand
+  SMLoc getStartLoc() const override { return StartLoc; }
+  /// getEndLoc - Gets location of the last token of this operand
+  SMLoc getEndLoc() const override { return EndLoc; }
+
+  unsigned getReg() const override {
+    assert(Kind == Register && "Invalid type access!");
+    return Reg.RegNum;
+  }
+
+  const MCExpr *getImm() const {
+    assert(Kind == Immediate && "Invalid type access!");
+    return Imm.Val;
+  }
+
+  StringRef getToken() const {
+    assert(Kind == Token && "Invalid type access!");
+    return StringRef(Tok.Data, Tok.Length);
+  }
+
+  void print(raw_ostream &OS) const override {
+    switch (Kind) {
+    case Immediate:
+      OS << *getImm();
+      break;
+    case Register:
+      OS << "<register x";
+      OS << getReg() << ">";
+      break;
+    case Token:
+      OS << "'" << getToken() << "'";
+      break;
+    }
+  }
+
+  static std::unique_ptr<RISCVOperand> createToken(StringRef Str, SMLoc S) {
+    auto Op = make_unique<RISCVOperand>(Token);
+    Op->Tok.Data = Str.data();
+    Op->Tok.Length = Str.size();
+    Op->StartLoc = S;
+    Op->EndLoc = S;
+    return Op;
+  }
+
+  static std::unique_ptr<RISCVOperand> createReg(unsigned RegNo, SMLoc S,
+                                                 SMLoc E) {
+    auto Op = make_unique<RISCVOperand>(Register);
+    Op->Reg.RegNum = RegNo;
+    Op->StartLoc = S;
+    Op->EndLoc = E;
+    return Op;
+  }
+
+  static std::unique_ptr<RISCVOperand> createImm(const MCExpr *Val, SMLoc S,
+                                                 SMLoc E) {
+    auto Op = make_unique<RISCVOperand>(Immediate);
+    Op->Imm.Val = Val;
+    Op->StartLoc = S;
+    Op->EndLoc = E;
+    return Op;
+  }
+
+  void addExpr(MCInst &Inst, const MCExpr *Expr) const {
+    assert(Expr && "Expr shouldn't be null!");
+    if (auto *CE = dyn_cast<MCConstantExpr>(Expr))
+      Inst.addOperand(MCOperand::createImm(CE->getValue()));
+    else
+      Inst.addOperand(MCOperand::createExpr(Expr));
+  }
+
+  // Used by the TableGen Code
+  void addRegOperands(MCInst &Inst, unsigned N) const {
+    assert(N == 1 && "Invalid number of operands!");
+    Inst.addOperand(MCOperand::createReg(getReg()));
+  }
+
+  void addImmOperands(MCInst &Inst, unsigned N) const {
+    assert(N == 1 && "Invalid number of operands!");
+    addExpr(Inst, getImm());
+  }
+};
+} // end anonymous namespace.
+
+#define GET_REGISTER_MATCHER
+#define GET_MATCHER_IMPLEMENTATION
+#define GET_SUBTARGET_FEATURE_NAME
+#include "RISCVGenAsmMatcher.inc"
+
+bool RISCVAsmParser::MatchAndEmitInstruction(SMLoc IDLoc, unsigned &Opcode,
+                                             OperandVector &Operands,
+                                             MCStreamer &Out,
+                                             uint64_t &ErrorInfo,
+                                             bool MatchingInlineAsm) {
+  MCInst Inst;
+  SMLoc ErrorLoc;
+
+  switch (MatchInstructionImpl(Operands, Inst, ErrorInfo, MatchingInlineAsm)) {
+  default:
+    break;
+  case Match_Success:
+    Inst.setLoc(IDLoc);
+    Out.EmitInstruction(Inst, getSTI());
+    return false;
+  case Match_MissingFeature:
+    return Error(IDLoc, "instruction use requires an option to be enabled");
+  case Match_MnemonicFail:
+    return Error(IDLoc, "unrecognized instruction mnemonic");
+  case Match_InvalidOperand:
+    ErrorLoc = IDLoc;
+    if (ErrorInfo != ~0U) {
+      if (ErrorInfo >= Operands.size())
+        return Error(ErrorLoc, "too few operands for instruction");
+
+      ErrorLoc = ((RISCVOperand &)*Operands[ErrorInfo]).getStartLoc();
+      if (ErrorLoc == SMLoc())
+        ErrorLoc = IDLoc;
+    }
+    return Error(ErrorLoc, "invalid operand for instruction");
+  case Match_InvalidSImm12:
+    SMLoc ErrorLoc = ((RISCVOperand &)*Operands[ErrorInfo]).getStartLoc();
+    return Error(ErrorLoc,
+                 "immediate must be an integer in the range [-2048, 2047]");
+  }
+
+  llvm_unreachable("Unknown match type detected!");
+}
+
+bool RISCVAsmParser::ParseRegister(unsigned &RegNo, SMLoc &StartLoc,
+                                   SMLoc &EndLoc) {
+  const AsmToken &Tok = getParser().getTok();
+  StartLoc = Tok.getLoc();
+  EndLoc = Tok.getEndLoc();
+  RegNo = 0;
+  StringRef Name = getLexer().getTok().getIdentifier();
+
+  if (!MatchRegisterName(Name) || !MatchRegisterAltName(Name)) {
+    getParser().Lex(); // Eat identifier token.
+    return false;
+  }
+
+  return Error(StartLoc, "invalid register name");
+}
+
+RISCVAsmParser::OperandMatchResultTy
+RISCVAsmParser::parseRegister(OperandVector &Operands) {
+  SMLoc S = getLoc();
+  SMLoc E = SMLoc::getFromPointer(S.getPointer() - 1);
+
+  switch (getLexer().getKind()) {
+  default:
+    return MatchOperand_NoMatch;
+  case AsmToken::Identifier:
+    StringRef Name = getLexer().getTok().getIdentifier();
+    unsigned RegNo = MatchRegisterName(Name);
+    if (RegNo == 0) {
+      RegNo = MatchRegisterAltName(Name);
+      if (RegNo == 0)
+        return MatchOperand_NoMatch;
+    }
+    getLexer().Lex();
+    Operands.push_back(RISCVOperand::createReg(RegNo, S, E));
+  }
+  return MatchOperand_Success;
+}
+
+RISCVAsmParser::OperandMatchResultTy
+RISCVAsmParser::parseImmediate(OperandVector &Operands) {
+  switch (getLexer().getKind()) {
+  default:
+    return MatchOperand_NoMatch;
+  case AsmToken::LParen:
+  case AsmToken::Minus:
+  case AsmToken::Plus:
+  case AsmToken::Integer:
+  case AsmToken::String:
+    break;
+  }
+
+  const MCExpr *IdVal;
+  SMLoc S = getLoc();
+  if (getParser().parseExpression(IdVal))
+    return MatchOperand_ParseFail;
+
+  SMLoc E = SMLoc::getFromPointer(S.getPointer() - 1);
+  Operands.push_back(RISCVOperand::createImm(IdVal, S, E));
+  return MatchOperand_Success;
+}
+
+/// Looks at a token type and creates the relevant operand
+/// from this information, adding to Operands.
+/// If operand was parsed, returns false, else true.
+bool RISCVAsmParser::parseOperand(OperandVector &Operands) {
+  // Attempt to parse token as register
+  if (parseRegister(Operands) == MatchOperand_Success)
+    return false;
+
+  // Attempt to parse token as an immediate
+  if (parseImmediate(Operands) == MatchOperand_Success)
+    return false;
+
+  // Finally we have exhausted all options and must declare defeat.
+  Error(getLoc(), "unknown operand");
+  return true;
+}
+
+bool RISCVAsmParser::ParseInstruction(ParseInstructionInfo &Info,
+                                      StringRef Name, SMLoc NameLoc,
+                                      OperandVector &Operands) {
+  // First operand is token for instruction
+  Operands.push_back(RISCVOperand::createToken(Name, NameLoc));
+
+  // If there are no more operands, then finish
+  if (getLexer().is(AsmToken::EndOfStatement))
+    return false;
+
+  // Parse first operand
+  if (parseOperand(Operands))
+    return true;
+
+  // Parse until end of statement, consuming commas between operands
+  while (getLexer().is(AsmToken::Comma)) {
+    // Consume comma token
+    getLexer().Lex();
+
+    // Parse next operand
+    if (parseOperand(Operands))
+      return true;
+  }
+
+  if (getLexer().isNot(AsmToken::EndOfStatement)) {
+    SMLoc Loc = getLexer().getLoc();
+    getParser().eatToEndOfStatement();
+    return Error(Loc, "unexpected token");
+  }
+
+  getParser().Lex(); // Consume the EndOfStatement.
+  return false;
+}
+
+bool RISCVAsmParser::ParseDirective(AsmToken DirectiveID) { return true; }
+
+extern "C" void LLVMInitializeRISCVAsmParser() {
+  RegisterMCAsmParser<RISCVAsmParser> X(TheRISCV32Target);
+  RegisterMCAsmParser<RISCVAsmParser> Y(TheRISCV64Target);
+}

lib/Target/RISCV/CMakeLists.txt

 set(LLVM_TARGET_DEFINITIONS RISCV.td)
 
 tablegen(LLVM RISCVGenRegisterInfo.inc -gen-register-info)
 tablegen(LLVM RISCVGenInstrInfo.inc -gen-instr-info)
 tablegen(LLVM RISCVGenMCCodeEmitter.inc -gen-emitter)
+tablegen(LLVM RISCVGenAsmMatcher.inc -gen-asm-matcher)
 
 add_public_tablegen_target(RISCVCommonTableGen)
 
 add_llvm_target(RISCVCodeGen
   RISCVTargetMachine.cpp
   )
 
+add_subdirectory(AsmParser)
 add_subdirectory(TargetInfo)
 add_subdirectory(MCTargetDesc)

lib/Target/RISCV/LLVMBuild.txt

 ;
 ; For more information on the LLVMBuild system, please see:
 ;
 ;   http://llvm.org/docs/LLVMBuild.html
 ;
 ;===------------------------------------------------------------------------===;
 
 [common]
-subdirectories = TargetInfo MCTargetDesc
+subdirectories = AsmParser TargetInfo MCTargetDesc
 
 [component_0]
 type = TargetGroup
 name = RISCV
 parent = Target
+has_asmparser = 1
 
 [component_1]
 type = Library
 name = RISCVCodeGen
 parent = RISCV
 required_libraries = Core CodeGen RISCVInfo Support Target
 add_to_library_groups = RISCV

lib/Target/RISCV/RISCV.td

 
 def Feature64Bit   : SubtargetFeature<"64bit", "HasRV64", "true",
                                       "Implements RV64">;
 
 def : ProcessorModel<"generic-rv32", NoSchedModel, []>;
 
 def : ProcessorModel<"generic-rv64", NoSchedModel, [Feature64Bit]>;
 
+def RISCVAsmParser : AsmParser {
+  let ShouldEmitMatchRegisterAltName = 1;
+}
+
 def RISCV : Target {
   let InstructionSet = RISCVInstrInfo;
+  let AssemblyParsers = [RISCVAsmParser];
 }

lib/Target/RISCV/RISCVInstrInfo.td

 //===-- RISCVInstrInfo.td - Target Description for RISCV ---*- tablegen -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file describes the RISC-V instructions in TableGen format.
 //
 //===----------------------------------------------------------------------===//
 
 include "RISCVInstrFormats.td"
 
-def simm12 : Operand<i32>;
+class SImmAsmOperand<int width>
+  : AsmOperandClass {
+  let Name = "SImm" # width;
+  let RenderMethod = "addImmOperands";
+  let DiagnosticType = !strconcat("Invalid", Name);
+}
+
+def simm12 : Operand<i32> {
+  let ParserMatchClass = SImmAsmOperand<12>;
+}
 
 // As noted in RISCVRegisterInfo.td, the hope is that support for
 // variable-sized register classes will mean that instruction definitions do
 // not need to be duplicated for 32-bit and 64-bit register classes. For now
 // we use 'GPR', which is 32-bit. When codegen for both RV32 and RV64 is
 // added, we will need to duplicate instruction definitions unless a proposal
 // like <http://lists.llvm.org/pipermail/llvm-dev/2016-September/105027.html>
 // is adopted.