This is an archive of the discontinued LLVM Phabricator instance.

[InstrProf][Correlate] Verify debug info with llvm-profdata show
ClosedPublic

Authored by ellis on Jan 25 2022, 12:47 PM.

Details

Summary

Use the llvm-profdata show command to verify debug info for profile correlation using the --debug-info option.

Diff Detail

Event Timeline

ellis created this revision.Jan 25 2022, 12:47 PM
ellis updated this revision to Diff 403056.Jan 25 2022, 3:42 PM

Working

ellis retitled this revision from Add show verify to [InstrProf][Correlate] Verify debug info with llvm-profdata show.Jan 25 2022, 3:44 PM
ellis edited the summary of this revision. (Show Details)
ellis added reviewers: kyulee, David.Xu, phosek.
ellis updated this revision to Diff 403072.Jan 25 2022, 4:18 PM

Display counters section size

ellis updated this revision to Diff 403073.Jan 25 2022, 4:21 PM

Add comment

ellis published this revision for review.Jan 25 2022, 4:21 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJan 25 2022, 4:21 PM
Herald added subscribers: llvm-commits, Restricted Project. · View Herald Transcript
kyulee added inline comments.Jan 26 2022, 12:08 AM
compiler-rt/test/profile/Linux/instrprof-show-debug-info-correlation.c
2

It appears there are many similar names, which is confusing to remember.
The compilation uses -debug-info-correlate.
For llvm-profdata merge, use -debug-info=<debug file>.
Now, llvm-profdata show use -debug-info-correlation. This usage actually specifies <debug file> in the place of <profile file> which is a required argument to llvm-profdata show.
One thing I can think about is to use llvm-profdata show -debug-info=<debug file> while allowing to omit the required <profile file>, which becomes optional. But this doesn't look ideal, either.
I don't have a strong suggestion, but I wish some more consistent naming.

phosek added inline comments.Jan 26 2022, 12:16 AM
compiler-rt/test/profile/Linux/instrprof-show-debug-info-correlation.c
2

I agree and in particular I'd prefer the last suggestion, that is making <profile file> optional when -debug-info=<debug file> is specified. I think that changing the meaning of the positional argument (that is either a profile or a binary) depending on other arguments is more confusing.

llvm/tools/llvm-profdata/llvm-profdata.cpp
2581

Nit: spelling.

ellis added inline comments.Jan 26 2022, 10:47 AM
compiler-rt/test/profile/Linux/instrprof-show-debug-info-correlation.c
2

I'm happy with using -debug-info=<debug file>. Originally I wanted to avoid removing the cl::Required in the Filename option, but this really isn't a big deal.

ellis updated this revision to Diff 403351.Jan 26 2022, 11:33 AM

Change option name and add another test file.

Thanks for making the flag more consistent! Could you quickly update the summary in this patch to reflect the current code?
LGTM

kyulee accepted this revision.Jan 27 2022, 10:02 AM
This revision is now accepted and ready to land.Jan 27 2022, 10:02 AM
ellis edited the summary of this revision. (Show Details)Jan 27 2022, 10:09 AM
This revision was landed with ongoing or failed builds.Jan 27 2022, 10:11 AM
This revision was automatically updated to reflect the committed changes.