[RISCV] Remove post-decoding instruction adjustments

[s-barannikov]

Some instructions implicitly define/use X2 (SP) register, but instead of being present in the Defs/Uses lists, it is sometimes modeled as an explicit operand with SP register class.
Since the operand is not encoded into the instruction, it cannot be disassembled, and we have RISCVDisassembler::addSPOperands() that addresses the issue by mutating the (incompletely) decoded instruction.

This change makes the operand decodable by adding bits<0> field for that operand to relevant instruction encodings and removes RISCVDisassembler::addSPOperands().

[s-barannikov]

• [AArch64] Remove post-decoding instruction mutations #156364
• [AArch64] Provide a custom decoder for LDR_ZA/STR_ZA #156363
• [AArch64] Correctly disassemble TSB instruction #156362
• [AVR] Remove workarounds for instructions using Z register (NFCI) #156361
• [RISCV] Remove post-decoding instruction adjustments #156360 👈 (View in Graphite)
• [Hexagon] Remove post-decoding instruction adjustments #156359
• [TableGen][Decoder] Decode operands with zero width or all bits known #156358
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[llvmbot]

@llvm/pr-subscribers-backend-risc-v

Author: Sergei Barannikov (s-barannikov)

Changes

Some instructions implicitly define/use X2 (SP) register, but instead of being present in the Defs/Uses lists, it is sometimes modeled as an explicit operand with SP register class.
Since the operand is not encoded into the instruction, it cannot be disassembled, and we have RISCVDisassembler::addSPOperands() that addresses the issue by mutating the (incompletely) decoded instruction.

This change makes the operand decodable by adding bits<0> field for that operand to relevant instruction encoding and removes RISCVDisassembler::addSPOperands().

---

Full diff: https://github.com/llvm/llvm-project/pull/156360.diff

5 Files Affected:

• (modified) llvm/lib/Target/RISCV/CMakeLists.txt (+1-2)
• (modified) llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp (+8-17)
• (modified) llvm/lib/Target/RISCV/RISCVInstrFormatsC.td (-1)
• (modified) llvm/lib/Target/RISCV/RISCVInstrInfoC.td (+6-2)
• (modified) llvm/lib/Target/RISCV/RISCVInstrInfoXwch.td (+4)

diff --git a/llvm/lib/Target/RISCV/CMakeLists.txt b/llvm/lib/Target/RISCV/CMakeLists.txt
index 720361dc3da5b..531238ae85029 100644
--- a/llvm/lib/Target/RISCV/CMakeLists.txt
+++ b/llvm/lib/Target/RISCV/CMakeLists.txt
@@ -8,8 +8,7 @@ tablegen(LLVM RISCVGenCompressInstEmitter.inc -gen-compress-inst-emitter)
 tablegen(LLVM RISCVGenMacroFusion.inc -gen-macro-fusion-pred)
 tablegen(LLVM RISCVGenDAGISel.inc -gen-dag-isel)
 tablegen(LLVM RISCVGenDisassemblerTables.inc -gen-disassembler
-              --specialize-decoders-per-bitwidth
-              -ignore-non-decodable-operands)
+              --specialize-decoders-per-bitwidth)
 tablegen(LLVM RISCVGenInstrInfo.inc -gen-instr-info)
 tablegen(LLVM RISCVGenMCCodeEmitter.inc -gen-emitter)
 tablegen(LLVM RISCVGenMCPseudoLowering.inc -gen-pseudo-lowering)
diff --git a/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp b/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
index b1b7ea5246fda..89df9d82f8780 100644
--- a/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
+++ b/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
@@ -46,8 +46,6 @@ class RISCVDisassembler : public MCDisassembler {
                               raw_ostream &CStream) const override;
 
 private:
-  void addSPOperands(MCInst &MI) const;
-
   DecodeStatus getInstruction48(MCInst &Instr, uint64_t &Size,
                                 ArrayRef<uint8_t> Bytes, uint64_t Address,
                                 raw_ostream &CStream) const;
@@ -196,6 +194,12 @@ static DecodeStatus DecodeFPR128RegisterClass(MCInst &Inst, uint32_t RegNo,
   return MCDisassembler::Success;
 }
 
+static DecodeStatus DecodeSPRegisterClass(MCInst &Inst,
+                                          const MCDisassembler *Decoder) {
+  Inst.addOperand(MCOperand::createReg(RISCV::X2));
+  return MCDisassembler::Success;
+}
+
 static DecodeStatus DecodeGPRNoX0RegisterClass(MCInst &Inst, uint32_t RegNo,
                                                uint64_t Address,
                                                const MCDisassembler *Decoder) {
@@ -600,15 +604,6 @@ static DecodeStatus decodeXTHeadMemPair(MCInst &Inst, uint32_t Insn,
 
 #include "RISCVGenDisassemblerTables.inc"
 
-// Add implied SP operand for C.*SP compressed instructions. The SP operand
-// isn't explicitly encoded in the instruction.
-void RISCVDisassembler::addSPOperands(MCInst &MI) const {
-  const MCInstrDesc &MCID = MCII->get(MI.getOpcode());
-  for (unsigned i = 0; i < MCID.getNumOperands(); i++)
-    if (MCID.operands()[i].RegClass == RISCV::SPRegClassID)
-      MI.insert(MI.begin() + i, MCOperand::createReg(RISCV::X2));
-}
-
 namespace {
 
 struct DecoderListEntry {
@@ -774,12 +769,8 @@ DecodeStatus RISCVDisassembler::getInstruction16(MCInst &MI, uint64_t &Size,
     LLVM_DEBUG(dbgs() << "Trying " << Entry.Desc << " table:\n");
     DecodeStatus Result =
         decodeInstruction(Entry.Table, MI, Insn, Address, this, STI);
-    if (Result == MCDisassembler::Fail)
-      continue;
-
-    addSPOperands(MI);
-
-    return Result;
+    if (Result != MCDisassembler::Fail)
+      return Result;
   }
 
   return MCDisassembler::Fail;
diff --git a/llvm/lib/Target/RISCV/RISCVInstrFormatsC.td b/llvm/lib/Target/RISCV/RISCVInstrFormatsC.td
index 209c3fae63f45..4c7cd05723ac8 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrFormatsC.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrFormatsC.td
@@ -54,7 +54,6 @@ class RVInst16CSS<bits<3> funct3, bits<2> opcode, dag outs, dag ins,
     : RVInst16<outs, ins, opcodestr, argstr, [], InstFormatCSS> {
   bits<10> imm;
   bits<5> rs2;
-  bits<5> rs1;
 
   let Inst{15-13} = funct3;
   let Inst{12-7} = imm{5-0};
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoC.td b/llvm/lib/Target/RISCV/RISCVInstrInfoC.td
index bfc766dfc27e5..9fc73662d9704 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoC.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoC.td
@@ -230,13 +230,17 @@ let hasSideEffects = 0, mayLoad = 1, mayStore = 0 in
 class CStackLoad<bits<3> funct3, string OpcodeStr,
                  DAGOperand cls, DAGOperand opnd>
     : RVInst16CI<funct3, 0b10, (outs cls:$rd), (ins SPMem:$rs1, opnd:$imm),
-                 OpcodeStr, "$rd, ${imm}(${rs1})">;
+                 OpcodeStr, "$rd, ${imm}(${rs1})"> {
+  bits<0> rs1;
+}
 
 let hasSideEffects = 0, mayLoad = 0, mayStore = 1 in
 class CStackStore<bits<3> funct3, string OpcodeStr,
                   DAGOperand cls, DAGOperand opnd>
     : RVInst16CSS<funct3, 0b10, (outs), (ins cls:$rs2, SPMem:$rs1, opnd:$imm),
-                  OpcodeStr, "$rs2, ${imm}(${rs1})">;
+                  OpcodeStr, "$rs2, ${imm}(${rs1})"> {
+  bits<0> rs1;
+}
 
 let hasSideEffects = 0, mayLoad = 1, mayStore = 0 in
 class CLoad_ri<bits<3> funct3, string OpcodeStr,
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoXwch.td b/llvm/lib/Target/RISCV/RISCVInstrInfoXwch.td
index a43cbadf6f308..bb1862cc88d64 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoXwch.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoXwch.td
@@ -106,6 +106,7 @@ def QK_C_LBUSP : QKStackInst<0b00, (outs GPRC:$rd_rs2),
                              (ins SPMem:$rs1, uimm4:$imm),
                              "qk.c.lbusp", "$rd_rs2, ${imm}(${rs1})">,
                  Sched<[WriteLDB, ReadMemBase]> {
+  bits<0> rs1;
   bits<4> imm;
   let Inst{10-7} = imm;
 }
@@ -115,6 +116,7 @@ def QK_C_SBSP : QKStackInst<0b10, (outs),
                                  uimm4:$imm),
                             "qk.c.sbsp", "$rd_rs2, ${imm}(${rs1})">,
                 Sched<[WriteSTB, ReadStoreData, ReadMemBase]> {
+  bits<0> rs1;
   bits<4> imm;
   let Inst{10-7} = imm;
 }
@@ -124,6 +126,7 @@ def QK_C_LHUSP : QKStackInst<0b01, (outs GPRC:$rd_rs2),
                              (ins SPMem:$rs1, uimm5_lsb0:$imm),
                              "qk.c.lhusp", "$rd_rs2, ${imm}(${rs1})">,
                  Sched<[WriteLDH, ReadMemBase]> {
+  bits<0> rs1;
   bits<5> imm;
   let Inst{10-8} = imm{3-1};
   let Inst{7} = imm{4};
@@ -133,6 +136,7 @@ def QK_C_SHSP : QKStackInst<0b11, (outs),
                             (ins GPRC:$rd_rs2, SPMem:$rs1, uimm5_lsb0:$imm),
                             "qk.c.shsp", "$rd_rs2, ${imm}(${rs1})">,
                 Sched<[WriteSTH, ReadStoreData, ReadMemBase]> {
+  bits<0> rs1;
   bits<5> imm;
   let Inst{10-8} = imm{3-1};
   let Inst{7} = imm{4};

[topperc]

LGTM

[lenary]

Thanks!