Skip to content

[ARM] FIX: change pkhtb custom parsing produce NoMatch rather than Error #85765

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Mar 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 40 additions & 24 deletions llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
#include <iterator>
#include <limits>
#include <memory>
#include <optional>
#include <string>
#include <utility>
#include <vector>
Expand Down Expand Up @@ -457,6 +458,7 @@ class ARMAsmParser : public MCTargetAsmParser {
int tryParseRegister(bool AllowOutofBoundReg = false);
bool tryParseRegisterWithWriteBack(OperandVector &);
int tryParseShiftRegister(OperandVector &);
std::optional<ARM_AM::ShiftOpc> tryParseShiftToken();
bool parseRegisterList(OperandVector &, bool EnforceOrder = true,
bool AllowRAAC = false,
bool AllowOutOfBoundReg = false);
Expand Down Expand Up @@ -647,12 +649,13 @@ class ARMAsmParser : public MCTargetAsmParser {
ParseStatus parseProcIFlagsOperand(OperandVector &);
ParseStatus parseMSRMaskOperand(OperandVector &);
ParseStatus parseBankedRegOperand(OperandVector &);
ParseStatus parsePKHImm(OperandVector &O, StringRef Op, int Low, int High);
ParseStatus parsePKHImm(OperandVector &O, ARM_AM::ShiftOpc, int Low,
int High);
ParseStatus parsePKHLSLImm(OperandVector &O) {
return parsePKHImm(O, "lsl", 0, 31);
return parsePKHImm(O, ARM_AM::lsl, 0, 31);
}
ParseStatus parsePKHASRImm(OperandVector &O) {
return parsePKHImm(O, "asr", 1, 32);
return parsePKHImm(O, ARM_AM::asr, 1, 32);
}
ParseStatus parseSetEndImm(OperandVector &);
ParseStatus parseShifterImm(OperandVector &);
Expand Down Expand Up @@ -4228,30 +4231,36 @@ int ARMAsmParser::tryParseRegister(bool AllowOutOfBoundReg) {
return RegNum;
}

// Try to parse a shifter (e.g., "lsl <amt>"). On success, return 0.
// If a recoverable error occurs, return 1. If an irrecoverable error
// occurs, return -1. An irrecoverable error is one where tokens have been
// consumed in the process of trying to parse the shifter (i.e., when it is
// indeed a shifter operand, but malformed).
int ARMAsmParser::tryParseShiftRegister(OperandVector &Operands) {
std::optional<ARM_AM::ShiftOpc> ARMAsmParser::tryParseShiftToken() {
MCAsmParser &Parser = getParser();
SMLoc S = Parser.getTok().getLoc();
const AsmToken &Tok = Parser.getTok();
if (Tok.isNot(AsmToken::Identifier))
return -1;
return std::nullopt;

std::string lowerCase = Tok.getString().lower();
ARM_AM::ShiftOpc ShiftTy = StringSwitch<ARM_AM::ShiftOpc>(lowerCase)
return StringSwitch<std::optional<ARM_AM::ShiftOpc>>(lowerCase)
.Case("asl", ARM_AM::lsl)
.Case("lsl", ARM_AM::lsl)
.Case("lsr", ARM_AM::lsr)
.Case("asr", ARM_AM::asr)
.Case("ror", ARM_AM::ror)
.Case("rrx", ARM_AM::rrx)
.Default(ARM_AM::no_shift);
.Default(std::nullopt);
}

// Try to parse a shifter (e.g., "lsl <amt>"). On success, return 0.
// If a recoverable error occurs, return 1. If an irrecoverable error
// occurs, return -1. An irrecoverable error is one where tokens have been
// consumed in the process of trying to parse the shifter (i.e., when it is
// indeed a shifter operand, but malformed).
int ARMAsmParser::tryParseShiftRegister(OperandVector &Operands) {
MCAsmParser &Parser = getParser();
SMLoc S = Parser.getTok().getLoc();

if (ShiftTy == ARM_AM::no_shift)
auto ShiftTyOpt = tryParseShiftToken();
if (ShiftTyOpt == std::nullopt)
return 1;
auto ShiftTy = ShiftTyOpt.value();

Parser.Lex(); // Eat the operator.

Expand Down Expand Up @@ -5284,17 +5293,24 @@ ParseStatus ARMAsmParser::parseBankedRegOperand(OperandVector &Operands) {
return ParseStatus::Success;
}

ParseStatus ARMAsmParser::parsePKHImm(OperandVector &Operands, StringRef Op,
int Low, int High) {
// FIXME: Unify the different methods for handling shift operators
// and use TableGen matching mechanisms to do the validation rather than
// separate parsing paths.
ParseStatus ARMAsmParser::parsePKHImm(OperandVector &Operands,
ARM_AM::ShiftOpc Op, int Low, int High) {
MCAsmParser &Parser = getParser();
const AsmToken &Tok = Parser.getTok();
if (Tok.isNot(AsmToken::Identifier))
return Error(Parser.getTok().getLoc(), Op + " operand expected.");
StringRef ShiftName = Tok.getString();
std::string LowerOp = Op.lower();
std::string UpperOp = Op.upper();
if (ShiftName != LowerOp && ShiftName != UpperOp)
return Error(Parser.getTok().getLoc(), Op + " operand expected.");
auto ShiftCodeOpt = tryParseShiftToken();

if (!ShiftCodeOpt.has_value())
return ParseStatus::NoMatch;
auto ShiftCode = ShiftCodeOpt.value();

// The wrong shift code has been provided. Can error here as has matched the
// correct operand in this case.
if (ShiftCode != Op)
return Error(Parser.getTok().getLoc(),
ARM_AM::getShiftOpcStr(Op) + " operand expected.");

Parser.Lex(); // Eat shift type token.

// There must be a '#' and a shift amount.
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Target/ARM/MCTargetDesc/ARMAddressingModes.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ namespace ARM_AM {

inline const char *getAddrOpcStr(AddrOpc Op) { return Op == sub ? "-" : ""; }

inline const char *getShiftOpcStr(ShiftOpc Op) {
inline const StringRef getShiftOpcStr(ShiftOpc Op) {
switch (Op) {
default: llvm_unreachable("Unknown shift opc!");
case ARM_AM::asr: return "asr";
Expand Down
4 changes: 4 additions & 0 deletions llvm/test/MC/ARM/basic-arm-instructions.s
Original file line number Diff line number Diff line change
Expand Up @@ -1774,6 +1774,9 @@ Lforward:
pkhtb r2, r2, r3, asr #31
pkhtb r2, r2, r3, asr #15

it ne
pkhtbne r2, r2, r3, asr #15

@ CHECK: pkhbt r2, r2, r3 @ encoding: [0x13,0x20,0x82,0xe6]
@ CHECK: pkhbt r2, r2, r3, lsl #31 @ encoding: [0x93,0x2f,0x82,0xe6]
@ CHECK: pkhbt r2, r2, r3 @ encoding: [0x13,0x20,0x82,0xe6]
Expand All @@ -1782,6 +1785,7 @@ Lforward:
@ CHECK: pkhbt r2, r3, r2 @ encoding: [0x12,0x20,0x83,0xe6]
@ CHECK: pkhtb r2, r2, r3, asr #31 @ encoding: [0xd3,0x2f,0x82,0xe6]
@ CHECK: pkhtb r2, r2, r3, asr #15 @ encoding: [0xd3,0x27,0x82,0xe6]
@ CHECK: pkhtbne r2, r2, r3, asr #15 @ encoding: [0xd3,0x27,0x82,0x16]

@------------------------------------------------------------------------------
@ FIXME: PLD
Expand Down