Page MenuHomePhabricator

Implement llvm-objdump -S and -l
ClosedPublic

Authored by khemant on Jul 28 2016, 1:15 PM.

Details

Summary

Implement the source and line number interleaving with the disassembly.

Diff Detail

Repository
rL LLVM

Event Timeline

khemant updated this revision to Diff 65975.Jul 28 2016, 1:15 PM
khemant retitled this revision from to Implement llvm-objdump -S and -l.
khemant updated this object.
khemant added reviewers: echristo, rafael.
khemant set the repository for this revision to rL LLVM.
khemant added subscribers: echristo, rafael, llvm-commits.
echristo edited edge metadata.Jul 28 2016, 2:24 PM

I'd suggest compiling down a source file to .ll, but keeping it in an Inputs directory for the tool to use in order to match source to object. You may have to modify the debug info in the IR slightly, but maybe not.

-eric

Thanks. Will try that an send for review.

khemant updated this revision to Diff 67011.Aug 5 2016, 1:46 PM
khemant updated this object.
khemant edited edge metadata.

Made changes so all pretty printers can interleave source with disassembly. Added tests that test X86 and Hexagon printers.

I think it would be better to split the hexagon support into a separate (trivial) patch. That would make this change only add the source and line number printing and tests for x86. The hexagon change would then add that target as well.

compnerd added inline comments.Aug 9 2016, 5:33 PM
tools/llvm-objdump/llvm-objdump.cpp
383

What exactly do you mean by "storage for the source" here?

400

Why triple semicolon? IIRC, the binutils interleaving uses '; '.

1308

Weird, why did this get reflowed?

I think it would be better to split the hexagon support into a separate (trivial) patch. That would make this change only add the source and line number printing and tests for x86. The hexagon change would then add that target as well.

Got it. I will split this into two patches.

tools/llvm-objdump/llvm-objdump.cpp
383

The comment should have been for line 385. The source files need to be opened and displayed when a line info is valid. The files may have to live till end of disassembly since using linker scripts the sections may have been laid out such that source from one file may need to be displayed at any point during disassembly.

400

Sure a single "; " can be added.

1308

Somehow clang format in one of the intermediate commits I squashed for this change may have triggered this.

khemant updated this revision to Diff 67755.Aug 11 2016, 3:20 PM

Removed hexagon changes to leave only generic pretty printer changes. Hexagon will be added as a separate patch.

khemant marked 2 inline comments as done.Aug 11 2016, 3:22 PM

@compnerd: Please review the changes. Took your suggestions into account.

compnerd added inline comments.Aug 11 2016, 4:36 PM
test/tools/llvm-objdump/X86/Inputs/source-interleave-x86_64.c
12

Might be nice to clang-format this file before checkin. It hurts my sense of aesthetics :-p.

test/tools/llvm-objdump/X86/source-interleave.s
5 ↗(On Diff #67755)

Is there a reason to prefer ASM over LLVM IR here? If we use LLVM IR, it might be much shorter. Since the DI should reference the LineNumber, I think that the test output should be consistent.

emaste added a subscriber: emaste.Aug 11 2016, 6:46 PM
khemant updated this revision to Diff 67879.Aug 12 2016, 11:58 AM

The test is now IR instead of asm. clang formatted the test.

khemant marked 2 inline comments as done.Aug 12 2016, 11:59 AM

@compnerd. You are right, IR is better choice for test. Formatted the test too. Please +1 the change.

compnerd accepted this revision.Aug 12 2016, 12:53 PM
compnerd edited edge metadata.

Thanks for the work on this! (BTW, feel free to add me to the other patch to add the hexagon stuff since I asked you to split that up, Im happy to do the extra work to review it).

This revision is now accepted and ready to land.Aug 12 2016, 12:53 PM
khemant closed this revision.Aug 18 2016, 12:27 PM

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@278725 91177308-0d34-0410-b5e6-96231b3b80d8