[llvm] [AVR] Refactor MUL/FMUL instruction descriptions (NFC) (PR #156862)

Sergei Barannikov via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 5 01:27:20 PDT 2025


https://github.com/s-barannikov updated https://github.com/llvm/llvm-project/pull/156862

>From a385ba361005ad4f501eed2571e2893bf6958332 Mon Sep 17 00:00:00 2001
From: Sergei Barannikov <barannikov88 at gmail.com>
Date: Thu, 4 Sep 2025 14:33:38 +0300
Subject: [PATCH 1/2] [AVR] Refactor MUL/FMUL instruction descriptions

* Split MULSU format from MULS and fix the comment
* Remove custom decoder functions
* Remove unnecessary RdrS suffix from instruction names
---
 llvm/lib/Target/AVR/AVRISelDAGToDAG.cpp       |  2 +-
 llvm/lib/Target/AVR/AVRISelLowering.cpp       |  4 +-
 llvm/lib/Target/AVR/AVRInstrFormats.td        | 37 +++++++++++------
 llvm/lib/Target/AVR/AVRInstrInfo.td           | 30 +++++++-------
 .../AVR/Disassembler/AVRDisassembler.cpp      | 41 ++++---------------
 5 files changed, 52 insertions(+), 62 deletions(-)

diff --git a/llvm/lib/Target/AVR/AVRISelDAGToDAG.cpp b/llvm/lib/Target/AVR/AVRISelDAGToDAG.cpp
index 20d35340bd15a..9abeb39835521 100644
--- a/llvm/lib/Target/AVR/AVRISelDAGToDAG.cpp
+++ b/llvm/lib/Target/AVR/AVRISelDAGToDAG.cpp
@@ -514,7 +514,7 @@ bool AVRDAGToDAGISel::selectMultiplication(llvm::SDNode *N) {
   assert(Type == MVT::i8 && "unexpected value type");
 
   bool isSigned = N->getOpcode() == ISD::SMUL_LOHI;
-  unsigned MachineOp = isSigned ? AVR::MULSRdRr : AVR::MULRdRr;
+  unsigned MachineOp = isSigned ? AVR::MULS : AVR::MUL;
 
   SDValue Lhs = N->getOperand(0);
   SDValue Rhs = N->getOperand(1);
diff --git a/llvm/lib/Target/AVR/AVRISelLowering.cpp b/llvm/lib/Target/AVR/AVRISelLowering.cpp
index 545bc3af05383..0d883ebd2dc51 100644
--- a/llvm/lib/Target/AVR/AVRISelLowering.cpp
+++ b/llvm/lib/Target/AVR/AVRISelLowering.cpp
@@ -2312,8 +2312,8 @@ AVRTargetLowering::EmitInstrWithCustomInserter(MachineInstr &MI,
   case AVR::Lsr32:
   case AVR::Asr32:
     return insertWideShift(MI, MBB);
-  case AVR::MULRdRr:
-  case AVR::MULSRdRr:
+  case AVR::MUL:
+  case AVR::MULS:
     return insertMul(MI, MBB);
   case AVR::CopyZero:
     return insertCopyZero(MI, MBB);
diff --git a/llvm/lib/Target/AVR/AVRInstrFormats.td b/llvm/lib/Target/AVR/AVRInstrFormats.td
index 65a229c13d5aa..6b974ba7fc560 100644
--- a/llvm/lib/Target/AVR/AVRInstrFormats.td
+++ b/llvm/lib/Target/AVR/AVRInstrFormats.td
@@ -228,22 +228,37 @@ class FMOVWRdRr<dag outs, dag ins, string asmstr, list<dag> pattern>
 }
 
 //===----------------------------------------------------------------------===//
-// MULSrr special encoding: <|0000|0010|dddd|rrrr|>
+// MULS special encoding: <|0000|0010|dddd|rrrr|>
 // d = multiplicand = 4 bits
 // r = multiplier = 4 bits
 // (Only accepts r16-r31)
 //===----------------------------------------------------------------------===//
-class FMUL2RdRr<bit f, dag outs, dag ins, string asmstr, list<dag> pattern>
+class FMULS<dag outs, dag ins, string asmstr, list<dag> pattern>
     : AVRInst16<outs, ins, asmstr, pattern> {
-  bits<5> rd; // accept 5 bits but only encode the lower 4
-  bits<5> rr; // accept 5 bits but only encode the lower 4
+  bits<4> rd;
+  bits<4> rr;
 
-  let Inst{15 - 9} = 0b0000001;
-  let Inst{8} = f;
-  let Inst{7 - 4} = rd{3 - 0};
-  let Inst{3 - 0} = rr{3 - 0};
+  let Inst{15 - 8} = 0b00000010;
+  let Inst{7 - 4} = rd;
+  let Inst{3 - 0} = rr;
+}
+
+//===----------------------------------------------------------------------===//
+// MULSU special encoding: <|0000|0011|0ddd|0rrr|>
+// d = multiplicand = 3 bits
+// r = multiplier = 3 bits
+// (Only accepts r16-r23)
+//===----------------------------------------------------------------------===//
+class FMULSU<dag outs, dag ins, string asmstr, list<dag> pattern>
+    : AVRInst16<outs, ins, asmstr, pattern> {
+  bits<3> rd;
+  bits<3> rr;
 
-  let DecoderMethod = "decodeFMUL2RdRr";
+  let Inst{15 - 8} = 0b00000011;
+  let Inst{7} = 0;
+  let Inst{6 - 4} = rd;
+  let Inst{3} = 0;
+  let Inst{2 - 0} = rr;
 }
 
 // Special encoding for the FMUL family of instructions.
@@ -256,7 +271,7 @@ class FMUL2RdRr<bit f, dag outs, dag ins, string asmstr, list<dag> pattern>
 //
 // ddd = destination register
 // rrr = source register
-class FFMULRdRr<bits<2> f, dag outs, dag ins, string asmstr, list<dag> pattern>
+class FFMUL<bits<2> f, dag outs, dag ins, string asmstr, list<dag> pattern>
     : AVRInst16<outs, ins, asmstr, pattern> {
   bits<3> rd;
   bits<3> rr;
@@ -266,8 +281,6 @@ class FFMULRdRr<bits<2> f, dag outs, dag ins, string asmstr, list<dag> pattern>
   let Inst{6 - 4} = rd;
   let Inst{3} = f{0};
   let Inst{2 - 0} = rr;
-
-  let DecoderMethod = "decodeFFMULRdRr";
 }
 
 //===----------------------------------------------------------------------===//
diff --git a/llvm/lib/Target/AVR/AVRInstrInfo.td b/llvm/lib/Target/AVR/AVRInstrInfo.td
index 6ecd82bfb8739..9d4df69353be7 100644
--- a/llvm/lib/Target/AVR/AVRInstrInfo.td
+++ b/llvm/lib/Target/AVR/AVRInstrInfo.td
@@ -491,29 +491,29 @@ let isCommutable = 1, Defs = [R1, R0, SREG] in {
   // MUL Rd, Rr
   // Multiplies Rd by Rr and places the result into R1:R0.
   let usesCustomInserter = 1 in {
-    def MULRdRr : FRdRr<0b1001, 0b11, (outs), (ins GPR8:$rd, GPR8:$rr),
-                        "mul\t$rd, $rr", []>,
-                  Requires<[SupportsMultiplication]>;
+    def MUL : FRdRr<0b1001, 0b11, (outs), (ins GPR8:$rd, GPR8:$rr),
+                    "mul\t$rd, $rr", []>,
+              Requires<[SupportsMultiplication]>;
 
-    def MULSRdRr : FMUL2RdRr<0, (outs), (ins LD8:$rd, LD8:$rr),
-                             "muls\t$rd, $rr", []>,
-                   Requires<[SupportsMultiplication]>;
+    def MULS : FMULS<(outs), (ins LD8:$rd, LD8:$rr),
+                     "muls\t$rd, $rr", []>,
+               Requires<[SupportsMultiplication]>;
   }
 
-  def MULSURdRr : FMUL2RdRr<1, (outs), (ins LD8lo:$rd, LD8lo:$rr),
-                            "mulsu\t$rd, $rr", []>,
-                  Requires<[SupportsMultiplication]>;
+  def MULSU : FMULSU<(outs), (ins LD8lo:$rd, LD8lo:$rr),
+                     "mulsu\t$rd, $rr", []>,
+              Requires<[SupportsMultiplication]>;
 
-  def FMUL : FFMULRdRr<0b01, (outs), (ins LD8lo:$rd, LD8lo:$rr),
-                       "fmul\t$rd, $rr", []>,
+  def FMUL : FFMUL<0b01, (outs), (ins LD8lo:$rd, LD8lo:$rr),
+                   "fmul\t$rd, $rr", []>,
              Requires<[SupportsMultiplication]>;
 
-  def FMULS : FFMULRdRr<0b10, (outs), (ins LD8lo:$rd, LD8lo:$rr),
-                        "fmuls\t$rd, $rr", []>,
+  def FMULS : FFMUL<0b10, (outs), (ins LD8lo:$rd, LD8lo:$rr),
+                    "fmuls\t$rd, $rr", []>,
               Requires<[SupportsMultiplication]>;
 
-  def FMULSU : FFMULRdRr<0b11, (outs), (ins LD8lo:$rd, LD8lo:$rr),
-                         "fmulsu\t$rd, $rr", []>,
+  def FMULSU : FFMUL<0b11, (outs), (ins LD8lo:$rd, LD8lo:$rr),
+                     "fmulsu\t$rd, $rr", []>,
                Requires<[SupportsMultiplication]>;
 }
 
diff --git a/llvm/lib/Target/AVR/Disassembler/AVRDisassembler.cpp b/llvm/lib/Target/AVR/Disassembler/AVRDisassembler.cpp
index c87887f17f914..d874e2deb2ce9 100644
--- a/llvm/lib/Target/AVR/Disassembler/AVRDisassembler.cpp
+++ b/llvm/lib/Target/AVR/Disassembler/AVRDisassembler.cpp
@@ -83,11 +83,16 @@ static DecodeStatus DecodeGPR8RegisterClass(MCInst &Inst, unsigned RegNo,
 static DecodeStatus DecodeLD8RegisterClass(MCInst &Inst, unsigned RegNo,
                                            uint64_t Address,
                                            const MCDisassembler *Decoder) {
-  if (RegNo > 15)
-    return MCDisassembler::Fail;
+  assert(isUInt<4>(RegNo));
+  Inst.addOperand(MCOperand::createReg(GPRDecoderTable[16 + RegNo]));
+  return MCDisassembler::Success;
+}
 
-  unsigned Register = GPRDecoderTable[RegNo + 16];
-  Inst.addOperand(MCOperand::createReg(Register));
+static DecodeStatus DecodeLD8loRegisterClass(MCInst &Inst, unsigned RegNo,
+                                             uint64_t Address,
+                                             const MCDisassembler *Decoder) {
+  assert(isUInt<3>(RegNo));
+  Inst.addOperand(MCOperand::createReg(GPRDecoderTable[16 + RegNo]));
   return MCDisassembler::Success;
 }
 
@@ -122,20 +127,6 @@ static DecodeStatus decodeRelCondBrTarget13(MCInst &Inst, unsigned Field,
   return MCDisassembler::Success;
 }
 
-static DecodeStatus decodeFFMULRdRr(MCInst &Inst, unsigned Insn,
-                                    uint64_t Address,
-                                    const MCDisassembler *Decoder) {
-  unsigned d = fieldFromInstruction(Insn, 4, 3) + 16;
-  unsigned r = fieldFromInstruction(Insn, 0, 3) + 16;
-  if (DecodeGPR8RegisterClass(Inst, d, Address, Decoder) ==
-      MCDisassembler::Fail)
-    return MCDisassembler::Fail;
-  if (DecodeGPR8RegisterClass(Inst, r, Address, Decoder) ==
-      MCDisassembler::Fail)
-    return MCDisassembler::Fail;
-  return MCDisassembler::Success;
-}
-
 static DecodeStatus decodeFMOVWRdRr(MCInst &Inst, unsigned Insn,
                                     uint64_t Address,
                                     const MCDisassembler *Decoder) {
@@ -166,20 +157,6 @@ static DecodeStatus decodeFWRdK(MCInst &Inst, unsigned Insn, uint64_t Address,
   return MCDisassembler::Success;
 }
 
-static DecodeStatus decodeFMUL2RdRr(MCInst &Inst, unsigned Insn,
-                                    uint64_t Address,
-                                    const MCDisassembler *Decoder) {
-  unsigned rd = fieldFromInstruction(Insn, 4, 4) + 16;
-  unsigned rr = fieldFromInstruction(Insn, 0, 4) + 16;
-  if (DecodeGPR8RegisterClass(Inst, rd, Address, Decoder) ==
-      MCDisassembler::Fail)
-    return MCDisassembler::Fail;
-  if (DecodeGPR8RegisterClass(Inst, rr, Address, Decoder) ==
-      MCDisassembler::Fail)
-    return MCDisassembler::Fail;
-  return MCDisassembler::Success;
-}
-
 static DecodeStatus decodeMemri(MCInst &Inst, unsigned Insn, uint64_t Address,
                                 const MCDisassembler *Decoder) {
   // As in the EncoderMethod `AVRMCCodeEmitter::encodeMemri`, the memory

>From 2ab53b69796817d5947ca5cd986f91bd15e58ce8 Mon Sep 17 00:00:00 2001
From: Sergei Barannikov <barannikov88 at gmail.com>
Date: Fri, 5 Sep 2025 11:27:07 +0300
Subject: [PATCH 2/2] Add a comment about acceptable registers

---
 llvm/lib/Target/AVR/Disassembler/AVRDisassembler.cpp | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/llvm/lib/Target/AVR/Disassembler/AVRDisassembler.cpp b/llvm/lib/Target/AVR/Disassembler/AVRDisassembler.cpp
index d874e2deb2ce9..a3dfc941962d6 100644
--- a/llvm/lib/Target/AVR/Disassembler/AVRDisassembler.cpp
+++ b/llvm/lib/Target/AVR/Disassembler/AVRDisassembler.cpp
@@ -84,6 +84,7 @@ static DecodeStatus DecodeLD8RegisterClass(MCInst &Inst, unsigned RegNo,
                                            uint64_t Address,
                                            const MCDisassembler *Decoder) {
   assert(isUInt<4>(RegNo));
+  // Only r16...r31 are legal.
   Inst.addOperand(MCOperand::createReg(GPRDecoderTable[16 + RegNo]));
   return MCDisassembler::Success;
 }
@@ -92,6 +93,7 @@ static DecodeStatus DecodeLD8loRegisterClass(MCInst &Inst, unsigned RegNo,
                                              uint64_t Address,
                                              const MCDisassembler *Decoder) {
   assert(isUInt<3>(RegNo));
+  // Only r16...r23 are legal.
   Inst.addOperand(MCOperand::createReg(GPRDecoderTable[16 + RegNo]));
   return MCDisassembler::Success;
 }



More information about the llvm-commits mailing list