Page MenuHomePhabricator

[CSSPGO] Aggregation by the last K context frames for cold profiles
ClosedPublic

Authored by wlei on Jun 11 2021, 10:13 AM.

Details

Summary

This change provides the option to merge and aggregate cold context by the last k frames instead of context-less name. By default K = 1 means the context-less one.

This is for better perf tuning. The more selective merging and trimming will rely on llvm-profgen's preinliner.

Diff Detail

Event Timeline

wlei created this revision.Jun 11 2021, 10:13 AM
wlei requested review of this revision.Jun 11 2021, 10:13 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 11 2021, 10:13 AM
wlei edited the summary of this revision. (Show Details)Jun 11 2021, 10:31 AM
wlei added reviewers: wenlei, hoy, wmi.

Thanks for the patch. I think this can be useful for tuning. However as we discussed, it'd be nice if the level of merging (number of frames to keep after merging) can be adaptive based on hotness. But to do that, the preinliner in llvm-profgen is the way to go.

llvm/include/llvm/ProfileData/SampleProf.h
441

We need to find @ instead of @ as mangle name can contain @. And how about using StringRef::rfind ?

llvm/lib/ProfileData/SampleProf.cpp
346–348

Now rename this to MergedProfileMap?

wlei updated this revision to Diff 351529.Jun 11 2021, 12:22 PM

Addressing Wenlei's comments

wlei updated this revision to Diff 351531.Jun 11 2021, 12:28 PM

change to use uint32_t

wenlei added inline comments.Jun 11 2021, 8:53 PM
llvm/include/llvm/ProfileData/SampleProf.h
1014

Looks like all call site have explicit argument, do we still need a default one?

llvm/tools/llvm-profgen/ProfileGenerator.cpp
47

nit: how about name it frame-depth-for-cold-context? similar name for llvm-profdata flag and all variables.

wlei updated this revision to Diff 351629.Jun 11 2021, 9:30 PM

Addressing Wenlei's comments

wenlei accepted this revision.Jun 11 2021, 9:31 PM

lgtm, thanks!

This revision is now accepted and ready to land.Jun 11 2021, 9:31 PM
hoy accepted this revision.Jun 12 2021, 11:04 AM

LGTM.

This revision was automatically updated to reflect the committed changes.
wlei marked 2 inline comments as done.