[llvm] [ARM] Fix problems with register list in vscclrm (PR #111825)
via llvm-commits
llvm-commits at lists.llvm.org
Thu Oct 10 05:11:35 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-backend-arm
Author: John Brawn (john-brawn-arm)
<details>
<summary>Changes</summary>
The register list in vscclrm is unusual in two ways:
* The encoded size can be zero, meaning the list contains only vpr.
* Double-precision registers past d15 are permitted even when the subtarget doesn't have them, they are instead ignored when the instruction executes.
Fixing this also incidentally changes a vlldm/vlstm error message: when the first register is in the range d16-d31 we now get the "operand must be exactly..." error instead of "register expected".
---
Full diff: https://github.com/llvm/llvm-project/pull/111825.diff
6 Files Affected:
- (modified) llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp (+11-4)
- (modified) llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp (+20-14)
- (modified) llvm/lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp (+1-1)
- (modified) llvm/test/MC/ARM/vlstm-vlldm-diag.s (+8)
- (modified) llvm/test/MC/ARM/vscclrm-asm.s (+30)
- (modified) llvm/test/MC/Disassembler/ARM/vscclrm.txt (+37-2)
``````````diff
diff --git a/llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp b/llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
index 75fb90477f8854..1cf9844be2695c 100644
--- a/llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
+++ b/llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
@@ -3810,6 +3810,10 @@ class ARMOperand : public MCParsedAsmOperand {
Kind = k_FPSRegisterListWithVPR;
else
Kind = k_SPRRegisterList;
+ } else if (Regs.front().second == ARM::VPR) {
+ assert(Regs.size() == 1 &&
+ "Register list starting with VPR expected to only contain VPR");
+ Kind = k_FPSRegisterListWithVPR;
}
if (Kind == k_RegisterList && Regs.back().second == ARM::APSR)
@@ -4617,15 +4621,15 @@ bool ARMAsmParser::parseRegisterList(OperandVector &Operands, bool EnforceOrder,
// Check the first register in the list to see what register class
// this is a list of.
- MCRegister Reg = tryParseRegister();
+ MCRegister Reg = tryParseRegister(AllowOutOfBoundReg);
if (!Reg)
return Error(RegLoc, "register expected");
if (!AllowRAAC && Reg == ARM::RA_AUTH_CODE)
return Error(RegLoc, "pseudo-register not allowed");
- // The reglist instructions have at most 16 registers, so reserve
+ // The reglist instructions have at most 32 registers, so reserve
// space for that many.
int EReg = 0;
- SmallVector<std::pair<unsigned, MCRegister>, 16> Registers;
+ SmallVector<std::pair<unsigned, MCRegister>, 32> Registers;
// Allow Q regs and just interpret them as the two D sub-registers.
if (ARMMCRegisterClasses[ARM::QPRRegClassID].contains(Reg)) {
@@ -4644,6 +4648,8 @@ bool ARMAsmParser::parseRegisterList(OperandVector &Operands, bool EnforceOrder,
RC = &ARMMCRegisterClasses[ARM::SPRRegClassID];
else if (ARMMCRegisterClasses[ARM::GPRwithAPSRnospRegClassID].contains(Reg))
RC = &ARMMCRegisterClasses[ARM::GPRwithAPSRnospRegClassID];
+ else if (Reg == ARM::VPR)
+ RC = &ARMMCRegisterClasses[ARM::FPWithVPRRegClassID];
else
return Error(RegLoc, "invalid register in register list");
@@ -6335,7 +6341,8 @@ bool ARMAsmParser::parseOperand(OperandVector &Operands, StringRef Mnemonic) {
case AsmToken::LBrac:
return parseMemory(Operands);
case AsmToken::LCurly: {
- bool AllowOutOfBoundReg = Mnemonic == "vlldm" || Mnemonic == "vlstm";
+ bool AllowOutOfBoundReg =
+ Mnemonic == "vlldm" || Mnemonic == "vlstm" || Mnemonic == "vscclrm";
return parseRegisterList(Operands, !Mnemonic.starts_with("clr"), false,
AllowOutOfBoundReg);
}
diff --git a/llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp b/llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp
index 93b74905fc59fc..fb08309aa3ab2e 100644
--- a/llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp
+++ b/llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp
@@ -1528,15 +1528,19 @@ static const uint16_t DPRDecoderTable[] = {
ARM::D28, ARM::D29, ARM::D30, ARM::D31
};
-static DecodeStatus DecodeDPRRegisterClass(MCInst &Inst, unsigned RegNo,
- uint64_t Address,
- const MCDisassembler *Decoder) {
+// Does this instruction/subtarget permit use of registers d16-d31?
+static bool PermitsD32(const MCInst &Inst, const MCDisassembler *Decoder) {
+ if (Inst.getOpcode() == ARM::VSCCLRMD)
+ return true;
const FeatureBitset &featureBits =
((const MCDisassembler*)Decoder)->getSubtargetInfo().getFeatureBits();
+ return featureBits[ARM::FeatureD32];
+}
- bool hasD32 = featureBits[ARM::FeatureD32];
-
- if (RegNo > 31 || (!hasD32 && RegNo > 15))
+static DecodeStatus DecodeDPRRegisterClass(MCInst &Inst, unsigned RegNo,
+ uint64_t Address,
+ const MCDisassembler *Decoder) {
+ if (RegNo > (PermitsD32(Inst, Decoder) ? 31 : 15))
return MCDisassembler::Fail;
unsigned Register = DPRDecoderTable[RegNo];
@@ -1815,10 +1819,11 @@ static DecodeStatus DecodeDPRRegListOperand(MCInst &Inst, unsigned Val,
unsigned regs = fieldFromInstruction(Val, 1, 7);
// In case of unpredictable encoding, tweak the operands.
- if (regs == 0 || regs > 16 || (Vd + regs) > 32) {
- regs = Vd + regs > 32 ? 32 - Vd : regs;
+ unsigned MaxReg = PermitsD32(Inst, Decoder) ? 32 : 16;
+ if (regs == 0 || (Vd + regs) > MaxReg) {
+ regs = Vd + regs > MaxReg ? MaxReg - Vd : regs;
regs = std::max( 1u, regs);
- regs = std::min(16u, regs);
+ regs = std::min(MaxReg, regs);
S = MCDisassembler::SoftFail;
}
@@ -6446,16 +6451,17 @@ static DecodeStatus DecodeVSCCLRM(MCInst &Inst, unsigned Insn, uint64_t Address,
Inst.addOperand(MCOperand::createImm(ARMCC::AL));
Inst.addOperand(MCOperand::createReg(0));
- if (Inst.getOpcode() == ARM::VSCCLRMD) {
- unsigned reglist = (fieldFromInstruction(Insn, 1, 7) << 1) |
- (fieldFromInstruction(Insn, 12, 4) << 8) |
+ unsigned regs = fieldFromInstruction(Insn, 0, 8);
+ if (regs == 0) {
+ // Register list contains only VPR
+ } else if (Inst.getOpcode() == ARM::VSCCLRMD) {
+ unsigned reglist = regs | (fieldFromInstruction(Insn, 12, 4) << 8) |
(fieldFromInstruction(Insn, 22, 1) << 12);
if (!Check(S, DecodeDPRRegListOperand(Inst, reglist, Address, Decoder))) {
return MCDisassembler::Fail;
}
} else {
- unsigned reglist = fieldFromInstruction(Insn, 0, 8) |
- (fieldFromInstruction(Insn, 22, 1) << 8) |
+ unsigned reglist = regs | (fieldFromInstruction(Insn, 22, 1) << 8) |
(fieldFromInstruction(Insn, 12, 4) << 9);
if (!Check(S, DecodeSPRRegListOperand(Inst, reglist, Address, Decoder))) {
return MCDisassembler::Fail;
diff --git a/llvm/lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp b/llvm/lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp
index 92427b41f0bb3d..53dad96a00f6de 100644
--- a/llvm/lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp
+++ b/llvm/lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp
@@ -1743,7 +1743,7 @@ getRegisterListOpValue(const MCInst &MI, unsigned Op,
unsigned Binary = 0;
- if (SPRRegs || DPRRegs) {
+ if (SPRRegs || DPRRegs || Reg == ARM::VPR) {
// VLDM/VSTM/VSCCLRM
unsigned RegNo = CTX.getRegisterInfo()->getEncodingValue(Reg);
unsigned NumRegs = (MI.getNumOperands() - Op) & 0xff;
diff --git a/llvm/test/MC/ARM/vlstm-vlldm-diag.s b/llvm/test/MC/ARM/vlstm-vlldm-diag.s
index b57f535c6a25cf..7aa48b96ff2f69 100644
--- a/llvm/test/MC/ARM/vlstm-vlldm-diag.s
+++ b/llvm/test/MC/ARM/vlstm-vlldm-diag.s
@@ -36,6 +36,14 @@ vlldm r8, {d3 - d31}
// ERR: error: operand must be exactly {d0-d15} (T1) or {d0-d31} (T2)
// ERR-NEXT: vlldm r8, {d3 - d31}
+vlstm r8, {d31}
+// ERR: error: operand must be exactly {d0-d15} (T1) or {d0-d31} (T2)
+// ERR-NEXT: vlstm r8, {d31}
+
+vlldm r8, {d31}
+// ERR: error: operand must be exactly {d0-d15} (T1) or {d0-d31} (T2)
+// ERR-NEXT: vlldm r8, {d31}
+
vlstm r8, {d0 - d35}
// ERR: error: register expected
// ERR-NEXT: vlstm r8, {d0 - d35}
diff --git a/llvm/test/MC/ARM/vscclrm-asm.s b/llvm/test/MC/ARM/vscclrm-asm.s
index 0989b38b07c06e..94d6d3c51dd8da 100644
--- a/llvm/test/MC/ARM/vscclrm-asm.s
+++ b/llvm/test/MC/ARM/vscclrm-asm.s
@@ -35,11 +35,41 @@ it hi
// CHECK: vscclrmhi {s3, s4, s5, s6, s7, s8, s9, s10, s11, s12, s13, s14, s15, s16, s17, s18, s19, s20, s21, s22, s23, s24, s25, s26, s27, s28, s29, s30, s31, vpr} @ encoding: [0xdf,0xec,0x1d,0x1a]
vscclrmhi {s3-s31, vpr}
+// CHECK: vscclrm {vpr} @ encoding: [0x9f,0xec,0x00,0x0a]
+vscclrm {vpr}
+
+// CHECK: vscclrm {d0, d1, d2, d3, d4, d5, d6, d7, d8, d9, d10, d11, d12, d13, d14, d15, d16, d17, d18, d19, d20, d21, d22, d23, d24, d25, d26, d27, d28, d29, d30, d31, vpr} @ encoding: [0x9f,0xec,0x40,0x0b]
+vscclrm {d0-d31, vpr}
+
+// CHECK: vscclrm {d31, vpr} @ encoding: [0xdf,0xec,0x02,0xfb]
+vscclrm {d31, vpr}
+
// ERROR: non-contiguous register range
vscclrm {s0, s3-s4, vpr}
// ERROR: register expected
vscclrm {s32, vpr}
+// ERROR: register expected
+vscclrm {d32, vpr}
+
+// ERROR: register expected
+vscclrm {s31-s32, vpr}
+
+// ERROR: register expected
+vscclrm {d31-d32, vpr}
+
// ERROR: invalid operand for instruction
vscclrm {s0-s1}
+
+// ERROR: register list not in ascending order
+vscclrm {vpr, s0}
+
+// ERROR: register list not in ascending order
+vscclrm {vpr, s31}
+
+// ERROR: register list not in ascending order
+vscclrm {vpr, d0}
+
+// ERROR: register list not in ascending order
+vscclrm {vpr, d31}
diff --git a/llvm/test/MC/Disassembler/ARM/vscclrm.txt b/llvm/test/MC/Disassembler/ARM/vscclrm.txt
index 8a89cfb76e4a45..b7adaaf69d30a3 100644
--- a/llvm/test/MC/Disassembler/ARM/vscclrm.txt
+++ b/llvm/test/MC/Disassembler/ARM/vscclrm.txt
@@ -1,5 +1,7 @@
-# RUN: llvm-mc -disassemble -triple=thumbv8.1m.main-none-eabi -mattr=+8msecext -show-encoding %s 2>&1 | FileCheck %s
-# RUN: llvm-mc -disassemble -triple=thumbv8.1m.main-none-eabi -mattr=+mve.fp,+8msecext -show-encoding %s 2>&1 | FileCheck %s
+# RUN: llvm-mc -disassemble -triple=thumbv8.1m.main-none-eabi -mattr=+8msecext -show-encoding %s 2> %t | FileCheck %s
+# RUN: FileCheck --check-prefix=WARN < %t %s
+# RUN: llvm-mc -disassemble -triple=thumbv8.1m.main-none-eabi -mattr=+mve.fp,+8msecext -show-encoding %s 2> %t | FileCheck %s
+# RUN: FileCheck --check-prefix=WARN < %t %s
[0x9f 0xec 0x04 0x0a]
# CHECK: vscclrm {s0, s1, s2, s3, vpr}
@@ -27,3 +29,36 @@
[0xdf 0xec 0x1d 0x1a]
# CHECK: vscclrmhi {s3, s4, s5, s6, s7, s8, s9, s10, s11, s12, s13, s14, s15, s16, s17, s18, s19, s20, s21, s22, s23, s24, s25, s26, s27, s28, s29, s30, s31, vpr}
+
+# If the list size is zero then we get a list of only vpr, and the Vd register
+# doesn't matter.
+
+[0x9f,0xec,0x00,0x0b]
+# CHECK: vscclrm {vpr} @ encoding: [0x9f,0xec,0x00,0x0b]
+
+[0xdf,0xec,0x00,0xfb]
+# CHECK: vscclrm {vpr} @ encoding: [0x9f,0xec,0x00,0x0b]
+
+[0x9f,0xec,0x00,0x0a]
+# CHECK: vscclrm {vpr} @ encoding: [0x9f,0xec,0x00,0x0a]
+
+[0xdf,0xec,0x00,0xfa]
+# CHECK: vscclrm {vpr} @ encoding: [0x9f,0xec,0x00,0x0a]
+
+# If Vd+size goes past 31 the excess registers are ignored and we get a warning.
+
+[0x9f,0xec,0xfe,0x0b]
+# WARN: [[@LINE-1]]:2: warning: potentially undefined instruction encoding
+# CHECK: vscclrm {d0, d1, d2, d3, d4, d5, d6, d7, d8, d9, d10, d11, d12, d13, d14, d15, d16, d17, d18, d19, d20, d21, d22, d23, d24, d25, d26, d27, d28, d29, d30, d31, vpr} @ encoding: [0x9f,0xec,0x40,0x0b]
+
+[0xdf,0xec,0x04,0xfb]
+# WARN: [[@LINE-1]]:2: warning: potentially undefined instruction encoding
+# CHECK: vscclrm {d31, vpr} @ encoding: [0xdf,0xec,0x02,0xfb]
+
+[0x9f,0xec,0xff,0x0a]
+# WARN: [[@LINE-1]]:2: warning: potentially undefined instruction encoding
+# CHECK: vscclrm {s0, s1, s2, s3, s4, s5, s6, s7, s8, s9, s10, s11, s12, s13, s14, s15, s16, s17, s18, s19, s20, s21, s22, s23, s24, s25, s26, s27, s28, s29, s30, s31, vpr} @ encoding: [0x9f,0xec,0x20,0x0a]
+
+[0xdf,0xec,0x02,0xfa]
+# WARN: [[@LINE-1]]:2: warning: potentially undefined instruction encoding
+# CHECK: vscclrm {s31, vpr} @ encoding: [0xdf,0xec,0x01,0xfa]
``````````
</details>
https://github.com/llvm/llvm-project/pull/111825
More information about the llvm-commits
mailing list