Skip to content

Commit

Permalink
[ARM] FIX: Fix parsing pkhtb with a condition code
Browse files Browse the repository at this point in the history
This was broken by #83436 as in
optional operands meant when the CC operand is provided the
`parsePKHImm` parser is applied to register operands, which previously
erroneously produced an error.
  • Loading branch information
AlfieRichardsArm committed Mar 19, 2024
1 parent 5cfcd9b commit e3030f1
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 25 deletions.
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

0 comments on commit e3030f1

Please sign in to comment.