[llvm] [RISCV][NFCI] Rationalize Immediate Definitions (PR #120718)
Sam Elliott via llvm-commits
llvm-commits at lists.llvm.org
Fri Dec 20 03:47:30 PST 2024
https://github.com/lenary created https://github.com/llvm/llvm-project/pull/120718
This change aims to bring a lot of the immediate handling code, especially in TableGen, to become more uniform, both for different sizes, and also for unsigned vs signed immediates.
Some changes, none of which should affect current behaviour:
- Both `RISCVUImmOp` and `RISCVSImmOp` now use getImmOpValue - which now has to return a 64-bit value because we have some 64-bit unsigned immediates for `.insn`
- Both `RISCVUImmOp` and `RISCVSImmOp` now define a default MCOperandPredicate, based on their width, which just checks their value with `isInt` or `isUInt`. This is overridden for the immediates that accept a bare symbol ref in addition to an immediate. This allows some duplicate definitions to be removed.
- I moved some OPERAND_ enum values around so that the unsigned immediates are beside each other, and so too for the signed immediates.
>From 878c34216c77aa79227db5a7f6b72a4c36474499 Mon Sep 17 00:00:00 2001
From: Sam Elliott <quic_aelliott at quicinc.com>
Date: Fri, 20 Dec 2024 03:35:23 -0800
Subject: [PATCH] [RISCV][NFCI] Rationalize Immediate Definitions
This change aims to bring a lot of the immediate handling code,
especially in TableGen, to become more uniform, both for different
sizes, and also for unsigned vs signed immediates.
Some changes, none of which should affect current behaviour:
- Both `RISCVUImmOp` and `RISCVSImmOp` now use getImmOpValue - which now
has to return a 64-bit value because we have some 64-bit unsigned
immediates for `.insn`
- Both `RISCVUImmOp` and `RISCVSImmOp` now define a default
MCOperandPredicate, based on their width, which just checks their
value with `isInt` or `isUInt`. This is overridden for the immediates
that accept a bare symbol ref in addition to an immediate. This allows
some duplicate definitions to be removed.
- I moved some OPERAND_ enum values around so that the unsigned
immediates are beside each other, and so too for the signed
immediates.
---
.../Target/RISCV/MCTargetDesc/RISCVBaseInfo.h | 6 +--
.../RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp | 10 ++---
llvm/lib/Target/RISCV/RISCVInstrInfo.cpp | 18 ++++----
llvm/lib/Target/RISCV/RISCVInstrInfo.td | 36 ++++++++--------
llvm/lib/Target/RISCV/RISCVInstrInfoC.td | 5 +--
llvm/lib/Target/RISCV/RISCVInstrInfoV.td | 8 +---
llvm/lib/Target/RISCV/RISCVInstrInfoXwch.td | 42 ++++++-------------
7 files changed, 48 insertions(+), 77 deletions(-)
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h
index 056b6da34ac708..61dc9754afb512 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h
@@ -316,6 +316,9 @@ enum OperandType : unsigned {
OPERAND_UIMM11,
OPERAND_UIMM12,
OPERAND_UIMM16,
+ OPERAND_UIMM20,
+ OPERAND_UIMMLOG2XLEN,
+ OPERAND_UIMMLOG2XLEN_NONZERO,
OPERAND_UIMM32,
OPERAND_UIMM48,
OPERAND_UIMM64,
@@ -327,9 +330,6 @@ enum OperandType : unsigned {
OPERAND_SIMM10_LSB0000_NONZERO,
OPERAND_SIMM12,
OPERAND_SIMM12_LSB00000,
- OPERAND_UIMM20,
- OPERAND_UIMMLOG2XLEN,
- OPERAND_UIMMLOG2XLEN_NONZERO,
OPERAND_CLUI_IMM,
OPERAND_VTYPEI10,
OPERAND_VTYPEI11,
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
index a28bf1186589d9..06d9cce48692a5 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
@@ -80,11 +80,11 @@ class RISCVMCCodeEmitter : public MCCodeEmitter {
SmallVectorImpl<MCFixup> &Fixups,
const MCSubtargetInfo &STI) const;
- unsigned getImmOpValueAsr1(const MCInst &MI, unsigned OpNo,
+ uint64_t getImmOpValueAsr1(const MCInst &MI, unsigned OpNo,
SmallVectorImpl<MCFixup> &Fixups,
const MCSubtargetInfo &STI) const;
- unsigned getImmOpValue(const MCInst &MI, unsigned OpNo,
+ uint64_t getImmOpValue(const MCInst &MI, unsigned OpNo,
SmallVectorImpl<MCFixup> &Fixups,
const MCSubtargetInfo &STI) const;
@@ -385,14 +385,14 @@ RISCVMCCodeEmitter::getMachineOpValue(const MCInst &MI, const MCOperand &MO,
return 0;
}
-unsigned
+uint64_t
RISCVMCCodeEmitter::getImmOpValueAsr1(const MCInst &MI, unsigned OpNo,
SmallVectorImpl<MCFixup> &Fixups,
const MCSubtargetInfo &STI) const {
const MCOperand &MO = MI.getOperand(OpNo);
if (MO.isImm()) {
- unsigned Res = MO.getImm();
+ uint64_t Res = MO.getImm();
assert((Res & 1) == 0 && "LSB is non-zero");
return Res >> 1;
}
@@ -400,7 +400,7 @@ RISCVMCCodeEmitter::getImmOpValueAsr1(const MCInst &MI, unsigned OpNo,
return getImmOpValue(MI, OpNo, Fixups, STI);
}
-unsigned RISCVMCCodeEmitter::getImmOpValue(const MCInst &MI, unsigned OpNo,
+uint64_t RISCVMCCodeEmitter::getImmOpValue(const MCInst &MI, unsigned OpNo,
SmallVectorImpl<MCFixup> &Fixups,
const MCSubtargetInfo &STI) const {
bool EnableRelax = STI.hasFeature(RISCV::FeatureRelax);
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
index 0af8161a307abd..ee720ceb22b00f 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
@@ -2466,6 +2466,10 @@ bool RISCVInstrInfo::verifyInstruction(const MachineInstr &MI,
#define CASE_OPERAND_UIMM(NUM) \
case RISCVOp::OPERAND_UIMM##NUM: \
Ok = isUInt<NUM>(Imm); \
+ break;
+#define CASE_OPERAND_SIMM(NUM) \
+ case RISCVOp::OPERAND_SIMM##NUM: \
+ Ok = isInt<NUM>(Imm); \
break;
CASE_OPERAND_UIMM(1)
CASE_OPERAND_UIMM(2)
@@ -2511,15 +2515,14 @@ bool RISCVInstrInfo::verifyInstruction(const MachineInstr &MI,
case RISCVOp::OPERAND_ZERO:
Ok = Imm == 0;
break;
- case RISCVOp::OPERAND_SIMM5:
- Ok = isInt<5>(Imm);
- break;
+ // clang-format off
+ CASE_OPERAND_SIMM(5)
+ CASE_OPERAND_SIMM(6)
+ CASE_OPERAND_SIMM(12)
+ // clang-format on
case RISCVOp::OPERAND_SIMM5_PLUS1:
Ok = (isInt<5>(Imm) && Imm != -16) || Imm == 16;
break;
- case RISCVOp::OPERAND_SIMM6:
- Ok = isInt<6>(Imm);
- break;
case RISCVOp::OPERAND_SIMM6_NONZERO:
Ok = Imm != 0 && isInt<6>(Imm);
break;
@@ -2529,9 +2532,6 @@ bool RISCVInstrInfo::verifyInstruction(const MachineInstr &MI,
case RISCVOp::OPERAND_VTYPEI11:
Ok = isUInt<11>(Imm);
break;
- case RISCVOp::OPERAND_SIMM12:
- Ok = isInt<12>(Imm);
- break;
case RISCVOp::OPERAND_SIMM12_LSB00000:
Ok = isShiftedInt<7, 5>(Imm);
break;
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.td b/llvm/lib/Target/RISCV/RISCVInstrInfo.td
index 24bf7da4dadc07..13f536affb03b7 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.td
@@ -159,8 +159,15 @@ class RISCVOp<ValueType vt = XLenVT> : Operand<vt> {
class RISCVUImmOp<int bitsNum> : RISCVOp {
let ParserMatchClass = UImmAsmOperand<bitsNum>;
+ let EncoderMethod = "getImmOpValue";
let DecoderMethod = "decodeUImmOperand<" # bitsNum # ">";
let OperandType = "OPERAND_UIMM" # bitsNum;
+ let MCOperandPredicate = [{
+ int64_t Imm;
+ if (!MCOp.evaluateAsConstantImm(Imm))
+ return false;
+ return isUInt<}]# bitsNum #[{>(Imm);
+ }];
}
class RISCVUImmLeafOp<int bitsNum> :
@@ -171,6 +178,12 @@ class RISCVSImmOp<int bitsNum> : RISCVOp {
let EncoderMethod = "getImmOpValue";
let DecoderMethod = "decodeSImmOperand<" # bitsNum # ">";
let OperandType = "OPERAND_SIMM" # bitsNum;
+ let MCOperandPredicate = [{
+ int64_t Imm;
+ if (!MCOp.evaluateAsConstantImm(Imm))
+ return false;
+ return isInt<}] # bitsNum # [{>(Imm);
+ }];
}
class RISCVSImmLeafOp<int bitsNum> :
@@ -221,16 +234,9 @@ def InsnDirectiveOpcode : AsmOperandClass {
}
def uimm1 : RISCVUImmLeafOp<1>;
-def uimm2 : RISCVUImmLeafOp<2> {
- let MCOperandPredicate = [{
- int64_t Imm;
- if (!MCOp.evaluateAsConstantImm(Imm))
- return false;
- return isUInt<2>(Imm);
- }];
-}
+def uimm2 : RISCVUImmLeafOp<2>;
def uimm3 : RISCVUImmOp<3>;
-def uimm4 : RISCVUImmOp<4>;
+def uimm4 : RISCVUImmLeafOp<4>;
def uimm5 : RISCVUImmLeafOp<5>;
def uimm6 : RISCVUImmLeafOp<6>;
def uimm7_opcode : RISCVUImmOp<7> {
@@ -277,7 +283,7 @@ class UImm20Operand : RISCVOp {
let OperandType = "OPERAND_UIMM20";
}
-class UImm20OperandMaybeSym : UImm20Operand {
+class UImm20OperandMaybeSym : RISCVUImmOp<20> {
let MCOperandPredicate = [{
int64_t Imm;
if (MCOp.evaluateAsConstantImm(Imm))
@@ -293,15 +299,7 @@ def uimm20_auipc : UImm20OperandMaybeSym {
let ParserMatchClass = UImmAsmOperand<20, "AUIPC">;
}
-def uimm20 : UImm20Operand {
- let ParserMatchClass = UImmAsmOperand<20>;
- let MCOperandPredicate = [{
- int64_t Imm;
- if (!MCOp.evaluateAsConstantImm(Imm))
- return false;
- return isUInt<20>(Imm);
- }];
-}
+def uimm20 : RISCVUImmOp<20>;
def Simm21Lsb0JALAsmOperand : SImmAsmOperand<21, "Lsb0JAL"> {
let ParserMethod = "parseJALOffset";
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoC.td b/llvm/lib/Target/RISCV/RISCVInstrInfoC.td
index e5a5f60f9fec10..ce994206cd785b 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoC.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoC.td
@@ -151,7 +151,6 @@ def simm9_lsb0 : Operand<OtherVT>,
if (MCOp.evaluateAsConstantImm(Imm))
return isShiftedInt<8, 1>(Imm);
return MCOp.isBareSymbolRef();
-
}];
let OperandType = "OPERAND_PCREL";
}
@@ -227,10 +226,8 @@ def InsnCDirectiveOpcode : AsmOperandClass {
let PredicateMethod = "isImm";
}
-def uimm2_opcode : RISCVOp {
+def uimm2_opcode : RISCVUImmOp<2> {
let ParserMatchClass = InsnCDirectiveOpcode;
- let DecoderMethod = "decodeUImmOperand<2>";
- let OperandType = "OPERAND_UIMM2";
}
//===----------------------------------------------------------------------===//
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoV.td b/llvm/lib/Target/RISCV/RISCVInstrInfoV.td
index 6506b6746b1517..24a881dc6810f8 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoV.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoV.td
@@ -66,15 +66,9 @@ def simm5 : RISCVSImmLeafOp<5> {
}];
}
-def SImm5Plus1AsmOperand : AsmOperandClass {
- let Name = "SImm5Plus1";
- let RenderMethod = "addImmOperands";
- let DiagnosticType = "InvalidSImm5Plus1";
-}
-
def simm5_plus1 : RISCVOp, ImmLeaf<XLenVT,
[{return (isInt<5>(Imm) && Imm != -16) || Imm == 16;}]> {
- let ParserMatchClass = SImm5Plus1AsmOperand;
+ let ParserMatchClass = SImmAsmOperand<5, "Plus1">;
let OperandType = "OPERAND_SIMM5_PLUS1";
let MCOperandPredicate = [{
int64_t Imm;
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoXwch.td b/llvm/lib/Target/RISCV/RISCVInstrInfoXwch.td
index 91ff804ba105ac..a43cbadf6f3080 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoXwch.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoXwch.td
@@ -25,24 +25,6 @@ class QKStackInst<bits<2> funct2, dag outs, dag ins,
// Operand definitions.
//===----------------------------------------------------------------------===//
-def uimm4_with_predicate : RISCVUImmLeafOp<4> {
- let MCOperandPredicate = [{
- int64_t Imm;
- if (!MCOp.evaluateAsConstantImm(Imm))
- return false;
- return isUInt<4>(Imm);
- }];
-}
-
-def uimm5_with_predicate : RISCVUImmLeafOp<5> {
- let MCOperandPredicate = [{
- int64_t Imm;
- if (!MCOp.evaluateAsConstantImm(Imm))
- return false;
- return isUInt<5>(Imm);
- }];
-}
-
// A 5-bit unsigned immediate where the least significant bit is zero.
def uimm5_lsb0 : RISCVOp,
ImmLeaf<XLenVT, [{return isShiftedUInt<4, 1>(Imm);}]> {
@@ -80,7 +62,7 @@ let Predicates = [HasVendorXwchc], DecoderNamespace = "Xwchc" in {
let hasSideEffects = 0, mayLoad = 1, mayStore = 0 in
def QK_C_LBU : RVInst16CL<0b001, 0b00, (outs GPRC:$rd),
- (ins GPRCMem:$rs1, uimm5_with_predicate:$imm),
+ (ins GPRCMem:$rs1, uimm5:$imm),
"qk.c.lbu", "$rd, ${imm}(${rs1})">,
Sched<[WriteLDB, ReadMemBase]> {
bits<5> imm;
@@ -91,7 +73,7 @@ def QK_C_LBU : RVInst16CL<0b001, 0b00, (outs GPRC:$rd),
let hasSideEffects = 0, mayLoad = 0, mayStore = 1 in
def QK_C_SB : RVInst16CS<0b101, 0b00, (outs),
(ins GPRC:$rs2, GPRCMem:$rs1,
- uimm5_with_predicate:$imm),
+ uimm5:$imm),
"qk.c.sb", "$rs2, ${imm}(${rs1})">,
Sched<[WriteSTB, ReadStoreData, ReadMemBase]> {
bits<5> imm;
@@ -121,7 +103,7 @@ def QK_C_SH : RVInst16CS<0b101, 0b10, (outs),
let hasSideEffects = 0, mayLoad = 1, mayStore = 0 in
def QK_C_LBUSP : QKStackInst<0b00, (outs GPRC:$rd_rs2),
- (ins SPMem:$rs1, uimm4_with_predicate:$imm),
+ (ins SPMem:$rs1, uimm4:$imm),
"qk.c.lbusp", "$rd_rs2, ${imm}(${rs1})">,
Sched<[WriteLDB, ReadMemBase]> {
bits<4> imm;
@@ -130,7 +112,7 @@ def QK_C_LBUSP : QKStackInst<0b00, (outs GPRC:$rd_rs2),
let hasSideEffects = 0, mayLoad = 0, mayStore = 1 in
def QK_C_SBSP : QKStackInst<0b10, (outs),
(ins GPRC:$rd_rs2, SPMem:$rs1,
- uimm4_with_predicate:$imm),
+ uimm4:$imm),
"qk.c.sbsp", "$rd_rs2, ${imm}(${rs1})">,
Sched<[WriteSTB, ReadStoreData, ReadMemBase]> {
bits<4> imm;
@@ -180,18 +162,18 @@ def : InstAlias<"qk.c.shsp $rs2, (${rs1})", (QK_C_SHSP GPRC:$rs2, SPMem:$rs1, 0)
//===----------------------------------------------------------------------===//
let Predicates = [HasVendorXwchc] in {
-def : CompressPat<(LBU GPRC:$rd, GPRCMem:$rs1, uimm5_with_predicate:$imm),
- (QK_C_LBU GPRC:$rd, GPRCMem:$rs1, uimm5_with_predicate:$imm)>;
-def : CompressPat<(SB GPRC:$rs2, GPRCMem:$rs1, uimm5_with_predicate:$imm),
- (QK_C_SB GPRC:$rs2, GPRCMem:$rs1, uimm5_with_predicate:$imm)>;
+def : CompressPat<(LBU GPRC:$rd, GPRCMem:$rs1, uimm5:$imm),
+ (QK_C_LBU GPRC:$rd, GPRCMem:$rs1, uimm5:$imm)>;
+def : CompressPat<(SB GPRC:$rs2, GPRCMem:$rs1, uimm5:$imm),
+ (QK_C_SB GPRC:$rs2, GPRCMem:$rs1, uimm5:$imm)>;
def : CompressPat<(LHU GPRC:$rd, GPRCMem:$rs1, uimm6_lsb0:$imm),
(QK_C_LHU GPRC:$rd, GPRCMem:$rs1, uimm6_lsb0:$imm)>;
def : CompressPat<(SH GPRC:$rs2, GPRCMem:$rs1, uimm6_lsb0:$imm),
(QK_C_SH GPRC:$rs2, GPRCMem:$rs1, uimm6_lsb0:$imm)>;
-def : CompressPat<(LBU GPRC:$rd, SPMem:$rs1, uimm4_with_predicate:$imm),
- (QK_C_LBUSP GPRC:$rd, SPMem:$rs1, uimm4_with_predicate:$imm)>;
-def : CompressPat<(SB GPRC:$rs2, SPMem:$rs1, uimm4_with_predicate:$imm),
- (QK_C_SBSP GPRC:$rs2, SPMem:$rs1, uimm4_with_predicate:$imm)>;
+def : CompressPat<(LBU GPRC:$rd, SPMem:$rs1, uimm4:$imm),
+ (QK_C_LBUSP GPRC:$rd, SPMem:$rs1, uimm4:$imm)>;
+def : CompressPat<(SB GPRC:$rs2, SPMem:$rs1, uimm4:$imm),
+ (QK_C_SBSP GPRC:$rs2, SPMem:$rs1, uimm4:$imm)>;
def : CompressPat<(LHU GPRC:$rd, SPMem:$rs1, uimm5_lsb0:$imm),
(QK_C_LHUSP GPRC:$rd, SPMem:$rs1, uimm5_lsb0:$imm)>;
def : CompressPat<(SH GPRC:$rs2, SPMem:$rs1, uimm5_lsb0:$imm),
More information about the llvm-commits
mailing list