[llvm] [RISCV] Use named sub-operands to simplify encoding/decoding for CoreV Reg-Reg instructions. (PR #133181)

Craig Topper via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 26 16:48:59 PDT 2025


https://github.com/topperc updated https://github.com/llvm/llvm-project/pull/133181

>From 95c94cbfd58ad48e8ca41d86bfff9518fc41e676 Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper at sifive.com>
Date: Wed, 26 Mar 2025 16:24:06 -0700
Subject: [PATCH 1/2] [RISCV] Use named sub-operands to simplify
 encoding/decoding for CoreV Reg-Reg instructions.

We can name the sub-operands using a DAG in the 'ins'. This allows
those names to be matched to the encoding fields. This removes the
need for a custom encoder/decoder that treats the 2 sub-operands as
a single 10-bit value.

While doing this, I noticed the base and offset names in the
MIOperandInfo were swapped relative to how the operands are parsed
and printed. Assuming that I've correclty understood the parsing/print
format as "offset(base)".
---
 .../RISCV/Disassembler/RISCVDisassembler.cpp  | 12 --------
 .../RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp | 17 -----------
 llvm/lib/Target/RISCV/RISCVInstrInfoXCV.td    | 29 +++++++------------
 3 files changed, 10 insertions(+), 48 deletions(-)

diff --git a/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp b/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
index 93cbf662bfa32..46b01417a1ea1 100644
--- a/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
+++ b/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
@@ -507,9 +507,6 @@ static DecodeStatus decodeXTHeadMemPair(MCInst &Inst, uint32_t Insn,
 static DecodeStatus decodeZcmpRlist(MCInst &Inst, uint32_t Imm,
                                     uint64_t Address, const void *Decoder);
 
-static DecodeStatus decodeRegReg(MCInst &Inst, uint32_t Insn, uint64_t Address,
-                                 const MCDisassembler *Decoder);
-
 static DecodeStatus decodeZcmpSpimm(MCInst &Inst, uint32_t Imm,
                                     uint64_t Address, const void *Decoder);
 
@@ -621,15 +618,6 @@ static DecodeStatus decodeZcmpRlist(MCInst &Inst, uint32_t Imm,
   return MCDisassembler::Success;
 }
 
-static DecodeStatus decodeRegReg(MCInst &Inst, uint32_t Insn, uint64_t Address,
-                                 const MCDisassembler *Decoder) {
-  uint32_t Rs1 = fieldFromInstruction(Insn, 0, 5);
-  uint32_t Rs2 = fieldFromInstruction(Insn, 5, 5);
-  DecodeGPRRegisterClass(Inst, Rs1, Address, Decoder);
-  DecodeGPRRegisterClass(Inst, Rs2, Address, Decoder);
-  return MCDisassembler::Success;
-}
-
 static DecodeStatus decodeZcmpSpimm(MCInst &Inst, uint32_t Imm,
                                     uint64_t Address, const void *Decoder) {
   Inst.addOperand(MCOperand::createImm(Imm));
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
index e589e6171d010..77d33f2e06871 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
@@ -103,10 +103,6 @@ class RISCVMCCodeEmitter : public MCCodeEmitter {
   unsigned getRlistOpValue(const MCInst &MI, unsigned OpNo,
                            SmallVectorImpl<MCFixup> &Fixups,
                            const MCSubtargetInfo &STI) const;
-
-  unsigned getRegReg(const MCInst &MI, unsigned OpNo,
-                     SmallVectorImpl<MCFixup> &Fixups,
-                     const MCSubtargetInfo &STI) const;
 };
 } // end anonymous namespace
 
@@ -621,17 +617,4 @@ unsigned RISCVMCCodeEmitter::getRlistOpValue(const MCInst &MI, unsigned OpNo,
   return Imm;
 }
 
-unsigned RISCVMCCodeEmitter::getRegReg(const MCInst &MI, unsigned OpNo,
-                                       SmallVectorImpl<MCFixup> &Fixups,
-                                       const MCSubtargetInfo &STI) const {
-  const MCOperand &MO = MI.getOperand(OpNo);
-  const MCOperand &MO1 = MI.getOperand(OpNo + 1);
-  assert(MO.isReg() && MO1.isReg() && "Expected registers.");
-
-  unsigned Op = Ctx.getRegisterInfo()->getEncodingValue(MO.getReg());
-  unsigned Op1 = Ctx.getRegisterInfo()->getEncodingValue(MO1.getReg());
-
-  return Op | Op1 << 5;
-}
-
 #include "RISCVGenMCCodeEmitter.inc"
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoXCV.td b/llvm/lib/Target/RISCV/RISCVInstrInfoXCV.td
index e18a61ad79278..9c59da3ee94b6 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoXCV.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoXCV.td
@@ -24,10 +24,8 @@ def CVrrAsmOperand : AsmOperandClass {
 def CVrr : Operand<i32>,
            ComplexPattern<i32, 2, "SelectAddrRegReg",[]> {
    let ParserMatchClass = CVrrAsmOperand;
-   let EncoderMethod =  "getRegReg";
-   let DecoderMethod = "decodeRegReg";
    let PrintMethod = "printRegReg";
-   let MIOperandInfo = (ops GPR:$base, GPR:$offset);
+   let MIOperandInfo = (ops GPR:$offset, GPR:$base);
 }
 
 def cv_tuimm2 : TImmLeaf<XLenVT, [{return isUInt<2>(Imm);}]>;
@@ -288,17 +286,9 @@ class CVLoad_rr_inc<bits<7> funct7, bits<3> funct3, string opcodestr>
 }
 
 class CVLoad_rr<bits<7> funct7, bits<3> funct3, string opcodestr>
-    : RVInstR<funct7, funct3, OPC_CUSTOM_1, (outs GPR:$rd), (ins CVrr:$cvrr),
-              opcodestr, "$rd, $cvrr"> {
-  bits<5> rd;
-  bits<10> cvrr;
-
-  let Inst{31-25} = funct7;
-  let Inst{24-20} = cvrr{4-0};
-  let Inst{19-15} = cvrr{9-5};
-  let Inst{14-12} = funct3;
-  let Inst{11-7} = rd;
-}
+    : RVInstR<funct7, funct3, OPC_CUSTOM_1, (outs GPR:$rd),
+              (ins (CVrr $rs2, $rs1):$cvrr),
+              opcodestr, "$rd, $cvrr">;
 } // hasSideEffects = 0, mayLoad = 1, mayStore = 0
 
 let hasSideEffects = 0, mayLoad = 0, mayStore = 1 in {
@@ -327,16 +317,17 @@ class CVStore_rr_inc<bits<3> funct3, bits<7> funct7, string opcodestr>
 
 
 class CVStore_rr<bits<3> funct3, bits<7> funct7, string opcodestr>
-    : RVInst<(outs), (ins GPR:$rs2, CVrr:$cvrr), opcodestr, "$rs2, $cvrr", [],
-             InstFormatOther> {
+    : RVInst<(outs), (ins GPR:$rs2, (CVrr $rs3, $rs1):$cvrr), opcodestr,
+             "$rs2, $cvrr", [], InstFormatOther> {
+  bits<5> rs1;
   bits<5> rs2;
-  bits<10> cvrr;
+  bits<5> rs3;
 
   let Inst{31-25} = funct7;
   let Inst{24-20} = rs2;
-  let Inst{19-15} = cvrr{9-5};
+  let Inst{19-15} = rs1;
   let Inst{14-12} = funct3;
-  let Inst{11-7} = cvrr{4-0};
+  let Inst{11-7} = rs3;
   let Inst{6-0} = OPC_CUSTOM_1.Value;
 }
 } // hasSideEffects = 0, mayLoad = 0, mayStore = 1

>From d1bad9961d20ea47d68544dec8c2564f8801070f Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper at sifive.com>
Date: Wed, 26 Mar 2025 16:48:33 -0700
Subject: [PATCH 2/2] fixup! rename the operand to $addr.

---
 llvm/lib/Target/RISCV/RISCVInstrInfoXCV.td | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoXCV.td b/llvm/lib/Target/RISCV/RISCVInstrInfoXCV.td
index 9c59da3ee94b6..9ab7e1ca2936c 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoXCV.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoXCV.td
@@ -287,8 +287,8 @@ class CVLoad_rr_inc<bits<7> funct7, bits<3> funct3, string opcodestr>
 
 class CVLoad_rr<bits<7> funct7, bits<3> funct3, string opcodestr>
     : RVInstR<funct7, funct3, OPC_CUSTOM_1, (outs GPR:$rd),
-              (ins (CVrr $rs2, $rs1):$cvrr),
-              opcodestr, "$rd, $cvrr">;
+              (ins (CVrr $rs2, $rs1):$addr),
+              opcodestr, "$rd, $addr">;
 } // hasSideEffects = 0, mayLoad = 1, mayStore = 0
 
 let hasSideEffects = 0, mayLoad = 0, mayStore = 1 in {
@@ -317,8 +317,8 @@ class CVStore_rr_inc<bits<3> funct3, bits<7> funct7, string opcodestr>
 
 
 class CVStore_rr<bits<3> funct3, bits<7> funct7, string opcodestr>
-    : RVInst<(outs), (ins GPR:$rs2, (CVrr $rs3, $rs1):$cvrr), opcodestr,
-             "$rs2, $cvrr", [], InstFormatOther> {
+    : RVInst<(outs), (ins GPR:$rs2, (CVrr $rs3, $rs1):$addr), opcodestr,
+             "$rs2, $addr", [], InstFormatOther> {
   bits<5> rs1;
   bits<5> rs2;
   bits<5> rs3;



More information about the llvm-commits mailing list