[MC][AVR] Implement decoding STD/LDD

[benshi001]

This comment was removed by benshi001.

[benshi001]

This comment has been deleted.

llvm/lib/Target/AVR/Disassembler/AVRDisassembler.cpp

279

LDD/STD should be fully decoded by decodeMemri, but due to very complex reason, decodeMemri only correctly decodes LDD/STD with offset > 7, and the case offset <= 7 falls into decodeLoadStore (decoder method of LD/ST).

We should re-think the organization of definition of LD/ST/LDD/STD in TD files to avoid that. But currently I can not figure out a proper way, it invloves register allocation.

[benshi001]

This patch fixes the crash reported in https://github.com/llvm/llvm-project/issues/54776.

But properly decoding LD/ST with postinc/predec will be fixed in a new patch.

[aykevl]

Looks good to me.

llvm/test/MC/AVR/inst-ldd.s

12

This instruction is not tested with an INST: line, is that intentional?

Same for std in the next file.

[benshi001]

llvm/test/MC/AVR/inst-ldd.s

12

That is intentional, since foo is an unresolved symbol.

llvm/lib/Target/AVR/AVRInstrFormats.td

 // An ST/LD instruction.
 // <|100i|00tr|rrrr|ppaa|>
 // t = type (1 for store, 0 for load)
 // a = regular/postinc/predec (reg = 0b00, postinc = 0b01, predec = 0b10)
 // p = pointer register
 // r = src/dst register
 //
 // Note that the bit labelled 'i' above does not follow a simple pattern,
-// so there exists a post encoder method to set it manually.
+// so there exists a post encoder method to set it manually. Also a specified
+// decoder method is needed.
 //===---------------------------------------------------------------------===//
 class FSTLD<bit type, bits<2> mode, dag outs, dag ins, string asmstr,
             list<dag> pattern> : AVRInst16<outs, ins, asmstr, pattern> {
   bits<2> ptrreg;
   bits<5> reg;
 
   let Inst{15 - 13} = 0b100;
   // This bit varies depending on the arguments and the mode.
   // We have a post encoder method to set this bit manually.
   let Inst{12} = 0;
 
   let Inst{11 - 10} = 0b00;
   let Inst{9} = type;
   let Inst{8} = reg{4};
 
   let Inst{7 - 4} = reg{3 - 0};
 
   let Inst{3 - 2} = ptrreg{1 - 0};
   let Inst{1 - 0} = mode{1 - 0};
 
+  let DecoderMethod = "decodeLoadStore";
   let PostEncoderMethod = "loadStorePostEncoder";
 }
 
 //===---------------------------------------------------------------------===//
 // Special format for the LPM/ELPM instructions
 // [E]LPM Rd, Z[+]
 // <|1001|000d|dddd|01ep>
 // d = destination register

llvm/lib/Target/AVR/AVRInstrInfo.td

 }
 
 /// Address operand for `reg+imm` used by STD and LDD.
 def memri : Operand<iPTR> {
   let MIOperandInfo = (ops PTRDISPREGS, i16imm);
 
   let PrintMethod = "printMemri";
   let EncoderMethod = "encodeMemri";
+  let DecoderMethod = "decodeMemri";
 
   let ParserMatchClass = MemriAsmOperand;
 }
 
 // Address operand for `SP+imm` used by STD{W}SPQRr
 def memspi : Operand<iPTR> { let MIOperandInfo = (ops GPRSP, i16imm); }
 
 def relbrtarget_7 : Operand<OtherVT> {

llvm/lib/Target/AVR/Disassembler/AVRDisassembler.cpp

 
 static DecodeStatus decodeFWRdK(MCInst &Inst, unsigned Insn, uint64_t Address,
                                 const MCDisassembler *Decoder);
 
 static DecodeStatus decodeFMUL2RdRr(MCInst &Inst, unsigned Insn,
                                     uint64_t Address,
                                     const MCDisassembler *Decoder);
 
+static DecodeStatus decodeMemri(MCInst &Inst, unsigned Insn, uint64_t Address,
+                                const MCDisassembler *Decoder);
+
+static DecodeStatus decodeLoadStore(MCInst &Inst, unsigned Insn,
+                                    uint64_t Address,
+                                    const MCDisassembler *Decoder);
+
 #include "AVRGenDisassemblerTables.inc"
 
 static DecodeStatus decodeFIOARr(MCInst &Inst, unsigned Insn, uint64_t Address,
                                  const MCDisassembler *Decoder) {
   unsigned addr = 0;
   addr |= fieldFromInstruction(Insn, 0, 4);
   addr |= fieldFromInstruction(Insn, 9, 2) << 4;
   unsigned reg = fieldFromInstruction(Insn, 4, 5);
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
+  // address is encoded into 7-bit, in which bits 0-5 are the immediate offset,
+  // and the bit-6 is the pointer register bit (Z=0, Y=1).
+  if (Insn > 127)
+    return MCDisassembler::Fail;
+
+  // Append the base register operand.
+  Inst.addOperand(
+      MCOperand::createReg((Insn & 0x40) ? AVR::R29R28 : AVR::R31R30));
+  // Append the immediate offset operand.
+  Inst.addOperand(MCOperand::createImm(Insn & 0x3f));
+
+  return MCDisassembler::Success;
+}
+
+static DecodeStatus decodeLoadStore(MCInst &Inst, unsigned Insn,
+                                    uint64_t Address,
+                                    const MCDisassembler *Decoder) {
+  // Decode LDD/STD with offset less than 8.
+  if ((Insn & 0xf000) == 0x8000) {
+    unsigned RegVal = GPRDecoderTable[(Insn >> 4) & 0x1f];
+    unsigned RegBase = (Insn & 0x8) ? AVR::R29R28 : AVR::R31R30;
+    unsigned Offset = Insn & 7; // We need not consider offset > 7.
+    if ((Insn & 0x200) == 0) { // Decode LDD.
+      Inst.setOpcode(AVR::LDDRdPtrQ);
+      Inst.addOperand(MCOperand::createReg(RegVal));
+      Inst.addOperand(MCOperand::createReg(RegBase));
+      Inst.addOperand(MCOperand::createImm(Offset));
+    } else { // Decode STD.
+      Inst.setOpcode(AVR::STDPtrQRr);
+      Inst.addOperand(MCOperand::createReg(RegBase));
+      Inst.addOperand(MCOperand::createImm(Offset));
+      Inst.addOperand(MCOperand::createReg(RegVal));
+    }
+    return MCDisassembler::Success;
+  }
+
+  // TODO: Decode ST/LD with postinc/predec properly.
+  return MCDisassembler::Fail;
+}
+
 static DecodeStatus readInstruction16(ArrayRef<uint8_t> Bytes, uint64_t Address,
                                       uint64_t &Size, uint32_t &Insn) {
   if (Bytes.size() < 2) {
     Size = 0;
     return MCDisassembler::Fail;
   }
 
   Size = 2;

llvm/test/MC/AVR/inst-ldd.s

 ; RUN: llvm-mc -triple avr -mattr=sram -show-encoding < %s | FileCheck %s
-
+; RUN: llvm-mc -filetype=obj -triple avr -mattr=sram < %s \
+; RUN:     | llvm-objdump -d --mattr=sram - | FileCheck --check-prefix=INST %s
 
 foo:
   ldd r2, Y+2
   ldd r0, Y+0
 
   ldd r9, Z+12
   ldd r7, Z+30
 
   ldd r9, Z+foo
 
 ; CHECK: ldd r2, Y+2                  ; encoding: [0x2a,0x80]
 ; CHECK: ldd r0, Y+0                  ; encoding: [0x08,0x80]
 
 ; CHECK: ldd r9, Z+12                 ; encoding: [0x94,0x84]
 ; CHECK: ldd r7, Z+30                 ; encoding: [0x76,0x8c]
 
 ; CHECK: ldd r9, Z+foo                ; encoding: [0x90'A',0x80'A']
 ; CHECK:                              ;   fixup A - offset: 0, value: +foo, kind: fixup_6
+
+; INST: ldd r2, Y+2
+; INST: ldd r0, Y+0
+; INST: ldd r9, Z+12
+; INST: ldd r7, Z+30

llvm/test/MC/AVR/inst-std.s

 ; RUN: llvm-mc -triple avr -mattr=sram -show-encoding < %s | FileCheck %s
-
+; RUN: llvm-mc -filetype=obj -triple avr -mattr=sram < %s \
+; RUN:     | llvm-objdump -d --mattr=sram - | FileCheck --check-prefix=INST %s
 
 foo:
 
   std Y+2, r2
   std Y+0, r0
 
   std Z+12, r9
   std Z+30, r7
 
   std Y+foo, r9
 
 ; CHECK: std Y+2,  r2                 ; encoding: [0x2a,0x82]
 ; CHECK: std Y+0,  r0                 ; encoding: [0x08,0x82]
 
 ; CHECK: std Z+12, r9                 ; encoding: [0x94,0x86]
 ; CHECK: std Z+30, r7                 ; encoding: [0x76,0x8e]
 
 ; CHECK: std Y+foo, r9                ; encoding: [0x98'A',0x82'A']
 ; CHECK:                              ;   fixup A - offset: 0, value: +foo, kind: fixup_6
 
+; INST: std Y+2, r2
+; INST: std Y+0, r0
+; INST: std Z+12, r9
+; INST: std Z+30, r7