This is an archive of the discontinued LLVM Phabricator instance.

[llvm-profgen] Clean up code dealing with multiple binaries
ClosedPublic

Authored by wlei on Aug 12 2021, 2:58 PM.

Details

Summary

As we decided to support only one binary each time, this patch cleans up the related code dealing with multiple binaries. We can use llvm-profdata to merge profile from multiple binaries.

Diff Detail

Event Timeline

wlei created this revision.Aug 12 2021, 2:58 PM
wlei requested review of this revision.Aug 12 2021, 2:58 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 12 2021, 2:58 PM
wlei updated this revision to Diff 366122.Aug 12 2021, 3:07 PM

fix comments

wlei edited the summary of this revision. (Show Details)Aug 12 2021, 3:12 PM
wlei added reviewers: hoy, wenlei, wmi.
hoy added a comment.Aug 13 2021, 10:41 AM

Thanks for the cleanup. The new code looks cleaner and I believe it'll also speed up the tool.

llvm/tools/llvm-profgen/PerfReader.h
535

Do we still need this? Looks like this can be just a single field of PerfReaderBase.

wlei updated this revision to Diff 366330.Aug 13 2021, 11:55 AM

remove unneeded code

llvm/tools/llvm-profgen/PerfReader.h
535

Good catch! We shouldn't need this, removed.

wlei updated this revision to Diff 366334.Aug 13 2021, 12:08 PM

fixed typos

hoy accepted this revision.Aug 13 2021, 12:56 PM

lgtm except for a minor issue.

llvm/tools/llvm-profgen/PerfReader.cpp
350

The comment could still be helpful?

This revision is now accepted and ready to land.Aug 13 2021, 12:56 PM
wlei updated this revision to Diff 366340.Aug 13 2021, 1:10 PM

Tweak some comments

wenlei accepted this revision.Aug 13 2021, 2:38 PM
wenlei added inline comments.
llvm/tools/llvm-profgen/PerfReader.cpp
296–297

is the removal of const intentional?

wlei updated this revision to Diff 366374.Aug 13 2021, 4:40 PM

add "const" qualifier

llvm/tools/llvm-profgen/PerfReader.cpp
296–297

Good catch!

This revision was automatically updated to reflect the committed changes.