Add method to MCInstPrinter for printing with disassembly context

[colinl]

Summary

Add a virtual method to MCInstPrinter which allows backends to specialize how the instruction is printed.

Generally this allows backends greater ability to format their instructions when being printed with a disassembly context like with objdump.

Specifically on Hexagon this allows us to print VLIW instructions in an 80-column readable way by placing one instruction per line and putting the disassembled bytes next to the associated instruction.

[• rafael]

I think this is fine with a few changes.

Do you have the patch actually using this ready somewhere?

include/llvm/MC/MCInstPrinter.h

26

dumpBytes

lib/MC/MCInstPrinter.cpp

20

This can now be static, no?

26

Why not send the chars to OS directly?

[colinl]

This is a sketch of looping we use to generate bundles:

for(auto i(MI.begin()), j (MI.end()); i != j; ++i) {
  OS << format("%8" PRIx64 ":", Address);
  Address += HEXAGON_INSTR_SIZE;
  if (ShowRawInsn) {
    OS << "\t";
    DumpBytes(StringRef(
      reinterpret_cast<const char *>(Bytes.data()), HEXAGON_INSTR_SIZE),
              HEXAGON_INSTR_SIZE, true, true);
    Bytes = Bytes.slice(HEXAGON_INSTR_SIZE);
  }
  if(i == MI.begin())
    OS << " { ";
  else
    OS << "   ";
  MCInst const * Inst = i->getInst();
  PrintOneInst(*Inst, OS);
  if(i + 1 == MI.end())
    OS << " }";
  else
    OS << '\n';
}
if(HexagonMCInstrInfo::IsEnd0(MI))
  OS << ":endloop0";
if(HexagonMCInstrInfo::IsEnd1(MI))
  OS << ":endloop1";

And it produces output like this:

0:       a3 43 02 8e 8e0243a3 { r3 += lsr(r2, #3)
4:       00 40 20 5c 5c204000   if (!p0) jump 0x0
8:       24 40 c1 41 41c14024   if (p0) r5:4 = memd(r1 + #8)
c:       08 e4 c0 ab abc0e408   if (p0) memd(r0++#8) = r5:4 }

lib/MC/MCInstPrinter.cpp

20

It's still used inside MachODump in a couple places.

26

Good point, I'll make that change.

[• rafael]

LGTM with the DumpBytes change and the condition this will be used soon :-)

[colinl]

I've been working more on this and have reconsidered the design. The issue I ran in to is there are other users of InstPrinter that need to render the instruction in different ways. The other major example besides objdump is lldb which prints out additional information like the function name and offset.

If things were done as in the last patch, each user of InstPrinter could require a new virtual method containing the information it wants to display, effectively InstPrinter would need to know about all its users.

This patch moves the rendering to the user which leaves InstPrinter unchanged. LLDB has existing infrastructure to do similar target-specific rendering.

include/llvm/MC/MCInstPrinter.h

 #ifndef LLVM_MC_MCINSTPRINTER_H
 #define LLVM_MC_MCINSTPRINTER_H
 
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/Support/DataTypes.h"
 #include "llvm/Support/Format.h"
 

 class MCRegisterInfo;
 class StringRef;
 
+/// Convert `Bytes' to a hex string and output to `OS'
+void DumpBytes(ArrayRef<uint8_t> Bytes, raw_ostream &OS);
+
 namespace HexStyle {
     enum Style {
         C,          ///< 0xff

   virtual void printInst(const MCInst *MI, raw_ostream &OS,
                          StringRef Annot) = 0;
 
+  /// Print the MCInst along with disassembly context
+  virtual void printInst(const MCInst *MI, bool ShowRawInsn,
+                         ArrayRef<uint8_t> Bytes, uint64_t Address,
+                         raw_ostream &OS);
+
   /// getOpcodeName - Return the name of the specified opcode enum (e.g.
   /// "MOV32ri") or empty if we can't resolve it.
   StringRef getOpcodeName(unsigned Opcode) const;

lib/MC/MCInstPrinter.cpp

 //
 //===----------------------------------------------------------------------===//
 
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/MC/MCInstPrinter.h"
-#include "llvm/ADT/StringRef.h"
 #include "llvm/MC/MCAsmInfo.h"
 #include "llvm/MC/MCInstrInfo.h"
 #include "llvm/Support/ErrorHandling.h"

 #include "llvm/Support/raw_ostream.h"
 using namespace llvm;
 
+void llvm::DumpBytes(ArrayRef<uint8_t> bytes, raw_ostream &OS) {
+  static const char hex_rep[] = "0123456789abcdef";
+  SmallString<64> output;
+
+  for (char i: bytes) {
+    output.push_back(hex_rep[(i & 0xF0) >> 4]);
+    output.push_back(hex_rep[i & 0xF]);
+    output.push_back(' ');
+  }
+
+  OS << output.c_str();
+}
+
 MCInstPrinter::~MCInstPrinter() {
 }
 
+void MCInstPrinter::printInst(const MCInst *MI, bool ShowRawInsn,
+                              ArrayRef<uint8_t> Bytes, uint64_t Address,
+                              raw_ostream &OS) {
+  outs() << format("%8" PRIx64 ":", Address);
+  if (ShowRawInsn) {
+    outs() << "\t";
+    DumpBytes(Bytes, OS);
+  }
+  printInst(MI, outs(), "");
+}
+
 /// getOpcodeName - Return the name of the specified opcode enum (e.g.
 /// "MOV32ri") or empty if we can't resolve it.
 StringRef MCInstPrinter::getOpcodeName(unsigned Opcode) const {

tools/llvm-objdump/llvm-objdump.h

 // Various helper functions.
 bool error(std::error_code ec);
 bool RelocAddressLess(object::RelocationRef a, object::RelocationRef b);
-void DumpBytes(ArrayRef<uint8_t> bytes);
 void ParseInputMachO(StringRef Filename);
 void printCOFFUnwindInfo(const object::COFFObjectFile* o);
 void printMachOUnwindInfo(const object::MachOObjectFile* o);

tools/llvm-objdump/llvm-objdump.cpp

   return TheTarget;
 }
 
-void llvm::DumpBytes(ArrayRef<uint8_t> bytes) {
-  static const char hex_rep[] = "0123456789abcdef";
-  SmallString<64> output;
-
-  for (char i: bytes) {
-    output.push_back(hex_rep[(i & 0xF0) >> 4]);
-    output.push_back(hex_rep[i & 0xF]);
-    output.push_back(' ');
-  }
-
-  outs() << output.c_str();
-}
-
 bool llvm::RelocAddressLess(RelocationRef a, RelocationRef b) {
   uint64_t a_addr, b_addr;
   if (error(a.getOffset(a_addr))) return false;

         if (DisAsm->getInstruction(Inst, Size, Bytes.slice(Index),
                                    SectionAddr + Index, DebugOut,
                                    CommentStream)) {
-          outs() << format("%8" PRIx64 ":", SectionAddr + Index);
-          if (!NoShowRawInsn) {
-            outs() << "\t";
-            DumpBytes(ArrayRef<uint8_t>(Bytes.data() + Index, Size));
-          }
-          IP->printInst(&Inst, outs(), "");
+          IP->printInst(&Inst, !NoShowRawInsn,
+                        ArrayRef<uint8_t>(Bytes.data (), Size),
+                        SectionAddr + Index, outs());
           outs() << CommentStream.str();
           Comments.clear();
           outs() << "\n";