Skip to content

Commit

Permalink
[RISCV][MC] Use a custom ParserMethod for the bare_symbol operand type
Browse files Browse the repository at this point in the history
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.

Differential Revision: https://reviews.llvm.org/D51733

llvm-svn: 342488
  • Loading branch information
asb committed Sep 18, 2018
1 parent 7d0e18d commit 68f73c1
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 33 deletions.
65 changes: 35 additions & 30 deletions llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
Expand Up @@ -91,8 +91,9 @@ class RISCVAsmParser : public MCTargetAsmParser {
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();

Expand Down Expand Up @@ -966,6 +967,24 @@ RISCVAsmParser::parseOperandWithModifier(OperandVector &Operands) {
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)) {
Expand Down Expand Up @@ -994,13 +1013,19 @@ RISCVAsmParser::parseMemOpBaseReg(OperandVector &Operands) {

/// 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
Expand All @@ -1016,26 +1041,6 @@ bool RISCVAsmParser::parseOperand(OperandVector &Operands,
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) {
Expand All @@ -1047,7 +1052,7 @@ bool RISCVAsmParser::ParseInstruction(ParseInstructionInfo &Info,
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
Expand All @@ -1057,7 +1062,7 @@ bool RISCVAsmParser::ParseInstruction(ParseInstructionInfo &Info,
getLexer().Lex();

// Parse next operand
if (parseOperand(Operands, shouldForceImediateOperand(Name, OperandIdx)))
if (parseOperand(Operands, Name))
return true;

++OperandIdx;
Expand Down
4 changes: 1 addition & 3 deletions llvm/lib/Target/RISCV/RISCVInstrInfo.td
Expand Up @@ -178,14 +178,12 @@ 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.
Expand Down

0 comments on commit 68f73c1

Please sign in to comment.