This is an archive of the discontinued LLVM Phabricator instance.

[llvm-profgen] Trim cold function profiles for non-CS AutoFDO
ClosedPublic

Authored by wlei on Nov 12 2021, 10:33 AM.

Details

Summary

This change allows to trim the profile if it's considered to be cold for baseline AutoFDO. We reuse the cold threshold from ProfileSummaryBuilder::getColdCountThreshold(..) which can be set by percent(--profile-summary-cutoff-cold) or by value(--profile-summary-cold-count).

Diff Detail

Event Timeline

wlei created this revision.Nov 12 2021, 10:33 AM
wlei requested review of this revision.Nov 12 2021, 10:33 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 12 2021, 10:33 AM
wlei retitled this revision from [llvm-profgen] Trim cold function profiles for non-CS mode to [llvm-profgen] Trim cold function profiles for non-CS AutoFDO.Nov 12 2021, 10:38 AM
wlei edited the summary of this revision. (Show Details)
wlei added reviewers: hoy, wenlei.
hoy added inline comments.Nov 12 2021, 3:29 PM
llvm/tools/llvm-profgen/ProfileGenerator.cpp
63–64

How about merge the new switch with this one?

wenlei added inline comments.Nov 12 2021, 4:28 PM
llvm/tools/llvm-profgen/ProfileGenerator.cpp
63–64

Yeah, let's merge them. And this may also mean we need to put it on afdo-specific path, instead having it in ProfileGenerator::postProcessProfiles.

wlei updated this revision to Diff 387374.Nov 15 2021, 1:07 PM

merge TrimColdProfile and CSProfTrimColdContext

hoy accepted this revision.Nov 15 2021, 1:11 PM

lgtm, thanks.

llvm/test/tools/llvm-profgen/merge-cold-profile.test
9

nit: this should not change?

This revision is now accepted and ready to land.Nov 15 2021, 1:11 PM
wlei updated this revision to Diff 387379.Nov 15 2021, 1:16 PM
Updating D113785: [llvm-profgen] Trim cold function profiles for non-CS AutoFDO

None of the tests cover trim vs no-trim difference, it'd be good to have one, so we check trimming is actually making a difference.

wenlei accepted this revision.Nov 29 2021, 6:02 PM

lgtm with a comment on test.

wlei added inline comments.Nov 30 2021, 12:14 PM
llvm/test/tools/llvm-profgen/inline-noprobe2.test
111 ↗(On Diff #387379)

None of the tests cover trim vs no-trim difference, it'd be good to have one, so we check trimming is actually making a difference.

@wenlei We do have a test case for the trimming here which checked the function won't appear if its total sample is smaller than --profile-summary-cold-count=100

wenlei added inline comments.Nov 30 2021, 12:17 PM
llvm/test/tools/llvm-profgen/inline-noprobe2.test
111 ↗(On Diff #387379)

But we don't have a check to make sure main/quick_sort actually appears without trimming, right? If for whatever reason these two never appears even without trimming (--trim-cold-profile=0), this test could be doing nothing.

wlei added inline comments.Nov 30 2021, 12:32 PM
llvm/test/tools/llvm-profgen/inline-noprobe2.test
89 ↗(On Diff #387379)

Here

111 ↗(On Diff #387379)

Oh, I got your meaning. In the test above, there is a CHECK for main/quick_sort, but I realize it's mixed with other test and not visible. I will add a separate test case for cold trimming.Thanks for the clarification.

wlei updated this revision to Diff 390797.Nov 30 2021, 12:56 PM

add a separated test case for trimming, added no trim case

wenlei added inline comments.Nov 30 2021, 12:57 PM
llvm/test/tools/llvm-profgen/inline-noprobe2.test
111 ↗(On Diff #387379)

Thanks, looks good now.