This is an archive of the discontinued LLVM Phabricator instance.

[SampleFDO] Add flag for non-dedicated profile.
ClosedPublic

Authored by wmi on Apr 3 2020, 12:24 PM.

Details

Summary

The common profile usage is to collect profile from a target and then use the profile to guide the optimized build for the same target dedicatedly. There are some cases that no profile can be collected for a target. In those cases, although no dedicated profile is available, it is possible to have some generic profile to optimize common libraries and utilities. A flag is needed to tell the non-dedicated profile from the dedicated profile apart, so compiler can use different strategy for them.

Diff Detail

Event Timeline

wmi created this revision.Apr 3 2020, 12:24 PM
davidxl added inline comments.Apr 3 2020, 3:44 PM
llvm/include/llvm/IR/ProfileSummary.h
54

--> If 'Dedicated' is false, it means the profile is for common/shared code. The common profile is merged from profiles collected from running other targets.

llvm/include/llvm/ProfileData/SampleProf.h
174

SecFlagPartial or SecFlagShared?

llvm/include/llvm/ProfileData/SampleProfWriter.h
223

setPartialProfile or setSharedProfile?

llvm/lib/ProfileData/SampleProfReader.cpp
850

partial or shared?

wmi marked 4 inline comments as done.Apr 3 2020, 5:01 PM
wmi added inline comments.
llvm/include/llvm/ProfileData/SampleProf.h
174

Change to SecFlagPartial here.

wmi updated this revision to Diff 254954.Apr 3 2020, 5:02 PM

Address David's comment.

wenlei added a comment.Apr 3 2020, 9:52 PM

Thanks for the patch! We're also thinking about differentiating default profile (from other targets) vs custom profile (from same target) for optimizations, so very curious about your compiler heuristic changes after this. :) I am guessing ProfileSampleAccurate and ProfileAccurateForSymsInList will one of those changes?

wmi added a comment.Apr 6 2020, 9:39 AM

Thanks for the patch! We're also thinking about differentiating default profile (from other targets) vs custom profile (from same target) for optimizations, so very curious about your compiler heuristic changes after this. :) I am guessing ProfileSampleAccurate and ProfileAccurateForSymsInList will one of those changes?

ProfileSampleAccurate and ProfileAccurateForSymsInList are already off by default so it is not urgent to guard the logic with the partial profile flag unless ProfileSampleAccurate and ProfileAccurateForSymsInList will be enabled by default.

An example for compiler heuristic to change is ProfileSummaryInfo::hasLargeWorkingSetSize/hasHugeWorkingSetSize. If the profile is a partial profile, those functions should return false in any case because there is no way to know the actual working set size from a partial profile.

davidxl accepted this revision.Apr 6 2020, 1:20 PM

lgtm

This revision is now accepted and ready to land.Apr 6 2020, 1:20 PM
wenlei accepted this revision.Apr 6 2020, 4:14 PM
In D77426#1964396, @wmi wrote:

Thanks for the patch! We're also thinking about differentiating default profile (from other targets) vs custom profile (from same target) for optimizations, so very curious about your compiler heuristic changes after this. :) I am guessing ProfileSampleAccurate and ProfileAccurateForSymsInList will one of those changes?

ProfileSampleAccurate and ProfileAccurateForSymsInList are already off by default so it is not urgent to guard the logic with the partial profile flag unless ProfileSampleAccurate and ProfileAccurateForSymsInList will be enabled by default.

An example for compiler heuristic to change is ProfileSummaryInfo::hasLargeWorkingSetSize/hasHugeWorkingSetSize. If the profile is a partial profile, those functions should return false in any case because there is no way to know the actual working set size from a partial profile.

Thanks for clarification, looking forward to the coming heuristic improvements.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptApr 7 2020, 12:33 PM
llvm/lib/ProfileData/SampleProfReader.cpp