[AVR] Improve AVR disassembly

[amikhalev]

Summary

• Add decoder for relative branch target operands.
• Add decoder for memri (reg + imm) operands.
• Change the way FSTLD instruction format is handled to be more compatible with disassembly. Instead of setting the inconsistent bit in a post encoder method, have it be part of the ptrreg operand so the LDSTDPtrReg encoder/decoder can handle it.
• Implement MCInstrAnalysis for AVR so llvm-objdump can show (some) branch targets.

[amikhalev]

Fix clang-tidy warnings

[dylanmckay]

The code is looking nice, thanks for the patch @amikhalev!

I've checked over everything except the bitmasks/bit positions and left a few comments. I intend to validate the bitmasks sometime in the next few days.

llvm/lib/Target/AVR/AVRInstrInfo.td

170

Add a LLVM unit test that covers this (suggest somewhere in test/MC/AVR), perhaps using something like inst-sts.s as an example.

186

Add a unit test for this new instruction decoder

194

Add a unit test for this new instruction decoder

254

Add a unit test for this new instruction decoder

258

I hadn't seen this TableGen flag before, nice

270

Add a unit test for this new instruction decoder

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

95

I too love using binary literal notation, but sadly I believe it is not fully supported by the minimum C++ toolchain versions LLVM supports. Specifically, binary literals are not mentioned in the C++ feature "whitelist"[1], nor can I spot any existing uses in LLVM via a quick grep, plus I remember committing a change that included binary literals not too long ago that failed on the buildbots due to lack of support.

Replace these binary literals with hex or decimal equivalents. If you want the binary representation to be shown, then it will have to be relegated to a code comment

• [0] LLVM C++ feature whitelist https://releases.llvm.org/8.0.0/docs/CodingStandards.html#supported-c-11-language-and-library-features

158

Nitpick: YOrZ variable implies a boolean value. It is fairly clear in the definition and usage that it is always treated like a boolean value, but I recommend moving the == 0 check up to the definition of YOrZ to make it clear right at the site of its initial declaration/definition. For example bool YOrZ = fieldFromInstruction(Insn, 6, 1) == 0

163

Nitpick: Captialize Displacement

209

I haven't looked deep into this sign extension implementation yet but recommend replacing the hand rolled SExt with a call to one of the generalized methods in MathExtras.h, for example, SignExtend64

235

Add some kind of assertion here to validate that you're covering all possible cases of input opcode without incorrectly falling back to IsPredec==false && isPostinc==false. The simplest solution might be done with switch but perhaps there is a more terse way to do it as this current code reads very nicely

llvm/lib/Target/AVR/MCTargetDesc/AVRMCTargetDesc.cpp

13

Nitpick: Either place a new line after this "primary" include to give it special vertical positioning, or order it alphabetically along with the other includes

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 it is an additional field in ptrreg which is encoded/decoded by 
+// LDSTPtrReg.
 //===---------------------------------------------------------------------===//
 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<3> 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;
+  // This bit varies depending on the pointer arguments and the mode.
+  // We have a operand encoder method to set this bit manually.
+  let Inst{12} = ptrreg{2};
 
   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 PostEncoderMethod = "loadStorePostEncoder";
 }
 
 //===---------------------------------------------------------------------===//
 // Special format for the LPM/ELPM instructions
 // [E]LPM Rd, Z[+]
 // <|1001|000d|dddd|01ep>
 // d = destination register
 // e = is elpm

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>
 {
+    let OperandType = "OPERAND_PCREL";
     let PrintMethod   = "printPCRelImm";
     let EncoderMethod = "encodeRelCondBrTarget<AVR::fixup_7_pcrel>";
+    let DecoderMethod = "decodeRelCondBrTarget<7>";
 }
 
 def brtarget_13 : Operand<OtherVT>
 {
+    let OperandType = "OPERAND_PCREL";
     let PrintMethod   = "printPCRelImm";
     let EncoderMethod = "encodeRelCondBrTarget<AVR::fixup_13_pcrel>";
+    let DecoderMethod = "decodeRelCondBrTarget<13>";
 }
 
 // The target of a 22 or 16-bit call/jmp instruction.
 def call_target : Operand<iPTR>
 {
     let EncoderMethod = "encodeCallTarget";
     let DecoderMethod = "decodeCallTarget";
 }

 
 // A special operand type for the LD/ST instructions.
 // It converts the pointer register number into a two-bit field used in the
 // instruction.
 def LDSTPtrReg : Operand<i16>
 {
     let MIOperandInfo = (ops PTRREGS);
     let EncoderMethod = "encodeLDSTPtrReg";
+    let DecoderMethod = "decodeLDSTPtrReg";
+
+    // May try multiple instructions while figuring out the value of the
+    // inconsistent bit.
+    let hasCompleteDecoder = 0;
 
     let ParserMatchClass = PtrRegAsmOperand;
 }
 
 // A special operand type for the LDD/STD instructions.
 // It behaves identically to the LD/ST version, except restricts
 // the pointer registers to Y and Z.
 def LDDSTDPtrReg : Operand<i16>
 {
     let MIOperandInfo = (ops PTRDISPREGS);
     let EncoderMethod = "encodeLDSTPtrReg";
+    let DecoderMethod = "decodeLDSTPtrReg";
 
     let ParserMatchClass = PtrRegAsmOperand;
 }
 
 //===----------------------------------------------------------------------===//
 // AVR predicates for subtarget features
 //===----------------------------------------------------------------------===//
 

 // Jump instructions
 //===----------------------------------------------------------------------===//
 let isBarrier = 1,
 isBranch = 1,
 isTerminator = 1 in
 {
   def RJMPk : FBRk<0,
                    (outs),
-                   (ins brtarget_13:$target),
-                   "rjmp\t$target",
-                   [(br bb:$target)]>;
+                   (ins brtarget_13:$k),
+                   "rjmp\t$k",
+                   [(br bb:$k)]>;
 
   let isIndirectBranch = 1,
   Uses = [R31R30] in
   def IJMP : F16<0b1001010000001001,
                  (outs),
                  (ins),
                  "ijmp",
                  []>,

 //===----------------------------------------------------------------------===//
 let isCall = 1 in
 {
   // SP is marked as a use to prevent stack-pointer assignments that appear
   // immediately before calls from potentially appearing dead.
   let Uses = [SP] in
   def RCALLk : FBRk<1,
                     (outs),
-                    (ins brtarget_13:$target),
-                    "rcall\t$target",
+                    (ins brtarget_13:$k),
+                    "rcall\t$k",
                     []>;
 
   // SP is marked as a use to prevent stack-pointer assignments that appear
   // immediately before calls from potentially appearing dead.
   let Uses = [SP, R31R30] in
   def ICALL : F16<0b1001010100001001,
                   (outs),
                   (ins variable_ops),

 // because we also need to be able to specify a pattern to match for ISel.
 let isBranch = 1,
 isTerminator = 1,
 Uses = [SREG] in
 {
   def BREQk : FBRsk<0,
                     0b001,
                     (outs),
-                    (ins relbrtarget_7:$target),
-                    "breq\t$target",
-                    [(AVRbrcond bb:$target, AVR_COND_EQ)]>;
+                    (ins relbrtarget_7:$k),
+                    "breq\t$k",
+                    [(AVRbrcond bb:$k, AVR_COND_EQ)]>;
 
   def BRNEk : FBRsk<1,
                     0b001,
                     (outs),
-                    (ins relbrtarget_7:$target),
-                    "brne\t$target",
-                    [(AVRbrcond bb:$target, AVR_COND_NE)]>;
+                    (ins relbrtarget_7:$k),
+                    "brne\t$k",
+                    [(AVRbrcond bb:$k, AVR_COND_NE)]>;
 
 
   def BRSHk : FBRsk<1,
                     0b000,
                     (outs),
-                    (ins relbrtarget_7:$target),
-                    "brsh\t$target",
-                    [(AVRbrcond bb:$target, AVR_COND_SH)]>;
+                    (ins relbrtarget_7:$k),
+                    "brsh\t$k",
+                    [(AVRbrcond bb:$k, AVR_COND_SH)]>;
 
   def BRLOk : FBRsk<0,
                     0b000,
                     (outs),
-                    (ins relbrtarget_7:$target),
-                    "brlo\t$target",
-                    [(AVRbrcond bb:$target, AVR_COND_LO)]>;
+                    (ins relbrtarget_7:$k),
+                    "brlo\t$k",
+                    [(AVRbrcond bb:$k, AVR_COND_LO)]>;
 
   def BRMIk : FBRsk<0,
                     0b010,
                     (outs),
                     (ins relbrtarget_7:$target),
                     "brmi\t$target",
                     [(AVRbrcond bb:$target, AVR_COND_MI)]>;
 
   def BRPLk : FBRsk<1,
                     0b010,
                     (outs),
-                    (ins relbrtarget_7:$target),
-                    "brpl\t$target",
-                    [(AVRbrcond bb:$target, AVR_COND_PL)]>;
+                    (ins relbrtarget_7:$k),
+                    "brpl\t$k",
+                    [(AVRbrcond bb:$k, AVR_COND_PL)]>;
 
   def BRGEk : FBRsk<1,
                     0b100,
                     (outs),
-                    (ins relbrtarget_7:$target),
-                    "brge\t$target",
-                    [(AVRbrcond bb:$target, AVR_COND_GE)]>;
+                    (ins relbrtarget_7:$k),
+                    "brge\t$k",
+                    [(AVRbrcond bb:$k, AVR_COND_GE)]>;
 
   def BRLTk : FBRsk<0,
                     0b100,
                     (outs),
-                    (ins relbrtarget_7:$target),
-                    "brlt\t$target",
-                    [(AVRbrcond bb:$target, AVR_COND_LT)]>;
+                    (ins relbrtarget_7:$k),
+                    "brlt\t$k",
+                    [(AVRbrcond bb:$k, AVR_COND_LT)]>;
 }
 
 //===----------------------------------------------------------------------===//
 // Data transfer instructions
 //===----------------------------------------------------------------------===//
 // 8 and 16-bit register move instructions.
 let hasSideEffects = 0 in
 {

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

static DecodeStatus DecodeLD8RegisterClass(MCInst &Inst, unsigned RegNo,
 
   unsigned Register = GPRDecoderTable[RegNo+16];
   Inst.addOperand(MCOperand::createReg(Register));
   return MCDisassembler::Success;
 }
 
 static DecodeStatus DecodePTRREGSRegisterClass(MCInst &Inst, unsigned RegNo,
                                                uint64_t Address, const void *Decoder) {
-  // Note: this function must be defined but does not seem to be called.
-  assert(false && "unimplemented: PTRREGS register class");
+  unsigned Register;
+  switch (RegNo) {
+  case 0b00:
+    Register = AVR::R31R30; // Z
+    break;
+  case 0b10:
+    Register = AVR::R29R28; // Y
+    break;
+  case 0b11:
+    Register = AVR::R27R26; // X
+    break;
+  default:
+    return MCDisassembler::Fail;
+  }
+  Inst.addOperand(MCOperand::createReg(Register));
   return MCDisassembler::Success;
 }
 
+static DecodeStatus decodeMemri(MCInst &Inst, unsigned Insn, uint64_t Address,
+                                const void *Decoder);
+
 static DecodeStatus decodeFIOARr(MCInst &Inst, unsigned Insn,
                                  uint64_t Address, const void *Decoder);
 
 static DecodeStatus decodeFIORdA(MCInst &Inst, unsigned Insn,
                                  uint64_t Address, const void *Decoder);
 
 static DecodeStatus decodeFIOBIT(MCInst &Inst, unsigned Insn,
                                  uint64_t Address, const void *Decoder);
 
+template <unsigned Bits>
+static DecodeStatus decodeRelCondBrTarget(MCInst &Inst, unsigned Insn,
+                                          uint64_t Address,
+                                          const void *Decoder);
+
 static DecodeStatus decodeCallTarget(MCInst &Inst, unsigned Insn,
                                      uint64_t Address, const void *Decoder);
 
+static DecodeStatus decodeLDSTPtrReg(MCInst &Inst, unsigned Insn,
+                                     uint64_t Address, const void *Decoder);
+
 static DecodeStatus decodeFRd(MCInst &Inst, unsigned Insn,
                               uint64_t Address, const void *Decoder);
 
 static DecodeStatus decodeFLPMX(MCInst &Inst, unsigned Insn,
                                 uint64_t Address, const void *Decoder);
 
 static DecodeStatus decodeFFMULRdRr(MCInst &Inst, unsigned Insn,
                                     uint64_t Address, const void *Decoder);
 
 static DecodeStatus decodeFMOVWRdRr(MCInst &Inst, unsigned Insn,
                                     uint64_t Address, const void *Decoder);
 
 static DecodeStatus decodeFWRdK(MCInst &Inst, unsigned Insn,
                                 uint64_t Address, const void *Decoder);
 
 static DecodeStatus decodeFMUL2RdRr(MCInst &Inst, unsigned Insn,
                                     uint64_t Address, const void *Decoder);
 
 #include "AVRGenDisassemblerTables.inc"
 
+static DecodeStatus decodeMemri(MCInst &Inst, unsigned Insn, uint64_t Address,
+                                const void *Decoder) {
+  unsigned YOrZ = fieldFromInstruction(Insn, 6, 1);
+  unsigned Reg;
+  if (YOrZ == 0) {
+    Reg = AVR::R31R30; // Z
+  } else {
+    Reg = AVR::R29R28; // Y
+  }
+  unsigned displacement = fieldFromInstruction(Insn, 0, 6);
+  Inst.addOperand(MCOperand::createReg(Reg));
+  Inst.addOperand(MCOperand::createImm(displacement));
+  return MCDisassembler::Success;
+}
+
 static DecodeStatus decodeFIOARr(MCInst &Inst, unsigned Insn,
                                  uint64_t Address, const void *Decoder) {
   unsigned addr = 0;
   addr |= fieldFromInstruction(Insn, 0, 4);
   addr |= fieldFromInstruction(Insn, 9, 2) << 4;
   unsigned reg = fieldFromInstruction(Insn, 4, 5);
   Inst.addOperand(MCOperand::createImm(addr));
   if (DecodeGPR8RegisterClass(Inst, reg, Address, Decoder) == MCDisassembler::Fail)
static DecodeStatus decodeFIOBIT(MCInst &Inst, unsigned Insn,
                                  uint64_t Address, const void *Decoder) {
   unsigned addr = fieldFromInstruction(Insn, 3, 5);
   unsigned b = fieldFromInstruction(Insn, 0, 3);
   Inst.addOperand(MCOperand::createImm(addr));
   Inst.addOperand(MCOperand::createImm(b));
   return MCDisassembler::Success;
 }
 
+template <unsigned Bits>
+static DecodeStatus decodeRelCondBrTarget(MCInst &Inst, unsigned Field,
+                                          uint64_t Address,
+                                          const void *Decoder) {
+  // Branch targets need to be shifted left by one
+  uint64_t Target = Field << 1;
+  // Sign extend
+  const uint64_t ValueMask = (1 << (Bits - 1)) - 1;
+  const uint64_t SignMask = static_cast<uint64_t>(-1) - ValueMask;
+  Target = (Target & ValueMask) | ((Target & SignMask) ? SignMask : 0);
+  Inst.addOperand(MCOperand::createImm(static_cast<int64_t>(Target)));
+  return MCDisassembler::Success;
+}
+
 static DecodeStatus decodeCallTarget(MCInst &Inst, unsigned Field,
                                      uint64_t Address, const void *Decoder) {
   // Call targets need to be shifted left by one so this needs a custom
   // decoder.
   Inst.addOperand(MCOperand::createImm(Field << 1));
   return MCDisassembler::Success;
 }
 
+// See AVRMCCodeEmitter::encodeLDSTPtrReg for more information.
+static DecodeStatus decodeLDSTPtrReg(MCInst &Inst, unsigned Field,
+                                     uint64_t Address, const void *Decoder) {
+  unsigned RegNo = fieldFromInstruction(Field, 0, 2);
+  bool InconsistentBit = fieldFromInstruction(Field, 2, 1) != 0;
+
+  unsigned Opcode = Inst.getOpcode();
+
+  // check whether the register is the X pointer register.
+  bool IsRegX = RegNo == 0x03;
+
+  bool IsPredec = Opcode == AVR::LDRdPtrPd || Opcode == AVR::STPtrPdRr;
+  bool IsPostinc = Opcode == AVR::LDRdPtrPi || Opcode == AVR::STPtrPiRr;
+
+  // Check if this is the right instruction for the value of the
+  // inconsistent bit. If it is not, fail and we will try to decode
+  // with a different opcode.
+  if ((IsRegX || IsPredec || IsPostinc) != InconsistentBit) {
+    return DecodeStatus::Fail;
+  }
+
+  return DecodePTRREGSRegisterClass(Inst, RegNo, Address, Decoder);
+}
+
 static DecodeStatus decodeFRd(MCInst &Inst, unsigned Insn,
                               uint64_t Address, const void *Decoder) {
   unsigned d = fieldFromInstruction(Insn, 4, 5);
   if (DecodeGPR8RegisterClass(Inst, d, Address, Decoder) == MCDisassembler::Fail)
     return MCDisassembler::Fail;
   return MCDisassembler::Success;
 }
 

llvm/lib/Target/AVR/MCTargetDesc/AVRInstrAnalysis.h

+//===- AVRInstrAnalysis.h - Analyze AVR MCInst ------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This class analyzes AVR MCInst. In particular, it implements evaluateBranch
+// so llvm-objdump can show branch targets.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_AVR_INSTR_ANALYSIS_H
+#define LLVM_AVR_INSTR_ANALYSIS_H
+
+#include "llvm/MC/MCInstrAnalysis.h"
+
+namespace llvm {
+
+class AVRMCInstrAnalysis : public MCInstrAnalysis {
+public:
+  AVRMCInstrAnalysis(const MCInstrInfo *Info) : MCInstrAnalysis(Info) {}
+
+  bool evaluateBranch(const MCInst &Inst, uint64_t Addr, uint64_t Size,
+                      uint64_t &Target) const override;
+};
+
+} // end namespace llvm
+
+#endif // LLVM_AVR_INSTR_ANALYSIS_H

llvm/lib/Target/AVR/MCTargetDesc/AVRInstrAnalysis.cpp

+//===- AVRInstrAnalysis.h - Analyze AVR MCInst ------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This class analyzes AVR MCInst. In particular, it implements evaluateBranch
+// so llvm-objdump can show branch targets.
+//
+//===----------------------------------------------------------------------===//
+
+#include "AVRInstrAnalysis.h"
+
+#define GET_INSTRINFO_ENUM
+#include "AVRGenInstrInfo.inc"
+
+namespace llvm {
+
+bool AVRMCInstrAnalysis::evaluateBranch(const MCInst &Inst, uint64_t Addr,
+                                        uint64_t Size, uint64_t &Target) const {
+
+  switch (Inst.getOpcode()) {
+  case AVR::JMPk:
+  case AVR::CALLk:
+    Target = Inst.getOperand(0).getImm();
+    break;
+
+  case AVR::RJMPk:
+  case AVR::RCALLk:
+  case AVR::BRBSsk:
+  case AVR::BRBCsk:
+  case AVR::BREQk:
+  case AVR::BRNEk:
+  case AVR::BRSHk:
+  case AVR::BRLOk:
+  case AVR::BRMIk:
+  case AVR::BRPLk:
+  case AVR::BRGEk:
+  case AVR::BRLTk:
+    // Add 2 because it is relative to next executed instruction
+    Target = Addr + 2 + Inst.getOperand(0).getImm();
+    break;
+
+  default:
+    return false;
+  }
+  return true;
+}
+
+} // namespace llvm
 No newline at end of file

llvm/lib/Target/AVR/MCTargetDesc/AVRMCCodeEmitter.h

 
 /// Writes AVR machine code to a stream.
 class AVRMCCodeEmitter : public MCCodeEmitter {
 public:
   AVRMCCodeEmitter(const MCInstrInfo &MCII, MCContext &Ctx)
       : MCII(MCII), Ctx(Ctx) {}
 
 private:
-  /// Finishes up encoding an LD/ST instruction.
-  /// The purpose of this function is to set an bit in the instruction
-  /// which follows no logical pattern. See the implementation for details.
-  unsigned loadStorePostEncoder(const MCInst &MI, unsigned EncodedValue,
-                                const MCSubtargetInfo &STI) const;
-
   /// Gets the encoding for a conditional branch target.
   template <AVR::Fixups Fixup>
   unsigned encodeRelCondBrTarget(const MCInst &MI, unsigned OpNo,
                                  SmallVectorImpl<MCFixup> &Fixups,
                                  const MCSubtargetInfo &STI) const;
 
   /// Encodes the `PTRREGS` operand to a load or store instruction.
   unsigned encodeLDSTPtrReg(const MCInst &MI, unsigned OpNo,

llvm/lib/Target/AVR/MCTargetDesc/AVRMCCodeEmitter.cpp

 #define DEBUG_TYPE "mccodeemitter"
 
 #define GET_INSTRMAP_INFO
 #include "AVRGenInstrInfo.inc"
 #undef GET_INSTRMAP_INFO
 
 namespace llvm {
 
-/// Performs a post-encoding step on a `LD` or `ST` instruction.
+template <AVR::Fixups Fixup>
+unsigned
+AVRMCCodeEmitter::encodeRelCondBrTarget(const MCInst &MI, unsigned OpNo,
+                                        SmallVectorImpl<MCFixup> &Fixups,
+                                        const MCSubtargetInfo &STI) const {
+  const MCOperand &MO = MI.getOperand(OpNo);
+
+  if (MO.isExpr()) {
+    Fixups.push_back(
+        MCFixup::create(0, MO.getExpr(), MCFixupKind(Fixup), MI.getLoc()));
+    return 0;
+  }
+
+  assert(MO.isImm());
+
+  // Take the size of the current instruction away.
+  // With labels, this is implicitly done.
+  auto Target = MO.getImm();
+  AVR::fixups::adjustBranchTarget(Target);
+  return Target;
+}
+
+/// Encodes Operand for a `LD` or `ST` instruction.
 ///
 /// The encoding of the LD/ST family of instructions is inconsistent w.r.t
 /// the pointer register and the addressing mode.
 ///
 /// The permutations of the format are as followed:
 /// ld Rd, X    `1001 000d dddd 1100`
 /// ld Rd, X+   `1001 000d dddd 1101`
 /// ld Rd, -X   `1001 000d dddd 1110`

 /// Note this one inconsistent bit - it is 1 sometimes and 0 at other times.
 /// There is no logical pattern. Looking at a truth table, the following
 /// formula can be derived to fit the pattern:
 //
 /// ```
 /// inconsistent_bit = is_predec OR is_postinc OR is_reg_x
 /// ```
 //
-/// We manually set this bit in this post encoder method.
-unsigned
-AVRMCCodeEmitter::loadStorePostEncoder(const MCInst &MI, unsigned EncodedValue,
+/// We manually set this bit in this operand encoder. It is mapped
+/// to bit 2 of the operand.
+unsigned AVRMCCodeEmitter::encodeLDSTPtrReg(const MCInst &MI, unsigned OpNo,
+                                            SmallVectorImpl<MCFixup> &Fixups,
                                             const MCSubtargetInfo &STI) const {
+  auto MO = MI.getOperand(OpNo);
+
+  // The operand should be a pointer register.
+  assert(MO.isReg());
 
-  assert(MI.getOperand(0).isReg() && MI.getOperand(1).isReg() &&
-         "the load/store operands must be registers");
+  unsigned EncodedValue;
+
+  switch (MO.getReg()) {
+  case AVR::R27R26:
+    EncodedValue = 0x03; // X: 0b11
+    break;
+  case AVR::R29R28:
+    EncodedValue = 0x02; // Y: 0b10
+    break;
+  case AVR::R31R30:
+    EncodedValue = 0x00; // Z: 0b00
+    break;
+  default:
+    llvm_unreachable("invalid pointer register");
+  }
 
   unsigned Opcode = MI.getOpcode();
 
   // check whether either of the registers are the X pointer register.
   bool IsRegX = MI.getOperand(0).getReg() == AVR::R27R26 ||
                   MI.getOperand(1).getReg() == AVR::R27R26;
 
   bool IsPredec = Opcode == AVR::LDRdPtrPd || Opcode == AVR::STPtrPdRr;
   bool IsPostinc = Opcode == AVR::LDRdPtrPi || Opcode == AVR::STPtrPiRr;
 
   // Check if we need to set the inconsistent bit
   if (IsRegX || IsPredec || IsPostinc) {
-    EncodedValue |= (1 << 12);
+    EncodedValue |= (1 << 2);
   }
 
   return EncodedValue;
 }
 
-template <AVR::Fixups Fixup>
-unsigned
-AVRMCCodeEmitter::encodeRelCondBrTarget(const MCInst &MI, unsigned OpNo,
-                                        SmallVectorImpl<MCFixup> &Fixups,
-                                        const MCSubtargetInfo &STI) const {
-  const MCOperand &MO = MI.getOperand(OpNo);
-
-  if (MO.isExpr()) {
-    Fixups.push_back(MCFixup::create(0, MO.getExpr(),
-                     MCFixupKind(Fixup), MI.getLoc()));
-    return 0;
-  }
-
-  assert(MO.isImm());
-
-  // Take the size of the current instruction away.
-  // With labels, this is implicitly done.
-  auto target = MO.getImm();
-  AVR::fixups::adjustBranchTarget(target);
-  return target;
-}
-
-unsigned AVRMCCodeEmitter::encodeLDSTPtrReg(const MCInst &MI, unsigned OpNo,
-                                            SmallVectorImpl<MCFixup> &Fixups,
-                                            const MCSubtargetInfo &STI) const {
-  auto MO = MI.getOperand(OpNo);
-
-  // The operand should be a pointer register.
-  assert(MO.isReg());
-
-  switch (MO.getReg()) {
-  case AVR::R27R26: return 0x03; // X: 0b11
-  case AVR::R29R28: return 0x02; // Y: 0b10
-  case AVR::R31R30: return 0x00; // Z: 0b00
-  default:
-    llvm_unreachable("invalid pointer register");
-  }
-}
-
 /// Encodes a `memri` operand.
 /// The operand is 7-bits.
 /// * The lower 6 bits is the immediate
 /// * The upper bit is the pointer register bit (Z=0,Y=1)
 unsigned AVRMCCodeEmitter::encodeMemri(const MCInst &MI, unsigned OpNo,
                                        SmallVectorImpl<MCFixup> &Fixups,
                                        const MCSubtargetInfo &STI) const {
   auto RegOp = MI.getOperand(OpNo);

llvm/lib/Target/AVR/MCTargetDesc/AVRMCTargetDesc.cpp

 //===-- AVRMCTargetDesc.cpp - AVR Target Descriptions ---------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file provides AVR specific target descriptions.
 //
 //===----------------------------------------------------------------------===//
 
+#include "AVRMCTargetDesc.h"
 #include "AVRELFStreamer.h"
 #include "AVRInstPrinter.h"
+#include "AVRInstrAnalysis.h"
 #include "AVRMCAsmInfo.h"
 #include "AVRMCELFStreamer.h"
-#include "AVRMCTargetDesc.h"
 #include "AVRTargetStreamer.h"
 #include "TargetInfo/AVRTargetInfo.h"
 
 #include "llvm/MC/MCAsmBackend.h"
-#include "llvm/MC/MCELFStreamer.h"
 #include "llvm/MC/MCCodeEmitter.h"
+#include "llvm/MC/MCELFStreamer.h"
+#include "llvm/MC/MCInstrAnalysis.h"
 #include "llvm/MC/MCInstrInfo.h"
 #include "llvm/MC/MCRegisterInfo.h"
 #include "llvm/MC/MCSubtargetInfo.h"
 #include "llvm/Support/TargetRegistry.h"
 
 #define GET_INSTRINFO_MC_DESC
 #include "AVRGenInstrInfo.inc"
 
 #define GET_SUBTARGETINFO_MC_DESC
 #include "AVRGenSubtargetInfo.inc"
 
 #define GET_REGINFO_MC_DESC
 #include "AVRGenRegisterInfo.inc"
 
 using namespace llvm;
 
 MCInstrInfo *llvm::createAVRMCInstrInfo() {
   MCInstrInfo *X = new MCInstrInfo();
   InitAVRMCInstrInfo(X);
 
   return X;
 }
 
+static MCInstrAnalysis *createAVRMCInstrAnalysis(const MCInstrInfo *Info) {
+  return new AVRMCInstrAnalysis(Info);
+}
+
 static MCRegisterInfo *createAVRMCRegisterInfo(const Triple &TT) {
   MCRegisterInfo *X = new MCRegisterInfo();
   InitAVRMCRegisterInfo(X, 0);
 
   return X;
 }
 
 static MCSubtargetInfo *createAVRMCSubtargetInfo(const Triple &TT,

 
 extern "C" LLVM_EXTERNAL_VISIBILITY void LLVMInitializeAVRTargetMC() {
   // Register the MC asm info.
   RegisterMCAsmInfo<AVRMCAsmInfo> X(getTheAVRTarget());
 
   // Register the MC instruction info.
   TargetRegistry::RegisterMCInstrInfo(getTheAVRTarget(), createAVRMCInstrInfo);
 
+  // Register the MC instruction analysis
+  TargetRegistry::RegisterMCInstrAnalysis(getTheAVRTarget(),
+                                          createAVRMCInstrAnalysis);
+
   // Register the MC register info.
   TargetRegistry::RegisterMCRegInfo(getTheAVRTarget(), createAVRMCRegisterInfo);
 
   // Register the MC subtarget info.
   TargetRegistry::RegisterMCSubtargetInfo(getTheAVRTarget(),
                                           createAVRMCSubtargetInfo);
 
   // Register the MCInstPrinter.

llvm/lib/Target/AVR/MCTargetDesc/CMakeLists.txt

 add_llvm_component_library(LLVMAVRDesc
   AVRAsmBackend.cpp
   AVRELFObjectWriter.cpp
   AVRELFStreamer.cpp
   AVRInstPrinter.cpp
+  AVRInstrAnalysis.cpp
   AVRMCAsmInfo.cpp
   AVRMCCodeEmitter.cpp
   AVRMCELFStreamer.cpp
   AVRMCExpr.cpp
   AVRMCTargetDesc.cpp
   AVRTargetStreamer.cpp
 )
 
 add_dependencies(LLVMAVRDesc AVRCommonTableGen)