[llvm] e3030f1 - [ARM] FIX: Fix parsing `pkhtb` with a condition code
via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 19 14:11:54 PDT 2024
Author: Alfie Richards
Date: 2024-03-19T23:11:48+02:00
New Revision: e3030f1e1958a2be51822bacce764395c16e682a
URL: https://github.com/llvm/llvm-project/commit/e3030f1e1958a2be51822bacce764395c16e682a
DIFF: https://github.com/llvm/llvm-project/commit/e3030f1e1958a2be51822bacce764395c16e682a.diff
LOG: [ARM] FIX: Fix parsing `pkhtb` with a condition code
This was broken by https://github.com/llvm/llvm-project/pull/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.
Added:
Modified:
llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
llvm/lib/Target/ARM/MCTargetDesc/ARMAddressingModes.h
llvm/test/MC/ARM/basic-arm-instructions.s
Removed:
################################################################################
diff --git a/llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp b/llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
index 026d4ca06e87b9..9cfdb15a0f43d3 100644
--- a/llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
+++ b/llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
@@ -63,6 +63,7 @@
#include <iterator>
#include <limits>
#include <memory>
+#include <optional>
#include <string>
#include <utility>
#include <vector>
@@ -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);
@@ -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 &);
@@ -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.
@@ -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
diff erent 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.
diff --git a/llvm/lib/Target/ARM/MCTargetDesc/ARMAddressingModes.h b/llvm/lib/Target/ARM/MCTargetDesc/ARMAddressingModes.h
index 163360c08ffba0..28e5840fdde5b4 100644
--- a/llvm/lib/Target/ARM/MCTargetDesc/ARMAddressingModes.h
+++ b/llvm/lib/Target/ARM/MCTargetDesc/ARMAddressingModes.h
@@ -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";
diff --git a/llvm/test/MC/ARM/basic-arm-instructions.s b/llvm/test/MC/ARM/basic-arm-instructions.s
index 055f3ce316153d..84a7cf52fa30e0 100644
--- a/llvm/test/MC/ARM/basic-arm-instructions.s
+++ b/llvm/test/MC/ARM/basic-arm-instructions.s
@@ -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]
@@ -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
More information about the llvm-commits
mailing list