Skip to content

[ARM] Fix problems with register list in vscclrm #111825

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.

Sign up for GitHub

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 2 commits into from
Oct 17, 2024
Merged

[ARM] Fix problems with register list in vscclrm #111825

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d1041be
[ARM] Fix problems with register list in vscclrm
john-brawn-arm Oct 8, 2024
d2749af
Allow double-precision registers in VSCCLRMS
john-brawn-arm Oct 14, 2024
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
15 changes: 11 additions & 4 deletions llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)) {
Expand All @@ -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");

Expand Down Expand Up @@ -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);
}
Expand Down
34 changes: 20 additions & 14 deletions llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
8 changes: 8 additions & 0 deletions llvm/test/MC/ARM/vlstm-vlldm-diag.s
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
30 changes: 30 additions & 0 deletions llvm/test/MC/ARM/vscclrm-asm.s
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}
39 changes: 37 additions & 2 deletions llvm/test/MC/Disassembler/ARM/vscclrm.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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}
Expand Down Expand Up @@ -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.
Copy link
Collaborator

@statham-arm statham-arm Oct 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think VSCCLRM is even more unusual than you say.

Reading the ARMARM for the single-precision version of VSCCLRM, the validity criteria are:

topReg = d+regs-1;
if topReg > 63 then UNPREDICTABLE;
if topReg > 31 && topReg[0] == '0' then UNPREDICTABLE;

which suggests that you're supposed to be able to go past s31 into the region past the end of the s-registers where d16-d31 live, and continue clearing d-registers, as long as you don't stop half way through one. Confirmed by the text about <sreglist>: "Registers above S31 are specified by using D registers in the register list."

I think that, for example, [0xdf,0xec,0x07,0xea] ought to disassemble as vscclrm {s29, s30, s31, d16, d17, vpr} with no unpredictability warning: clearing 7 s-register sized pieces starting at s29 means you overrun the end of the s regs by 4, i.e. by 2 d-regs.


[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]
Loading