Page MenuHomePhabricator

[SampleFDO] Add symbol whitelist in the profile and use it when profile-sample-accurate is enabled
Needs ReviewPublic

Authored by wmi on Aug 16 2019, 4:56 PM.

Details

Reviewers
davidxl
mtrofin
Summary

For sampleFDO, because the optimized build uses profile generated from previous release, we cannot tell a function without profile is truely cold or just newly created. Currently we treat them conservatively and put them in .text section instead of .text.unlikely. The result is when we persuing the best performance by locking .text.hot and .text in memory, and use huge pages for them, we waste a lot of memory and huge pages for functions actually being cold.

The patch add a new section to the profile. The section contains a list of function names showing up in the binary used to generate the current profile. During profile use compilation, when profile-sample-accurate is enabled, a function without profile will be regarded as cold only when it is contained in that list.

In order to add the symbol whitelist to the profile, I extend the profile format so it is easier to add a new section or extend an existing section in the future.

Diff Detail

Repository
rL LLVM

Event Timeline

wmi created this revision.Aug 16 2019, 4:56 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 16 2019, 4:56 PM
Herald added a subscriber: mgrang. · View Herald Transcript

Can you split the format extension change into a separate patch?

wmi added a comment.Aug 19 2019, 8:44 AM

Can you split the format extension change into a separate patch?

Ok, I will do that.

mgrang added inline comments.Aug 19 2019, 2:51 PM
lib/ProfileData/SampleProf.cpp
222

Please use a range-based llvm::sort function here. See https://llvm.org/docs/CodingStandards.html#beware-of-non-deterministic-sorting-order-of-equal-elements.

llvm::sort(SortedList);
245

Please use a range-based llvm::sort function here. See https://llvm.org/docs/CodingStandards.html#beware-of-non-deterministic-sorting-order-of-equal-elements.

llvm::sort(SortedList);
wmi added a comment.Aug 20 2019, 9:09 PM

Split the patch of format extension into https://reviews.llvm.org/D66513. The symbol whitelist patch is on top of D66513 and will be sent out later.

davidxl added inline comments.Aug 22 2019, 3:29 PM
include/llvm/ProfileData/SampleProf.h
116

Name it 'SectEdgeProfile' or 'SecLBRProfile'?

Also move this after the WhiteList and reserve some enum range between WhiteList and the firstProfile

enum SecType {

SecProfileSummary = 0,
SecNameTable = 1,
SecWhiteList = 2,
SecFuncProfileFirst = 0x10,   // marker for the first type of profile.
SecLBRProfile = SecFuncProfileFirst

}

We should shoot for one profile type per section, but the hybrid mode you mentioned can also be supported if it is a size win.

wmi added a comment.Aug 26 2019, 12:30 PM
In D66374#1638720, @wmi wrote:

Split the patch of format extension into https://reviews.llvm.org/D66513. The symbol whitelist patch is on top of D66513 and will be sent out later.

The second half is here: https://reviews.llvm.org/D66766