Page MenuHomePhabricator

Add method to MCInstPrinter for printing with disassembly context
ClosedPublic

Authored by colinl on Mar 18 2015, 4:16 PM.

Details

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.

Diff Detail

Repository
rL LLVM

Event Timeline

colinl updated this revision to Diff 22224.Mar 18 2015, 4:16 PM
colinl retitled this revision from to Add method to MCInstPrinter for printing with disassembly context.
colinl updated this object.
colinl edited the test plan for this revision. (Show Details)
colinl added reviewers: Bigcheese, rafael.
colinl set the repository for this revision to rL LLVM.
colinl added subscribers: sidneym, mlambert, Unknown Object (MLST).
rafael edited edge metadata.Mar 19 2015, 1:25 PM

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 ↗(On Diff #22224)

dumpBytes

lib/MC/MCInstPrinter.cpp
20 ↗(On Diff #22224)

This can now be static, no?

26 ↗(On Diff #22224)

Why not send the chars to OS directly?

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 ↗(On Diff #22224)

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

26 ↗(On Diff #22224)

Good point, I'll make that change.

rafael accepted this revision.Mar 19 2015, 2:03 PM
rafael edited edge metadata.

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

This revision is now accepted and ready to land.Mar 19 2015, 2:03 PM
colinl updated this revision to Diff 26609.May 27 2015, 9:49 AM
colinl edited edge metadata.

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.

This revision was automatically updated to reflect the committed changes.