[AVR] Refactor ADIW/SBIW/MOVW instruction descriptions (NFCI) (llvm#156876)

s-barannikov · web-flow · commit f28f1c24e91c · 2025-09-05T15:32:53.000+03:00
* Remove custom decoders for these instructions
* Instead, provide decoders for DREGS/IWREGS register classes
* Change register pair encodings to simplify instruction descriptions
* Add/fix a few clarifying comments
diff --git a/llvm/lib/Target/AVR/AVRInstrFormats.td b/llvm/lib/Target/AVR/AVRInstrFormats.td
@@ -213,18 +213,16 @@ class FLPMX<bit e, bit p, dag outs, dag ins, string asmstr, list<dag> pattern>
 // MOVWRdRr special encoding: <|0000|0001|dddd|rrrr|>
 // d = destination = 4 bits
 // r = source = 4 bits
-// (Only accepts even registers)
+// (Only accepts register pairs)
 //===----------------------------------------------------------------------===//
 class FMOVWRdRr<dag outs, dag ins, string asmstr, list<dag> pattern>
     : AVRInst16<outs, ins, asmstr, pattern> {
-  bits<5> rd;
-  bits<5> rr;
+  bits<4> rd;
+  bits<4> rr;
 
   let Inst{15 - 8} = 0b00000001;
-  let Inst{7 - 4} = rd{4 - 1};
-  let Inst{3 - 0} = rr{4 - 1};
-
-  let DecoderMethod = "decodeFMOVWRdRr";
+  let Inst{7 - 4} = rd;
+  let Inst{3 - 0} = rr;
 }
 
 //===----------------------------------------------------------------------===//
@@ -292,16 +290,14 @@ class FFMULRdRr<bits<2> f, dag outs, dag ins, string asmstr, list<dag> pattern>
 //===----------------------------------------------------------------------===//
 class FWRdK<bit f, dag outs, dag ins, string asmstr, list<dag> pattern>
     : AVRInst16<outs, ins, asmstr, pattern> {
-  bits<5> rd; // accept 5 bits but only encode bits 1 and 2
+  bits<2> rd;
   bits<6> k;
 
   let Inst{15 - 9} = 0b1001011;
   let Inst{8} = f;
   let Inst{7 - 6} = k{5 - 4};
-  let Inst{5 - 4} = rd{2 - 1};
+  let Inst{5 - 4} = rd;
   let Inst{3 - 0} = k{3 - 0};
-
-  let DecoderMethod = "decodeFWRdK";
 }
 
 //===----------------------------------------------------------------------===//
diff --git a/llvm/lib/Target/AVR/AVRRegisterInfo.td b/llvm/lib/Target/AVR/AVRRegisterInfo.td
@@ -68,33 +68,37 @@ def R31 : AVRReg<31, "r31", [], ["zh"]>, DwarfRegNum<[31]>;
 def SPL : AVRReg<32, "SPL">, DwarfRegNum<[32]>;
 def SPH : AVRReg<33, "SPH">, DwarfRegNum<[33]>;
 
+// 16 bit GPR pairs.
 let SubRegIndices = [sub_lo, sub_hi], CoveredBySubRegs = 1 in {
-  // 16 bit GPR pairs.
-  def SP : AVRReg<32, "SP", [SPL, SPH]>, DwarfRegNum<[32]>;
+  // The value 16 for the encoding is arbitrary. SP register is not encoded
+  // into instructions, they use it implicitly depending on the opcode.
+  def SP : AVRReg<16, "SP", [SPL, SPH]>, DwarfRegNum<[32]>;
 
   // The pointer registers (X,Y,Z) are a special case because they
   // are printed as a `high:low` pair when a DREG is expected,
   // but printed using `X`, `Y`, `Z` when a pointer register is expected.
+  // DREG registers are only used in ADIW, SBIW and MOVW instructions.
   let RegAltNameIndices = [ptr] in {
-    def R31R30 : AVRReg<30, "r31:r30", [R30, R31], ["Z"]>, DwarfRegNum<[30]>;
-    def R29R28 : AVRReg<28, "r29:r28", [R28, R29], ["Y"]>, DwarfRegNum<[28]>;
-    def R27R26 : AVRReg<26, "r27:r26", [R26, R27], ["X"]>, DwarfRegNum<[26]>;
+    def R31R30 : AVRReg<15, "r31:r30", [R30, R31], ["Z"]>, DwarfRegNum<[30]>;
+    def R29R28 : AVRReg<14, "r29:r28", [R28, R29], ["Y"]>, DwarfRegNum<[28]>;
+    def R27R26 : AVRReg<13, "r27:r26", [R26, R27], ["X"]>, DwarfRegNum<[26]>;
   }
-  def R25R24 : AVRReg<24, "r25:r24", [R24, R25]>, DwarfRegNum<[24]>;
-  def R23R22 : AVRReg<22, "r23:r22", [R22, R23]>, DwarfRegNum<[22]>;
-  def R21R20 : AVRReg<20, "r21:r20", [R20, R21]>, DwarfRegNum<[20]>;
-  def R19R18 : AVRReg<18, "r19:r18", [R18, R19]>, DwarfRegNum<[18]>;
-  def R17R16 : AVRReg<16, "r17:r16", [R16, R17]>, DwarfRegNum<[16]>;
-  def R15R14 : AVRReg<14, "r15:r14", [R14, R15]>, DwarfRegNum<[14]>;
-  def R13R12 : AVRReg<12, "r13:r12", [R12, R13]>, DwarfRegNum<[12]>;
-  def R11R10 : AVRReg<10, "r11:r10", [R10, R11]>, DwarfRegNum<[10]>;
-  def R9R8 : AVRReg<8, "r9:r8", [R8, R9]>, DwarfRegNum<[8]>;
-  def R7R6 : AVRReg<6, "r7:r6", [R6, R7]>, DwarfRegNum<[6]>;
-  def R5R4 : AVRReg<4, "r5:r4", [R4, R5]>, DwarfRegNum<[4]>;
-  def R3R2 : AVRReg<2, "r3:r2", [R2, R3]>, DwarfRegNum<[2]>;
+  def R25R24 : AVRReg<12, "r25:r24", [R24, R25]>, DwarfRegNum<[24]>;
+  def R23R22 : AVRReg<11, "r23:r22", [R22, R23]>, DwarfRegNum<[22]>;
+  def R21R20 : AVRReg<10, "r21:r20", [R20, R21]>, DwarfRegNum<[20]>;
+  def R19R18 : AVRReg<9, "r19:r18", [R18, R19]>, DwarfRegNum<[18]>;
+  def R17R16 : AVRReg<8, "r17:r16", [R16, R17]>, DwarfRegNum<[16]>;
+  def R15R14 : AVRReg<7, "r15:r14", [R14, R15]>, DwarfRegNum<[14]>;
+  def R13R12 : AVRReg<6, "r13:r12", [R12, R13]>, DwarfRegNum<[12]>;
+  def R11R10 : AVRReg<5, "r11:r10", [R10, R11]>, DwarfRegNum<[10]>;
+  def R9R8 : AVRReg<4, "r9:r8", [R8, R9]>, DwarfRegNum<[8]>;
+  def R7R6 : AVRReg<3, "r7:r6", [R6, R7]>, DwarfRegNum<[6]>;
+  def R5R4 : AVRReg<2, "r5:r4", [R4, R5]>, DwarfRegNum<[4]>;
+  def R3R2 : AVRReg<1, "r3:r2", [R2, R3]>, DwarfRegNum<[2]>;
   def R1R0 : AVRReg<0, "r1:r0", [R0, R1]>, DwarfRegNum<[0]>;
 
-  // Pseudo registers for unaligned i16
+  // Pseudo registers for unaligned i16. These are only used in pseudo
+  // instructions, so encoding values are arbitrary.
   def R26R25 : AVRReg<25, "r26:r25", [R25, R26]>, DwarfRegNum<[25]>;
   def R24R23 : AVRReg<23, "r24:r23", [R23, R24]>, DwarfRegNum<[23]>;
   def R22R21 : AVRReg<21, "r22:r21", [R21, R22]>, DwarfRegNum<[21]>;
diff --git a/llvm/lib/Target/AVR/Disassembler/AVRDisassembler.cpp b/llvm/lib/Target/AVR/Disassembler/AVRDisassembler.cpp
@@ -61,14 +61,21 @@ LLVMInitializeAVRDisassembler() {
                                          createAVRDisassembler);
 }
 
-static const uint16_t GPRDecoderTable[] = {
+static constexpr MCRegister GPRDecoderTable[] = {
     AVR::R0,  AVR::R1,  AVR::R2,  AVR::R3,  AVR::R4,  AVR::R5,  AVR::R6,
     AVR::R7,  AVR::R8,  AVR::R9,  AVR::R10, AVR::R11, AVR::R12, AVR::R13,
     AVR::R14, AVR::R15, AVR::R16, AVR::R17, AVR::R18, AVR::R19, AVR::R20,
     AVR::R21, AVR::R22, AVR::R23, AVR::R24, AVR::R25, AVR::R26, AVR::R27,
     AVR::R28, AVR::R29, AVR::R30, AVR::R31,
 };
 
+static constexpr MCRegister GPRPairDecoderTable[] = {
+    AVR::R1R0,   AVR::R3R2,   AVR::R5R4,   AVR::R7R6,
+    AVR::R9R8,   AVR::R11R10, AVR::R13R12, AVR::R15R14,
+    AVR::R17R16, AVR::R19R18, AVR::R21R20, AVR::R23R22,
+    AVR::R25R24, AVR::R27R26, AVR::R29R28, AVR::R31R30,
+};
+
 static DecodeStatus DecodeGPR8RegisterClass(MCInst &Inst, unsigned RegNo,
                                             uint64_t Address,
                                             const MCDisassembler *Decoder) {
@@ -98,6 +105,23 @@ static DecodeStatus DecodeLD8loRegisterClass(MCInst &Inst, unsigned RegNo,
   return MCDisassembler::Success;
 }
 
+static DecodeStatus DecodeDREGSRegisterClass(MCInst &Inst, unsigned RegNo,
+                                             uint64_t Address,
+                                             const MCDisassembler *Decoder) {
+  assert(isUInt<4>(RegNo));
+  Inst.addOperand(MCOperand::createReg(GPRPairDecoderTable[RegNo]));
+  return MCDisassembler::Success;
+}
+
+static DecodeStatus DecodeIWREGSRegisterClass(MCInst &Inst, unsigned RegNo,
+                                              uint64_t Address,
+                                              const MCDisassembler *Decoder) {
+  assert(isUInt<2>(RegNo));
+  // Only AVR::R25R24, AVR::R27R26, AVR::R29R28, AVR::R31R30 are legal.
+  Inst.addOperand(MCOperand::createReg(GPRPairDecoderTable[12 + RegNo]));
+  return MCDisassembler::Success;
+}
+
 static DecodeStatus DecodeZREGRegisterClass(MCInst &Inst,
                                             const MCDisassembler *Decoder) {
   Inst.addOperand(MCOperand::createReg(AVR::R31R30));
@@ -129,36 +153,6 @@ static DecodeStatus decodeRelCondBrTarget13(MCInst &Inst, unsigned Field,
   return MCDisassembler::Success;
 }
 
-static DecodeStatus decodeFMOVWRdRr(MCInst &Inst, unsigned Insn,
-                                    uint64_t Address,
-                                    const MCDisassembler *Decoder) {
-  unsigned r = fieldFromInstruction(Insn, 4, 4) * 2;
-  unsigned d = fieldFromInstruction(Insn, 0, 4) * 2;
-  if (DecodeGPR8RegisterClass(Inst, r, Address, Decoder) ==
-      MCDisassembler::Fail)
-    return MCDisassembler::Fail;
-  if (DecodeGPR8RegisterClass(Inst, d, Address, Decoder) ==
-      MCDisassembler::Fail)
-    return MCDisassembler::Fail;
-  return MCDisassembler::Success;
-}
-
-static DecodeStatus decodeFWRdK(MCInst &Inst, unsigned Insn, uint64_t Address,
-                                const MCDisassembler *Decoder) {
-  unsigned d = fieldFromInstruction(Insn, 4, 2) * 2 + 24; // starts at r24:r25
-  unsigned k = 0;
-  k |= fieldFromInstruction(Insn, 0, 4);
-  k |= fieldFromInstruction(Insn, 6, 2) << 4;
-  if (DecodeGPR8RegisterClass(Inst, d, Address, Decoder) ==
-      MCDisassembler::Fail)
-    return MCDisassembler::Fail;
-  if (DecodeGPR8RegisterClass(Inst, d, Address, Decoder) ==
-      MCDisassembler::Fail)
-    return MCDisassembler::Fail;
-  Inst.addOperand(MCOperand::createImm(k));
-  return MCDisassembler::Success;
-}
-
 static DecodeStatus decodeMemri(MCInst &Inst, unsigned Insn, uint64_t Address,
                                 const MCDisassembler *Decoder) {
   // As in the EncoderMethod `AVRMCCodeEmitter::encodeMemri`, the memory
