[RISCV][Xqccmp] Correctly Parse/Disassemble pushfp (#133188)
In the `qc.cm.pushfp` instruction, it is like `cm.pushfp` except in one
important way - `qc.cm.pushfp {ra}, -N*16` is not a valid encoding,
because this would update `s0`/`fp`/`x8` without saving it.
This change now correctly rejects this variant of the instruction, both
during parsing and during disassembly. I also implemented validation for
immediates that represent register lists (both kinds), which may help to
catch bugs in the future.diff --git a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
index 02cb1b5..3c225fb 100644
--- a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
+++ b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
@@ -215,7 +215,14 @@
ParseStatus parseGPRPair(OperandVector &Operands, bool IsRV64Inst);
ParseStatus parseFRMArg(OperandVector &Operands);
ParseStatus parseFenceArg(OperandVector &Operands);
- ParseStatus parseReglist(OperandVector &Operands);
+ ParseStatus parseReglist(OperandVector &Operands) {
+ return parseRegListCommon(Operands, /*MustIncludeS0=*/false);
+ }
+ ParseStatus parseReglistS0(OperandVector &Operands) {
+ return parseRegListCommon(Operands, /*MustIncludeS0=*/true);
+ }
+ ParseStatus parseRegListCommon(OperandVector &Operands, bool MustIncludeS0);
+
ParseStatus parseRegReg(OperandVector &Operands);
ParseStatus parseRetval(OperandVector &Operands);
ParseStatus parseZcmpStackAdj(OperandVector &Operands,
@@ -474,6 +481,9 @@
bool isSystemRegister() const { return Kind == KindTy::SystemRegister; }
bool isRegReg() const { return Kind == KindTy::RegReg; }
bool isRlist() const { return Kind == KindTy::Rlist; }
+ bool isRlistS0() const {
+ return Kind == KindTy::Rlist && Rlist.Val != RISCVZC::RA;
+ }
bool isSpimm() const { return Kind == KindTy::Spimm; }
bool isGPR() const {
@@ -2794,9 +2804,15 @@
return ParseStatus::Success;
}
-ParseStatus RISCVAsmParser::parseReglist(OperandVector &Operands) {
+ParseStatus RISCVAsmParser::parseRegListCommon(OperandVector &Operands,
+ bool MustIncludeS0) {
// Rlist: {ra [, s0[-sN]]}
// XRlist: {x1 [, x8[-x9][, x18[-xN]]]}
+
+ // When MustIncludeS0 = true (not the default) (used for `qc.cm.pushfp`) which
+ // must include `fp`/`s0` in the list:
+ // Rlist: {ra, s0[-sN]}
+ // XRlist: {x1, x8[-x9][, x18[-xN]]}
SMLoc S = getLoc();
if (parseToken(AsmToken::LCurly, "register list must start with '{'"))
@@ -2814,8 +2830,20 @@
return Error(getLoc(), "register list must start from 'ra' or 'x1'");
getLexer().Lex();
- // parse case like ,s0
- if (parseOptionalToken(AsmToken::Comma)) {
+ bool SeenComma = parseOptionalToken(AsmToken::Comma);
+
+ // There are two choices here:
+ // - `s0` is not required (usual case), so only try to parse `s0` if there is
+ // a comma
+ // - `s0` is required (qc.cm.pushfp), and so we must see the comma between
+ // `ra` and `s0` and must always try to parse `s0`, below
+ if (MustIncludeS0 && !SeenComma) {
+ Error(getLoc(), "register list must include 's0' or 'x8'");
+ return ParseStatus::Failure;
+ }
+
+ // parse case like ,s0 (knowing the comma must be there if required)
+ if (SeenComma) {
if (getLexer().isNot(AsmToken::Identifier))
return Error(getLoc(), "invalid register");
StringRef RegName = getLexer().getTok().getIdentifier();
@@ -2884,6 +2912,8 @@
auto Encode = RISCVZC::encodeRlist(RegEnd, IsEABI);
assert(Encode != RISCVZC::INVALID_RLIST);
+ if (MustIncludeS0)
+ assert(Encode != RISCVZC::RA);
Operands.push_back(RISCVOperand::createRlist(Encode, S));
return ParseStatus::Success;
diff --git a/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp b/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
index 46b0141..15f9ea0 100644
--- a/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
+++ b/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
@@ -507,6 +507,9 @@
static DecodeStatus decodeZcmpRlist(MCInst &Inst, uint32_t Imm,
uint64_t Address, const void *Decoder);
+static DecodeStatus decodeXqccmpRlistS0(MCInst &Inst, uint32_t Imm,
+ uint64_t Address, const void *Decoder);
+
static DecodeStatus decodeZcmpSpimm(MCInst &Inst, uint32_t Imm,
uint64_t Address, const void *Decoder);
@@ -612,7 +615,15 @@
static DecodeStatus decodeZcmpRlist(MCInst &Inst, uint32_t Imm,
uint64_t Address, const void *Decoder) {
- if (Imm <= 3)
+ if (Imm < RISCVZC::RA)
+ return MCDisassembler::Fail;
+ Inst.addOperand(MCOperand::createImm(Imm));
+ return MCDisassembler::Success;
+}
+
+static DecodeStatus decodeXqccmpRlistS0(MCInst &Inst, uint32_t Imm,
+ uint64_t Address, const void *Decoder) {
+ if (Imm < RISCVZC::RA_S0)
return MCDisassembler::Fail;
Inst.addOperand(MCOperand::createImm(Imm));
return MCDisassembler::Success;
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.cpp
index 1829291..d5f08ac 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.cpp
@@ -242,12 +242,12 @@
void RISCVZC::printRlist(unsigned SlistEncode, raw_ostream &OS) {
OS << "{ra";
- if (SlistEncode > 4) {
+ if (SlistEncode > RISCVZC::RA) {
OS << ", s0";
- if (SlistEncode == 15)
+ if (SlistEncode == RISCVZC::RA_S0_S11)
OS << "-s11";
- else if (SlistEncode > 5 && SlistEncode <= 14)
- OS << "-s" << (SlistEncode - 5);
+ else if (SlistEncode > RISCVZC::RA_S0 && SlistEncode <= RISCVZC::RA_S0_S11)
+ OS << "-s" << (SlistEncode - RISCVZC::RA_S0);
}
OS << "}";
}
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h
index db305b0..0ede9ba 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h
@@ -343,6 +343,8 @@
OPERAND_RVKRNUM_0_7,
OPERAND_RVKRNUM_1_10,
OPERAND_RVKRNUM_2_14,
+ OPERAND_RLIST,
+ OPERAND_RLIST_S0,
OPERAND_SPIMM,
// Operand is a 3-bit rounding mode, '111' indicates FRM register.
// Represents 'frm' argument passing to floating-point operations.
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
index 77d33f2..f333ffe 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
@@ -103,6 +103,10 @@
unsigned getRlistOpValue(const MCInst &MI, unsigned OpNo,
SmallVectorImpl<MCFixup> &Fixups,
const MCSubtargetInfo &STI) const;
+
+ unsigned getRlistS0OpValue(const MCInst &MI, unsigned OpNo,
+ SmallVectorImpl<MCFixup> &Fixups,
+ const MCSubtargetInfo &STI) const;
};
} // end anonymous namespace
@@ -616,5 +620,16 @@
assert(Imm >= 4 && "EABI is currently not implemented");
return Imm;
}
+unsigned
+RISCVMCCodeEmitter::getRlistS0OpValue(const MCInst &MI, unsigned OpNo,
+ SmallVectorImpl<MCFixup> &Fixups,
+ const MCSubtargetInfo &STI) const {
+ const MCOperand &MO = MI.getOperand(OpNo);
+ assert(MO.isImm() && "Rlist operand must be immediate");
+ auto Imm = MO.getImm();
+ assert(Imm >= 4 && "EABI is currently not implemented");
+ assert(Imm != RISCVZC::RA && "Rlist operand must include s0");
+ return Imm;
+}
#include "RISCVGenMCCodeEmitter.inc"
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
index de775f4..e9fa0c4 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
@@ -2679,6 +2679,12 @@
case RISCVOp::OPERAND_RVKRNUM_2_14:
Ok = Imm >= 2 && Imm <= 14;
break;
+ case RISCVOp::OPERAND_RLIST:
+ Ok = Imm >= RISCVZC::RA && Imm <= RISCVZC::RA_S0_S11;
+ break;
+ case RISCVOp::OPERAND_RLIST_S0:
+ Ok = Imm >= RISCVZC::RA_S0 && Imm <= RISCVZC::RA_S0_S11;
+ break;
case RISCVOp::OPERAND_SPIMM:
Ok = (Imm & 0xf) == 0;
break;
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoXqccmp.td b/llvm/lib/Target/RISCV/RISCVInstrInfoXqccmp.td
index 5bb9c1e..bee937f 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoXqccmp.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoXqccmp.td
@@ -25,6 +25,30 @@
// Operand and SDNode transformation definitions.
//===----------------------------------------------------------------------===//
+def RlistS0AsmOperand : AsmOperandClass {
+ let Name = "RlistS0";
+ let ParserMethod = "parseReglistS0";
+ let RenderMethod = "addRlistOperands";
+ let DiagnosticType = "InvalidRlistS0";
+ let DiagnosticString = "operand must be {ra, s0[-sN]} or {x1, x8[-x9][, x18[-xN]]}";
+}
+
+def rlist_s0 : RISCVOp<OtherVT> {
+ let ParserMatchClass = RlistS0AsmOperand;
+ let PrintMethod = "printRlist";
+ let DecoderMethod = "decodeXqccmpRlistS0";
+ let EncoderMethod = "getRlistS0OpValue";
+ let MCOperandPredicate = [{
+ int64_t Imm;
+ if (!MCOp.evaluateAsConstantImm(Imm))
+ return false;
+ // 0~4 invalid for `qc.cm.pushfp`
+ return isUInt<4>(Imm) && Imm >= RISCVZC::RA_S0;
+ }];
+
+ string OperandType = "OPERAND_RLIST_S0";
+}
+
//===----------------------------------------------------------------------===//
// Instruction Formats
//===----------------------------------------------------------------------===//
@@ -33,6 +57,20 @@
// Instruction Class Templates
//===----------------------------------------------------------------------===//
+class RVInstXqccmpCPPPFP<bits<5> funct5, string opcodestr,
+ DAGOperand immtype = stackadj>
+ : RVInst16<(outs), (ins rlist_s0:$rlist, immtype:$stackadj),
+ opcodestr, "$rlist, $stackadj", [], InstFormatOther> {
+ bits<4> rlist;
+ bits<16> stackadj;
+
+ let Inst{1-0} = 0b10;
+ let Inst{3-2} = stackadj{5-4};
+ let Inst{7-4} = rlist;
+ let Inst{12-8} = funct5;
+ let Inst{15-13} = 0b101;
+}
+
//===----------------------------------------------------------------------===//
// Instructions
//===----------------------------------------------------------------------===//
@@ -59,7 +97,7 @@
ReadStoreData, ReadStoreData, ReadStoreData]>;
let hasSideEffects = 0, mayLoad = 0, mayStore = 1, Uses = [X2], Defs = [X2, X8] in
-def QC_CM_PUSHFP : RVInstZcCPPP<0b11001, "qc.cm.pushfp", negstackadj>,
+def QC_CM_PUSHFP : RVInstXqccmpCPPPFP<0b11001, "qc.cm.pushfp", negstackadj>,
Sched<[WriteIALU, WriteIALU, ReadIALU, ReadStoreData, ReadStoreData,
ReadStoreData, ReadStoreData, ReadStoreData, ReadStoreData,
ReadStoreData, ReadStoreData, ReadStoreData, ReadStoreData,
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoZc.td b/llvm/lib/Target/RISCV/RISCVInstrInfoZc.td
index efed74c..3f90714 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoZc.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoZc.td
@@ -58,7 +58,7 @@
let RenderMethod = "addSpimmOperands";
}
-def rlist : Operand<OtherVT> {
+def rlist : RISCVOp<OtherVT> {
let ParserMatchClass = RlistAsmOperand;
let PrintMethod = "printRlist";
let DecoderMethod = "decodeZcmpRlist";
@@ -68,8 +68,10 @@
if (!MCOp.evaluateAsConstantImm(Imm))
return false;
// 0~3 Reserved for EABI
- return isUInt<4>(Imm) && Imm >= 4;
+ return isUInt<4>(Imm) && Imm >= RISCVZC::RA;
}];
+
+ let OperandType = "OPERAND_RLIST";
}
def stackadj : RISCVOp<OtherVT> {
diff --git a/llvm/test/MC/Disassembler/RISCV/xqccmp-invalid-rlist.txt b/llvm/test/MC/Disassembler/RISCV/xqccmp-invalid-rlist.txt
new file mode 100644
index 0000000..fe81c01
--- /dev/null
+++ b/llvm/test/MC/Disassembler/RISCV/xqccmp-invalid-rlist.txt
@@ -0,0 +1,11 @@
+# RUN: not llvm-mc -disassemble -triple=riscv32 -mattr=+experimental-xqccmp %s \
+# RUN: | FileCheck -check-prefixes=CHECK,CHECK-XQCCMP %s
+
+[0x00,0x00]
+# CHECK: unimp
+
+[0x42,0xb9]
+# CHECK-XQCCMP-NOT: qc.cm.pushfp {ra}, -{{[0-9]+}}
+
+[0x00,0x00]
+# CHECK: unimp
diff --git a/llvm/test/MC/RISCV/rv32xqccmp-invalid.s b/llvm/test/MC/RISCV/rv32xqccmp-invalid.s
index 74f96f0..059009d 100644
--- a/llvm/test/MC/RISCV/rv32xqccmp-invalid.s
+++ b/llvm/test/MC/RISCV/rv32xqccmp-invalid.s
@@ -33,3 +33,7 @@
# CHECK-ERROR: error: stack adjustment for register list must be a multiple of 16 bytes in the range [16, 64]
qc.cm.pop {ra, s0-s1}, -40
+
+# CHECK-ERROR: error: register list must include 's0' or 'x8'
+qc.cm.pushfp {ra}, -16
+
diff --git a/llvm/test/MC/RISCV/rv32xqccmp-valid.s b/llvm/test/MC/RISCV/rv32xqccmp-valid.s
index 5827777..b1f373e 100644
--- a/llvm/test/MC/RISCV/rv32xqccmp-valid.s
+++ b/llvm/test/MC/RISCV/rv32xqccmp-valid.s
@@ -288,14 +288,6 @@
# CHECK-ASM: encoding: [0xfe,0xb8]
qc.cm.push {x1, x8-x9, x18-x27}, -112
-# CHECK-ASM-AND-OBJ: qc.cm.pushfp {ra}, -16
-# CHECK-ASM: encoding: [0x42,0xb9]
-qc.cm.pushfp {ra}, -16
-
-# CHECK-ASM-AND-OBJ: qc.cm.pushfp {ra}, -16
-# CHECK-ASM: encoding: [0x42,0xb9]
-qc.cm.pushfp {x1}, -16
-
# CHECK-ASM-AND-OBJ: qc.cm.pushfp {ra, s0}, -32
# CHECK-ASM: encoding: [0x56,0xb9]
qc.cm.pushfp {ra, s0}, -32
diff --git a/llvm/test/MC/RISCV/rv64e-xqccmp-valid.s b/llvm/test/MC/RISCV/rv64e-xqccmp-valid.s
index 8f9e3ce..003f985 100644
--- a/llvm/test/MC/RISCV/rv64e-xqccmp-valid.s
+++ b/llvm/test/MC/RISCV/rv64e-xqccmp-valid.s
@@ -72,10 +72,6 @@
# CHECK-ASM: encoding: [0x62,0xb8]
qc.cm.push {ra, s0-s1}, -32
-# CHECK-ASM-AND-OBJ: qc.cm.pushfp {ra}, -16
-# CHECK-ASM: encoding: [0x42,0xb9]
-qc.cm.pushfp {ra}, -16
-
# CHECK-ASM-AND-OBJ: qc.cm.pushfp {ra, s0}, -32
# CHECK-ASM: encoding: [0x56,0xb9]
qc.cm.pushfp {ra, s0}, -32
diff --git a/llvm/test/MC/RISCV/rv64xqccmp-valid.s b/llvm/test/MC/RISCV/rv64xqccmp-valid.s
index 06ba33f..29cf6f0 100644
--- a/llvm/test/MC/RISCV/rv64xqccmp-valid.s
+++ b/llvm/test/MC/RISCV/rv64xqccmp-valid.s
@@ -148,10 +148,6 @@
# CHECK-ASM: encoding: [0xf6,0xb8]
qc.cm.push {ra, s0-s11}, -128
-# CHECK-ASM-AND-OBJ: qc.cm.pushfp {ra}, -16
-# CHECK-ASM: encoding: [0x42,0xb9]
-qc.cm.pushfp {ra}, -16
-
# CHECK-ASM-AND-OBJ: qc.cm.pushfp {ra, s0}, -32
# CHECK-ASM: encoding: [0x56,0xb9]
qc.cm.pushfp {ra, s0}, -32
