[RISCV][MC] Improve parsing of jal/j operands

[asb]

Summary

Examples such as jal a3, j a3 and jal a3, a3 were previously accepted by gas but rejected by LLVM MC. This patch rectifies this. I introduce RISCVAsmParser::parseJALOffset to ensure that symbol names that coincide with register names can safely be parsed. This is made a somewhat fiddly due to the single-operand alias form (see the comment in parseJALOffset for more info).

[apazos]

I ran the fuzzer with this patch. No issues detected, LGTM.

lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

 #include "RISCVGenAsmMatcher.inc"
 
   OperandMatchResultTy parseImmediate(OperandVector &Operands);
   OperandMatchResultTy parseRegister(OperandVector &Operands,
                                      bool AllowParens = false);
   OperandMatchResultTy parseMemOpBaseReg(OperandVector &Operands);
   OperandMatchResultTy parseOperandWithModifier(OperandVector &Operands);
   OperandMatchResultTy parseBareSymbol(OperandVector &Operands);
+  OperandMatchResultTy parseJALOffset(OperandVector &Operands);
 
   bool parseOperand(OperandVector &Operands, StringRef Mnemonic);
 
   bool parseDirectiveOption();
 
   void setFeatureBits(uint64_t Feature, StringRef FeatureString) {
     if (!(getSTI().getFeatureBits()[Feature])) {
       MCSubtargetInfo &STI = copySTI();
if (!IsConstantImm) {
       IsValid = RISCVAsmParser::classifySymbolRef(getImm(), VK, Imm);
       return IsValid && VK == RISCVMCExpr::VK_RISCV_PCREL_HI;
     } else {
       return isUInt<20>(Imm) && (VK == RISCVMCExpr::VK_RISCV_None ||
                                  VK == RISCVMCExpr::VK_RISCV_PCREL_HI);
     }
   }
 
-  bool isSImm21Lsb0() const { return isBareSimmNLsb0<21>(); }
+  bool isSImm21Lsb0JAL() const { return isBareSimmNLsb0<21>(); }
 
   /// 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; }
   /// True if this operand is for an RV64 instruction
   bool isRV64() const { return IsRV64; }
 
case Match_InvalidUImm20LUI:
     return generateImmOutOfRangeError(Operands, ErrorInfo, 0, (1 << 20) - 1,
                                       "operand must be a symbol with %hi() "
                                       "modifier or an integer in the range");
   case Match_InvalidUImm20AUIPC:
     return generateImmOutOfRangeError(
         Operands, ErrorInfo, 0, (1 << 20) - 1,
         "operand must be a symbol with %pcrel_hi() modifier or an integer in "
         "the range");
-  case Match_InvalidSImm21Lsb0:
+  case Match_InvalidSImm21Lsb0JAL:
     return generateImmOutOfRangeError(
         Operands, ErrorInfo, -(1 << 20), (1 << 20) - 2,
         "immediate must be a multiple of 2 bytes in the range");
   case Match_InvalidFenceArg: {
     SMLoc ErrorLoc = ((RISCVOperand &)*Operands[ErrorInfo]).getStartLoc();
     return Error(
         ErrorLoc,
         "operand must be formed of letters selected in-order from 'iorw'");
if (getParser().parseIdentifier(Identifier))
     return MatchOperand_ParseFail;
 
   MCSymbol *Sym = getContext().getOrCreateSymbol(Identifier);
   Res = MCSymbolRefExpr::create(Sym, MCSymbolRefExpr::VK_None, getContext());
   Operands.push_back(RISCVOperand::createImm(Res, S, E, isRV64()));
   return MatchOperand_Success;
 }
 
+OperandMatchResultTy RISCVAsmParser::parseJALOffset(OperandVector &Operands) {
+  // Parsing jal operands is fiddly due to the `jal foo` and `jal ra, foo`
+  // both being acceptable forms. When parsing `jal ra, foo` this function
+  // will be called for the `ra` register operand in an attempt to match the
+  // single-operand alias. parseJALOffset must fail for this case. It would
+  // seem logical to try parse the operand using parseImmediate and return
+  // NoMatch if the next token is a comma (meaning we must be parsing a jal in
+  // the second form rather than the first). We can't do this as there's no
+  // way of rewinding the lexer state. Instead, return NoMatch if this operand
+  // is an identifier and is followed by a comma.
+  if (getLexer().is(AsmToken::Identifier) &&
+      getLexer().peekTok().is(AsmToken::Comma))
+    return MatchOperand_NoMatch;
+
+  return parseImmediate(Operands);
+}
+
 OperandMatchResultTy
 RISCVAsmParser::parseMemOpBaseReg(OperandVector &Operands) {
   if (getLexer().isNot(AsmToken::LParen)) {
     Error(getLoc(), "expected '('");
     return MatchOperand_ParseFail;
   }
 
   getParser().Lex(); // Eat '('

lib/Target/RISCV/RISCVInstrInfo.td

 
 def uimm20_lui : UImm20Operand {
   let ParserMatchClass = UImmAsmOperand<20, "LUI">;
 }
 def uimm20_auipc : UImm20Operand {
   let ParserMatchClass = UImmAsmOperand<20, "AUIPC">;
 }
 
+def Simm21Lsb0JALAsmOperand : SImmAsmOperand<21, "Lsb0JAL"> {
+  let ParserMethod = "parseJALOffset";
+}
+
 // A 21-bit signed immediate where the least significant bit is zero.
-def simm21_lsb0 : Operand<OtherVT> {
-  let ParserMatchClass = SImmAsmOperand<21, "Lsb0">;
+def simm21_lsb0_jal : Operand<OtherVT> {
+  let ParserMatchClass = Simm21Lsb0JALAsmOperand;
   let EncoderMethod = "getImmOpValueAsr1";
   let DecoderMethod = "decodeSImmOperandAndLsl1<21>";
   let MCOperandPredicate = [{
     int64_t Imm;
     if (MCOp.evaluateAsConstantImm(Imm))
       return isShiftedInt<20, 1>(Imm);
     return MCOp.isBareSymbolRef();
   }];

 let hasSideEffects = 0, isReMaterializable = 1, mayLoad = 0, mayStore = 0 in {
 def LUI : RVInstU<OPC_LUI, (outs GPR:$rd), (ins uimm20_lui:$imm20),
                   "lui", "$rd, $imm20">;
 
 def AUIPC : RVInstU<OPC_AUIPC, (outs GPR:$rd), (ins uimm20_auipc:$imm20),
                     "auipc", "$rd, $imm20">;
 
 let isCall = 1 in
-def JAL : RVInstJ<OPC_JAL, (outs GPR:$rd), (ins simm21_lsb0:$imm20),
+def JAL : RVInstJ<OPC_JAL, (outs GPR:$rd), (ins simm21_lsb0_jal:$imm20),
                   "jal", "$rd, $imm20">;
 
 let isCall = 1 in
 def JALR : RVInstI<0b000, OPC_JALR, (outs GPR:$rd),
                    (ins GPR:$rs1, simm12:$imm12),
                    "jalr", "$rd, $rs1, $imm12">;
 } // hasSideEffects = 0, mayLoad = 0, mayStore = 0
 

 def : InstAlias<"ble $rs, $rt, $offset",
                 (BGE  GPR:$rt, GPR:$rs, simm13_lsb0:$offset), 0>;
 def : InstAlias<"bgtu $rs, $rt, $offset",
                 (BLTU GPR:$rt, GPR:$rs, simm13_lsb0:$offset), 0>;
 def : InstAlias<"bleu $rs, $rt, $offset",
                 (BGEU GPR:$rt, GPR:$rs, simm13_lsb0:$offset), 0>;
 
 // "ret" has more weight since "ret" and "jr" alias the same "jalr" instruction.
-def : InstAlias<"j $offset",   (JAL  X0, simm21_lsb0:$offset)>;
-def : InstAlias<"jal $offset", (JAL  X1, simm21_lsb0:$offset)>;
+def : InstAlias<"j $offset",   (JAL  X0, simm21_lsb0_jal:$offset)>;
+def : InstAlias<"jal $offset", (JAL  X1, simm21_lsb0_jal:$offset)>;
 def : InstAlias<"jr $rs",      (JALR X0, GPR:$rs, 0)>;
 def : InstAlias<"jalr $rs",    (JALR X1, GPR:$rs, 0)>;
 def : InstAlias<"ret",         (JALR X0,      X1, 0), 2>;
 // TODO call
 // TODO tail
 
 def : InstAlias<"fence", (FENCE 0xF, 0xF)>; // 0xF == iorw
 

 def : BccSwapPat<setugt, BLTU>;
 def : BccSwapPat<setule, BGEU>;
 
 // An extra pattern is needed for a brcond without a setcc (i.e. where the
 // condition was calculated elsewhere).
 def : Pat<(brcond GPR:$cond, bb:$imm12), (BNE GPR:$cond, X0, bb:$imm12)>;
 
 let isBarrier = 1, isBranch = 1, isTerminator = 1 in
-def PseudoBR : Pseudo<(outs), (ins simm21_lsb0:$imm20), [(br bb:$imm20)]>,
-               PseudoInstExpansion<(JAL X0, simm21_lsb0:$imm20)>;
+def PseudoBR : Pseudo<(outs), (ins simm21_lsb0_jal:$imm20), [(br bb:$imm20)]>,
+               PseudoInstExpansion<(JAL X0, simm21_lsb0_jal:$imm20)>;
 
 let isCall = 1, Defs=[X1] in
 let isBarrier = 1, isBranch = 1, isIndirectBranch = 1, isTerminator = 1 in
 def PseudoBRIND : Pseudo<(outs), (ins GPR:$rs1, simm12:$imm12), []>,
                   PseudoInstExpansion<(JALR X0, GPR:$rs1, simm12:$imm12)>;
 
 def : Pat<(brind GPR:$rs1), (PseudoBRIND GPR:$rs1, 0)>;
 def : Pat<(brind (add GPR:$rs1, simm12:$imm12)),

test/MC/RISCV/rv32i-invalid.s

 sltiu s2, s3, 0x50, 0x60 # CHECK: :[[@LINE]]:21: error: invalid operand for instruction
 
 # Memory operand not formatted correctly
 lw a4, a5, 111 # CHECK: :[[@LINE]]:8: error: operand must be a symbol with %lo/%pcrel_lo modifier or an integer in the range [-2048, 2047]
 
 # Too few operands
 ori a0, a1 # CHECK: :[[@LINE]]:1: error: too few operands for instruction
 xor s2, s2 # CHECK: :[[@LINE]]:1: error: too few operands for instruction
-# FIXME: Fix jal behavior to interpret a3 as a symbol rather than a register.
-jal a3     # CHECK: :[[@LINE]]:1: error: too few operands for instruction
 
 # Instruction not in the base ISA
 mul a4, ra, s0 # CHECK: :[[@LINE]]:1: error: instruction use requires an option to be enabled
 amomaxu.w s5, s4, (s3) # CHECK: :[[@LINE]]:1: error: instruction use requires an option to be enabled
 fadd.s ft0, ft1, ft2 # CHECK: :[[@LINE]]:1: error: instruction use requires an option to be enabled
 
 # Using floating point registers when integer registers are expected
 addi a2, ft0, 24 # CHECK: :[[@LINE]]:10: error: invalid operand for instruction
 
 # fence.tso accepts no operands
 fence.tso rw, rw # CHECK: :[[@LINE]]:11: error: invalid operand for instruction

test/MC/RISCV/rv32i-valid.s

 auipc a0, %pcrel_hi(foo)
 
 # CHECK-ASM-AND-OBJ: jal a2, 1048574
 # CHECK-ASM: encoding: [0x6f,0xf6,0xff,0x7f]
 jal a2, 1048574
 # CHECK-ASM-AND-OBJ: jal a3, 256
 # CHECK-ASM: encoding: [0xef,0x06,0x00,0x10]
 jal a3, 256
+# CHECK-ASM: jal a0, foo
+# CHECK-ASM: encoding: [0x6f,0bAAAA0101,A,A]
+# CHECK-OBJ: jal a0, 0
+# CHECK-OBJ: R_RISCV_JAL foo
+jal a0, foo
+# CHECK-ASM: jal a0, a0
+# CHECK-ASM: encoding: [0x6f,0bAAAA0101,A,A]
+# CHECK-OBJ: jal a0, 0
+# CHECK-OBJ: R_RISCV_JAL a0
+jal a0, a0
 
 # CHECK-ASM-AND-OBJ: jalr a0, a1, -2048
 # CHECK-ASM: encoding: [0x67,0x85,0x05,0x80]
 jalr a0, a1, -2048
 # CHECK-ASM-AND-OBJ: jalr a0, a1, -2048
 # CHECK-ASM: encoding: [0x67,0x85,0x05,0x80]
 jalr a0, a1, %lo(2048)
 # CHECK-ASM-AND-OBJ: jalr t2, t1, 2047

test/MC/RISCV/rvi-aliases-valid.s

 # RUN: llvm-mc %s -triple=riscv32 -riscv-no-aliases \
 # RUN:     | FileCheck -check-prefixes=CHECK-S-NOALIAS,CHECK-S-OBJ-NOALIAS %s
 # RUN: llvm-mc %s -triple=riscv32 \
 # RUN:     | FileCheck -check-prefixes=CHECK-S,CHECK-S-OBJ %s
 # RUN: llvm-mc %s -triple=riscv64 -riscv-no-aliases\
 # RUN:     | FileCheck -check-prefixes=CHECK-S-NOALIAS,CHECK-S-OBJ-NOALIAS %s
 # RUN: llvm-mc %s -triple=riscv64 \
 # RUN:     | FileCheck -check-prefixes=CHECK-S,CHECK-S-OBJ %s
 # RUN: llvm-mc -filetype=obj -triple riscv32 < %s \
-# RUN:     | llvm-objdump -d -riscv-no-aliases - \
+# RUN:     | llvm-objdump -d -r -riscv-no-aliases - \
 # RUN:     | FileCheck -check-prefixes=CHECK-OBJ-NOALIAS,CHECK-S-OBJ-NOALIAS %s
 # RUN: llvm-mc -filetype=obj -triple riscv32 < %s \
-# RUN:     | llvm-objdump -d - \
+# RUN:     | llvm-objdump -d -r - \
 # RUN:     | FileCheck -check-prefixes=CHECK-OBJ,CHECK-S-OBJ %s
 # RUN: llvm-mc -filetype=obj -triple riscv64 < %s \
-# RUN:     | llvm-objdump -d -riscv-no-aliases - \
+# RUN:     | llvm-objdump -d -r -riscv-no-aliases - \
 # RUN:     | FileCheck -check-prefixes=CHECK-OBJ-NOALIAS,CHECK-S-OBJ-NOALIAS %s
 # RUN: llvm-mc -filetype=obj -triple riscv64 < %s \
-# RUN:     | llvm-objdump -d - \
+# RUN:     | llvm-objdump -d -r - \
 # RUN:     | FileCheck -check-prefixes=CHECK-OBJ,CHECK-S-OBJ %s
 
 # The following check prefixes are used in this test:
 # CHECK-S                 Match the .s output with aliases enabled
 # CHECK-S-NOALIAS         Match the .s output with aliases disabled
 # CHECK-OBJ               Match the objdumped object output with aliases enabled
 # CHECK-OBJ-NOALIAS       Match the objdumped object output with aliases enabled
 # CHECK-S-OBJ             Match both the .s and objdumped object output with

 bgtu x17, x18, 28
 # CHECK-S-OBJ-NOALIAS: bgeu s3, s2, 32
 # CHECK-S-OBJ: bgeu s3, s2, 32
 bleu x18, x19, 32
 
 # CHECK-S-OBJ-NOALIAS: jal zero, 2044
 # CHECK-S-OBJ: j 2044
 j 2044
+# CHECK-S-NOALIAS: jal zero, foo
+# CHECK-S: j foo
+# CHECK-OBJ-NOALIAS: jal zero, 0
+# CHECK-OBJ: j 0
+# CHECK-OBJ: R_RISCV_JAL foo
+j foo
+# CHECK-S-NOALIAS: jal zero, a0
+# CHECK-S: j a0
+# CHECK-OBJ-NOALIAS: jal zero, 0
+# CHECK-OBJ: j 0
+# CHECK-OBJ: R_RISCV_JAL a0
+j a0
 # CHECK-S-OBJ-NOALIAS: jal ra, 2040
 # CHECK-S-OBJ: jal 2040
 jal 2040
+# CHECK-S-NOALIAS: jal ra, foo
+# CHECK-S: jal foo
+# CHECK-OBJ-NOALIAS: jal ra, 0
+# CHECK-OBJ: jal 0
+# CHECK-OBJ: R_RISCV_JAL foo
+jal foo
+# CHECK-S-NOALIAS: jal ra, a0
+# CHECK-S: jal a0
+# CHECK-OBJ-NOALIAS: jal ra, 0
+# CHECK-OBJ: jal 0
+# CHECK-OBJ: R_RISCV_JAL a0
+jal a0
 # CHECK-S-OBJ-NOALIAS: jalr zero, s4, 0
 # CHECK-S-OBJ: jr s4
 jr x20
 # CHECK-S-OBJ-NOALIAS: jalr ra, s5, 0
 # CHECK-S-OBJ: jalr s5
 jalr x21
 # CHECK-S-OBJ-NOALIAS: jalr zero, ra, 0
 # CHECK-S-OBJ: ret