This is an archive of the discontinued LLVM Phabricator instance.

[ThinLTO] Add dump-summary command to llvm-lto2 tool
Needs RevisionPublic

Authored by ncharlie on Jun 10 2017, 7:47 AM.

Details

Summary

[ThinLTO] Add dump-summary command to llvm-lto2 tool

Adds command to dump ThinLTO module summaries in YAML to the llvm-lto2 tool.

Diff Detail

Event Timeline

ncharlie created this revision.Jun 10 2017, 7:47 AM
tejohnson added inline comments.Jun 10 2017, 7:54 AM
test/tools/llvm-lto2/X86/dump-summary.ll
2

Rather than repeating this command and piping through egrep, you should run once and pipe through FileCheck (see the other tests in this directory for examples). I.e. pipe through "FileCheck %s", then have several lines prefixed by "CHECK: " to look for the expected output. E.g.:

CHECK: NotEligibleToImport: false

ncharlie updated this revision to Diff 102226.Jun 12 2017, 12:20 PM

Use FileCheck rather than egrep

ncharlie marked an inline comment as done.Jun 12 2017, 12:22 PM
ncharlie added inline comments.
test/tools/llvm-lto2/X86/dump-summary.ll
2

Oh, cool - I saw FileCheck before, but assumed that the 'CHECK' lines had to be outputted throughout the pipeline and couldn't be fed into FileCheck like that. I fixed it.

davide edited edge metadata.Jun 18 2017, 9:43 PM

Minor comment, LGTM with that (once your other patch goes in).

tools/llvm-lto2/llvm-lto2.cpp
356–367

Add a comment to this function.

davide accepted this revision.Jun 18 2017, 9:43 PM
This revision is now accepted and ready to land.Jun 18 2017, 9:43 PM
ncharlie updated this revision to Diff 103023.Jun 19 2017, 5:31 AM
ncharlie marked an inline comment as done.

Added comment.

ncharlie marked an inline comment as done.Jun 19 2017, 5:31 AM
Prazek removed a subscriber: Prazek.Jun 19 2017, 1:58 PM
davide added a comment.Jul 7 2017, 9:24 AM

Teresa, any further comments or we can go ahead and commit Charles' patch?

davide requested changes to this revision.Jul 7 2017, 10:05 AM
davide added subscribers: dblaikie, chandlerc.

Please hold the press on this one for a bit.
I didn't see @chandlerc / @dblaikie comments on the RFC and they seem to not have been fully addressed.
I guess you may want to continue the discussion there before coming back to this.

This revision now requires changes to proceed.Jul 7 2017, 10:05 AM