[AVR] Fix incorrect decoding of conditional branch instructions

[benshi001]

Summary

This patch fixes the inaccurate decoding of the offset operand of
the conditional branch instructions.

[benshi001]

This patch fixes https://github.com/llvm/llvm-project/issues/47093 .

[benshi001]

We can not test this in llvm/test/MC/AVR/, since the llvm-objdump decodes all addresses/offsets (before linking) to zeros. We have to test them in lld/test/ELF/.

[benshi001]

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

19

The llvm-objdump's option --no-aliases has not been implemented on AVR. So we can not decoded to their original forms.

[benshi001]

The conditional branch instructions are defined in two ways:

1. Defined as stand alone instructions: such as breq, brne, brlo, ...
2. Defined as aliases of brbc/brbs, such brbs, brbc, brid, brie, ...

The above two situations are both tested in current patch.

[benshi001]

ping ...

This patch block another patch (https://reviews.llvm.org/D141197) which has already been approved. If this one can be accepted, all known dissassembler bugs should be fixed.

[MaskRay]

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

300

A pair array will be more efficient with so few values. You can use llvm::partition_point to do bisection.

[benshi001]

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

300

Thanks. I will change to llvm::partition_point when commiting.

llvm/lib/Target/AVR/AVRInstrFormats.td

 class FBRsk<bit f, bits<3> s, dag outs, dag ins, string asmstr,
             list<dag> pattern> : AVRInst16<outs, ins, asmstr, pattern> {
   bits<7> k;
 
   let Inst{15 - 11} = 0b11110;
   let Inst{10} = f;
   let Inst{9 - 3} = k;
   let Inst{2 - 0} = s;
+
+  let DecoderMethod = "decodeCondBranch";
 }
 
 //===----------------------------------------------------------------------===//
 // Special, opcode only instructions: <|opcode|>
 //===----------------------------------------------------------------------===//
 
 class F16<bits<16> opcode, dag outs, dag ins, string asmstr, list<dag> pattern>
     : AVRInst16<outs, ins, asmstr, pattern> {
class FSK<bit f, dag outs, dag ins, string asmstr, list<dag> pattern>
   let Inst{11} = 0;
   let Inst{10} = f;
   let Inst{9 - 8} = k{6 - 5};
 
   let Inst{7 - 4} = k{4 - 1};
 
   let Inst{3} = k{0};
   let Inst{2 - 0} = s;
+
+  let DecoderMethod = "decodeCondBranch";
 }
 
 class ExtensionPseudo<dag outs, dag ins, string asmstr, list<dag> pattern>
     : Pseudo<outs, ins, asmstr, pattern> {
   let Defs = [SREG];
 }
 
 class StorePseudo<dag outs, dag ins, string asmstr, list<dag> pattern>

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

 //===----------------------------------------------------------------------===//
 
 #include "AVR.h"
 #include "AVRRegisterInfo.h"
 #include "AVRSubtarget.h"
 #include "MCTargetDesc/AVRMCTargetDesc.h"
 #include "TargetInfo/AVRTargetInfo.h"
 
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/STLExtras.h"
+
 #include "llvm/MC/MCAsmInfo.h"
 #include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCDecoderOps.h"
 #include "llvm/MC/MCDisassembler/MCDisassembler.h"
 #include "llvm/MC/MCInst.h"
 #include "llvm/MC/TargetRegistry.h"
 
 using namespace llvm;
static DecodeStatus decodeFMUL2RdRr(MCInst &Inst, unsigned Insn,
                                     const MCDisassembler *Decoder);
 
 static DecodeStatus decodeMemri(MCInst &Inst, unsigned Insn, uint64_t Address,
                                 const MCDisassembler *Decoder);
 
 static DecodeStatus decodeFBRk(MCInst &Inst, unsigned Insn, uint64_t Address,
                                const MCDisassembler *Decoder);
 
+static DecodeStatus decodeCondBranch(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) {
default: // Unknown relative branch instruction.
     return MCDisassembler::Fail;
   }
   // Decode the relative offset.
   int16_t Offset = ((int16_t)((Insn & 0xfff) << 4)) >> 3;
   Inst.addOperand(MCOperand::createImm(Offset));
   return MCDisassembler::Success;
 }
 
+static DecodeStatus decodeCondBranch(MCInst &Inst, unsigned Insn,
+                                     uint64_t Address,
+                                     const MCDisassembler *Decoder) {
+  // These 8 instructions are not defined as aliases of BRBS/BRBC.
+  DenseMap<unsigned, unsigned> brInsts = {
+      {0x000, AVR::BRLOk}, {0x400, AVR::BRSHk}, {0x001, AVR::BREQk},
+      {0x401, AVR::BRNEk}, {0x002, AVR::BRMIk}, {0x402, AVR::BRPLk},
+      {0x004, AVR::BRLTk}, {0x404, AVR::BRGEk}};
+
+  // Get the relative offset.
+  int16_t Offset = ((int16_t)((Insn & 0x3f8) << 6)) >> 8;
+
+  // Search the instruction pattern.
+  auto NotAlias = [&Insn](const std::pair<unsigned, unsigned> &I) {
+    return (Insn & 0x407) != I.first;
+  };
+  llvm::partition(brInsts, NotAlias);
+  auto It = llvm::partition_point(brInsts, NotAlias);
+
+  // Decode the instruction.
+  if (It != brInsts.end()) {
+    // This instruction is not an alias of BRBC/BRBS.
+    Inst.setOpcode(It->second);
+    Inst.addOperand(MCOperand::createImm(Offset));
+  } else {
+    // Fall back to an ordinary BRBS/BRBC.
+    Inst.setOpcode(Insn & 0x400 ? AVR::BRBCsk : AVR::BRBSsk);
+    Inst.addOperand(MCOperand::createImm(Insn & 7));
+    Inst.addOperand(MCOperand::createImm(Offset));
+  }
+
+  return MCDisassembler::Success;
+}
+
 static DecodeStatus decodeLoadStore(MCInst &Inst, unsigned Insn,
                                     uint64_t Address,
                                     const MCDisassembler *Decoder) {
   // Get the register will be loaded or stored.
   unsigned RegVal = GPRDecoderTable[(Insn >> 4) & 0x1f];
 
   // Decode LDD/STD with offset less than 8.
   if ((Insn & 0xf000) == 0x8000) {

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

 ; RUN: llvm-mc -triple avr -show-encoding < %s | FileCheck %s
-
+; RUN: llvm-mc -filetype=obj -triple avr < %s \
+; RUN:     | llvm-objdump -d - | FileCheck --check-prefix=INST %s
 
 foo:
 
   brbc 3, .+8
   brbc 0, .-16
+  .short 0xf759
+  .short 0xf752
+  .short 0xf74c
+  .short 0xf4c7
 
 ; CHECK: brvc .Ltmp0+8              ; encoding: [0bAAAAA011,0b111101AA]
 ; CHECK:                            ; fixup A - offset: 0, value: .Ltmp0+8, kind: fixup_7_pcrel
 ; CHECK: brcc .Ltmp1-16             ; encoding: [0bAAAAA000,0b111101AA]
 ; CHECK:                            ; fixup A - offset: 0, value: .Ltmp1-16, kind: fixup_7_pcrel
+
+; INST: brvc .+0
+; INST: brsh .+0
+; INST: brne .-42
+; INST: brpl .-44
+; INST: brge .-46
+; INST: brid .+48

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

 ; RUN: llvm-mc -triple avr -show-encoding < %s | FileCheck %s
-
+; RUN: llvm-mc -filetype=obj -triple avr < %s \
+; RUN:     | llvm-objdump -d - | FileCheck --check-prefix=INST %s
 
 foo:
 
   brbs 3, .+8
   brbs 0, .-12
+  .short 0xf359
+  .short 0xf352
+  .short 0xf34c
+  .short 0xf077
 
 ; CHECK: brvs .Ltmp0+8              ; encoding: [0bAAAAA011,0b111100AA]
 ; CHECK:                            ; fixup A - offset: 0, value: .Ltmp0+8, kind: fixup_7_pcrel
 ; CHECK: brcs .Ltmp1-12             ; encoding: [0bAAAAA000,0b111100AA]
 ; CHECK:                            ; fixup A - offset: 0, value: .Ltmp1-12, kind: fixup_7_pcrel
+
+; INST: brvs .+0
+; INST: brlo .+0
+; INST: breq .-42
+; INST  brmi .-44
+; INST  brlt .-46
+; InST: brie .+28