[RISCV][MC] Use a custom ParserMethod for the bare_symbol operand type

[asb]

Summary

This allows the hard-coded shouldForceImmediate logic to be removed because the generated MatchOperandParserImpl makes use of the current context (i.e. the current mnemonic) to determine parsing behaviour, and so won't first try to parse a register before parsing a symbol name.

No functional change is intended. gas accepts immediate arguments for call, tail and lla. This patch doesn't address this discrepancy.

[rogfer01]

Oh this is great! Scales much better than my earlier hack. Thanks a lot @asb !

I think I will be able to apply something similar with foo@plt symbols. Currently I have a nasty hack in parseImmediate

LGTM

[apazos]

LGTM, I also verified this patch with the fuzzer tool.

llvm/trunk/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

 #define GET_ASSEMBLER_HEADER
 #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);
 
-  bool parseOperand(OperandVector &Operands, bool ForceImmediate);
+  bool parseOperand(OperandVector &Operands, StringRef Mnemonic);
 
   bool parseDirectiveOption();
 
   void setFeatureBits(uint64_t Feature, StringRef FeatureString) {
     if (!(getSTI().getFeatureBits()[Feature])) {
       MCSubtargetInfo &STI = copySTI();
       setAvailableFeatures(
           ComputeAvailableFeatures(STI.ToggleFeature(FeatureString)));
if (getParser().parseParenExpression(SubExpr, E)) {
     return MatchOperand_ParseFail;
   }
 
   const MCExpr *ModExpr = RISCVMCExpr::create(SubExpr, VK, getContext());
   Operands.push_back(RISCVOperand::createImm(ModExpr, S, E, isRV64()));
   return MatchOperand_Success;
 }
 
+OperandMatchResultTy RISCVAsmParser::parseBareSymbol(OperandVector &Operands) {
+  SMLoc S = getLoc();
+  SMLoc E = SMLoc::getFromPointer(S.getPointer() - 1);
+  const MCExpr *Res;
+
+  if (getLexer().getKind() != AsmToken::Identifier)
+    return MatchOperand_NoMatch;
+
+  StringRef Identifier;
+  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::parseMemOpBaseReg(OperandVector &Operands) {
   if (getLexer().isNot(AsmToken::LParen)) {
     Error(getLoc(), "expected '('");
     return MatchOperand_ParseFail;
   }
 
   getParser().Lex(); // Eat '('
RISCVAsmParser::parseMemOpBaseReg(OperandVector &Operands) {
   getParser().Lex(); // Eat ')'
   Operands.push_back(RISCVOperand::createToken(")", getLoc(), isRV64()));
 
   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. If ForceImmediate is true, no attempt will be made to parse the
-/// operand as a register, which is needed for pseudoinstructions such as
-/// call.
-bool RISCVAsmParser::parseOperand(OperandVector &Operands,
-                                  bool ForceImmediate) {
-  // Attempt to parse token as register, unless ForceImmediate.
-  if (!ForceImmediate && parseRegister(Operands, true) == MatchOperand_Success)
+/// true.
+bool RISCVAsmParser::parseOperand(OperandVector &Operands, StringRef Mnemonic) {
+  // Check if the current operand has a custom associated parser, if so, try to
+  // custom parse the operand, or fallback to the general approach.
+  OperandMatchResultTy Result =
+      MatchOperandParserImpl(Operands, Mnemonic, /*ParseForAllFeatures=*/true);
+  if (Result == MatchOperand_Success)
+    return false;
+  if (Result == MatchOperand_ParseFail)
+    return true;
+
+  // Attempt to parse token as a register.
+  if (parseRegister(Operands, true) == MatchOperand_Success)
     return false;
 
   // Attempt to parse token as an immediate
   if (parseImmediate(Operands) == MatchOperand_Success) {
     // Parse memory base register if present
     if (getLexer().is(AsmToken::LParen))
       return parseMemOpBaseReg(Operands) != MatchOperand_Success;
     return false;
   }
 
   // Finally we have exhausted all options and must declare defeat.
   Error(getLoc(), "unknown operand");
   return true;
 }
 
-/// Return true if the operand at the OperandIdx for opcode Name should be
-/// 'forced' to be parsed as an immediate. This is required for
-/// pseudoinstructions such as tail or call, which allow bare symbols to be used
-/// that could clash with register names.
-static bool shouldForceImediateOperand(StringRef Name, unsigned OperandIdx) {
-  // FIXME: This may not scale so perhaps we want to use a data-driven approach
-  // instead.
-  switch (OperandIdx) {
-  case 0:
-    // call imm
-    // tail imm
-    return Name == "tail" || Name == "call";
-  case 1:
-    // lla rdest, imm
-    return Name == "lla";
-  default:
-    return false;
-  }
-}
-
 bool RISCVAsmParser::ParseInstruction(ParseInstructionInfo &Info,
                                       StringRef Name, SMLoc NameLoc,
                                       OperandVector &Operands) {
   // First operand is token for instruction
   Operands.push_back(RISCVOperand::createToken(Name, NameLoc, isRV64()));
 
   // If there are no more operands, then finish
   if (getLexer().is(AsmToken::EndOfStatement))
     return false;
 
   // Parse first operand
-  if (parseOperand(Operands, shouldForceImediateOperand(Name, 0)))
+  if (parseOperand(Operands, Name))
     return true;
 
   // Parse until end of statement, consuming commas between operands
   unsigned OperandIdx = 1;
   while (getLexer().is(AsmToken::Comma)) {
     // Consume comma token
     getLexer().Lex();
 
     // Parse next operand
-    if (parseOperand(Operands, shouldForceImediateOperand(Name, OperandIdx)))
+    if (parseOperand(Operands, Name))
       return true;
 
     ++OperandIdx;
   }
 
   if (getLexer().isNot(AsmToken::EndOfStatement)) {
     SMLoc Loc = getLexer().getLoc();
     getParser().eatToEndOfStatement();

llvm/trunk/lib/Target/RISCV/RISCVInstrInfo.td

let MCOperandPredicate = [{
     return MCOp.isBareSymbolRef();
   }];
 }
 
 def BareSymbol : AsmOperandClass {
   let Name = "BareSymbol";
   let RenderMethod = "addImmOperands";
   let DiagnosticType = "InvalidBareSymbol";
+  let ParserMethod = "parseBareSymbol";
 }
 
 // A bare symbol.
 def bare_symbol : Operand<XLenVT> {
   let ParserMatchClass = BareSymbol;
-  let MCOperandPredicate = [{
-     return MCOp.isBareSymbolRef();
-  }];
 }
 
 // A parameterized register class alternative to i32imm/i64imm from Target.td.
 def ixlenimm : Operand<XLenVT> {
   let ParserMatchClass = ImmXLenAsmOperand<"">;
 }
 
 // Standalone (codegen-only) immleaf patterns.