<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
</head>
<body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="">
Thanks!
<div class=""><br class="">
</div>
<div class="">Artur<br class="">
<div><br class="">
<blockquote type="cite" class="">
<div class="">On 14 May 2020, at 14:12, Wei Mi <<a href="mailto:wmi@google.com" class="">wmi@google.com</a>> wrote:</div>
<br class="Apple-interchange-newline">
<div class="">
<div dir="ltr" class="">
<div dir="ltr" class="">
<div class="gmail_default" style="font-family:times new roman,serif">Artur, I have renamed the flag in llvm-profdata: <a href="https://github.com/llvm/llvm-project/commit/56926ae0faa67ac4c3b75609bfeea2eb324c0851" style="font-family:Arial,Helvetica,sans-serif" class="">https://github.com/llvm/llvm-project/commit/56926ae0faa67ac4c3b75609bfeea2eb324c0851</a></div>
<div class="gmail_default" style="font-family:times new roman,serif"><br class="">
</div>
<div class="gmail_default" style="font-family:times new roman,serif">Thanks,</div>
<div class="gmail_default" style="font-family:times new roman,serif">Wei.</div>
</div>
<br class="">
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Tue, May 12, 2020 at 10:31 AM Wei Mi <<a href="mailto:wmi@google.com" class="">wmi@google.com</a>> wrote:<br class="">
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div dir="ltr" class="">
<div dir="ltr" class="">
<div style="font-family:"times new roman",serif" class="">Thanks Artur for letting me know the problem. I will prepare a patch to fix it. </div>
<div style="font-family:"times new roman",serif" class=""><br class="">
</div>
<div style="font-family:"times new roman",serif" class="">Wei.</div>
</div>
<br class="">
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Tue, May 12, 2020 at 9:50 AM Artur Pilipenko <<a href="mailto:apilipenko@azul.com" target="_blank" class="">apilipenko@azul.com</a>> wrote:<br class="">
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Hi Wei, <br class="">
<br class="">
This change introduces a cl::opt partial-profile which conflicts with partial-profile option in llvm-profdata tool. The conflict happens in builds with LLVM_LINK_LLVM_DYLIB enabled. In this case the tools are linked with libLLVM and we end up with two definitions
for the same cl::opt.<br class="">
<br class="">
Can we rename one of the options to avoid this?<br class="">
<br class="">
Artur<br class="">
<br class="">
> On 8 May 2020, at 11:30, Wei Mi via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank" class="">llvm-commits@lists.llvm.org</a>> wrote:<br class="">
> <br class="">
> <br class="">
> Author: Wei Mi<br class="">
> Date: 2020-05-08T11:18:09-07:00<br class="">
> New Revision: aa2ddfc73d6e4a3369b7992aecaf107987c505b6<br class="">
> <br class="">
> URL: <a href="https://github.com/llvm/llvm-project/commit/aa2ddfc73d6e4a3369b7992aecaf107987c505b6" rel="noreferrer" target="_blank" class="">
https://github.com/llvm/llvm-project/commit/aa2ddfc73d6e4a3369b7992aecaf107987c505b6</a><br class="">
> DIFF: <a href="https://github.com/llvm/llvm-project/commit/aa2ddfc73d6e4a3369b7992aecaf107987c505b6.diff" rel="noreferrer" target="_blank" class="">
https://github.com/llvm/llvm-project/commit/aa2ddfc73d6e4a3369b7992aecaf107987c505b6.diff</a><br class="">
> <br class="">
> LOG: [SampleFDO] For functions without profiles, provide an option to put<br class="">
> them in a special text section.<br class="">
> <br class="">
> For sampleFDO, because the optimized build uses profile generated from<br class="">
> previous release, previously we couldn't tell a function without profile<br class="">
> was truely cold or just newly created so we had to treat them conservatively<br class="">
> and put them in .text section instead of .text.unlikely. The result was when<br class="">
> we persuing the best performance by locking .text.hot and .text in memory,<br class="">
> we wasted a lot of memory to keep cold functions inside.<br class="">
> <br class="">
> In <a href="https://reviews.llvm.org/D66374" rel="noreferrer" target="_blank" class="">
https://reviews.llvm.org/D66374</a>, we introduced profile symbol list to<br class="">
> discriminate functions being cold versus functions being newly added.<br class="">
> This mechanism works quite well for regular use cases in AutoFDO. However,<br class="">
> in some case, we can only have a partial profile when optimizing a target.<br class="">
> The partial profile may be an aggregated profile collected from many targets.<br class="">
> The profile symbol list method used for regular sampleFDO profile is not<br class="">
> applicable to partial profile use case because it may be too large and<br class="">
> introduce many false positives.<br class="">
> <br class="">
> To solve the problem for partial profile use case, we provide an option called<br class="">
> --profile-unknown-in-special-section. For functions without profile, we will<br class="">
> still treat them conservatively in compiler optimizations -- for example,<br class="">
> treat them as warm instead of cold in inliner. When we use profile info to<br class="">
> add section prefix for functions, we will discriminate functions known to be<br class="">
> not cold versus functions without profile (being unknown), and we will put<br class="">
> functions being unknown in a special text section called .text.unknown.<br class="">
> Runtime system will have the flexibility to decide where to put the special<br class="">
> section in order to achieve a balance between performance and memory saving.<br class="">
> <br class="">
> Differential Revision: <a href="https://reviews.llvm.org/D62540" rel="noreferrer" target="_blank" class="">
https://reviews.llvm.org/D62540</a><br class="">
> <br class="">
> Added: <br class="">
> <br class="">
> <br class="">
> Modified: <br class="">
> llvm/include/llvm/Analysis/ProfileSummaryInfo.h<br class="">
> llvm/lib/Analysis/ProfileSummaryInfo.cpp<br class="">
> llvm/lib/CodeGen/CodeGenPrepare.cpp<br class="">
> llvm/test/Transforms/SampleProfile/section-accurate-samplepgo.ll<br class="">
> <br class="">
> Removed: <br class="">
> <br class="">
> <br class="">
> <br class="">
> ################################################################################<br class="">
> diff --git a/llvm/include/llvm/Analysis/ProfileSummaryInfo.h b/llvm/include/llvm/Analysis/ProfileSummaryInfo.h<br class="">
> index e293d069f1f1..8fbc9e8990b2 100644<br class="">
> --- a/llvm/include/llvm/Analysis/ProfileSummaryInfo.h<br class="">
> +++ b/llvm/include/llvm/Analysis/ProfileSummaryInfo.h<br class="">
> @@ -72,13 +72,6 @@ class ProfileSummaryInfo {<br class="">
> Summary->getKind() == ProfileSummary::PSK_Sample;<br class="">
> }<br class="">
> <br class="">
> - /// Returns true if module \c M has partial-profile sample profile.<br class="">
> - bool hasPartialSampleProfile() {<br class="">
> - return hasProfileSummary() &&<br class="">
> - Summary->getKind() == ProfileSummary::PSK_Sample &&<br class="">
> - Summary->isPartialProfile();<br class="">
> - }<br class="">
> -<br class="">
> /// Returns true if module \c M has instrumentation profile.<br class="">
> bool hasInstrumentationProfile() {<br class="">
> return hasProfileSummary() &&<br class="">
> @@ -106,6 +99,8 @@ class ProfileSummaryInfo {<br class="">
> Optional<uint64_t> getProfileCount(const CallBase &CallInst,<br class="">
> BlockFrequencyInfo *BFI,<br class="">
> bool AllowSynthetic = false);<br class="">
> + /// Returns true if module \c M has partial-profile sample profile.<br class="">
> + bool hasPartialSampleProfile();<br class="">
> /// Returns true if the working set size of the code is considered huge.<br class="">
> bool hasHugeWorkingSetSize();<br class="">
> /// Returns true if the working set size of the code is considered large.<br class="">
> @@ -118,6 +113,8 @@ class ProfileSummaryInfo {<br class="">
> bool isFunctionEntryCold(const Function *F);<br class="">
> /// Returns true if \p F contains only cold code.<br class="">
> bool isFunctionColdInCallGraph(const Function *F, BlockFrequencyInfo &BFI);<br class="">
> + /// Returns true if the hotness of \p F is unknown.<br class="">
> + bool isFunctionHotnessUnknown(const Function &F);<br class="">
> /// Returns true if \p F contains hot code with regard to a given hot<br class="">
> /// percentile cutoff value.<br class="">
> bool isFunctionHotInCallGraphNthPercentile(int PercentileCutoff,<br class="">
> <br class="">
> diff --git a/llvm/lib/Analysis/ProfileSummaryInfo.cpp b/llvm/lib/Analysis/ProfileSummaryInfo.cpp<br class="">
> index dd53aa78f40f..ef33b9b1de5a 100644<br class="">
> --- a/llvm/lib/Analysis/ProfileSummaryInfo.cpp<br class="">
> +++ b/llvm/lib/Analysis/ProfileSummaryInfo.cpp<br class="">
> @@ -66,6 +66,10 @@ static cl::opt<int> ProfileSummaryColdCount(<br class="">
> cl::desc("A fixed cold count that overrides the count derived from"<br class="">
> " profile-summary-cutoff-cold"));<br class="">
> <br class="">
> +static cl::opt<bool> PartialProfile(<br class="">
> + "partial-profile", cl::Hidden, cl::init(false),<br class="">
> + cl::desc("Specify the current profile is used as a partial profile."));<br class="">
> +<br class="">
> // Find the summary entry for a desired percentile of counts.<br class="">
> static const ProfileSummaryEntry &getEntryForPercentile(SummaryEntryVector &DS,<br class="">
> uint64_t Percentile) {<br class="">
> @@ -192,6 +196,11 @@ bool ProfileSummaryInfo::isFunctionColdInCallGraph(const Function *F,<br class="">
> return true;<br class="">
> }<br class="">
> <br class="">
> +bool ProfileSummaryInfo::isFunctionHotnessUnknown(const Function &F) {<br class="">
> + assert(hasPartialSampleProfile() && "Expect partial sample profile");<br class="">
> + return !F.getEntryCount().hasValue();<br class="">
> +}<br class="">
> +<br class="">
> template<bool isHot><br class="">
> bool ProfileSummaryInfo::isFunctionHotOrColdInCallGraphNthPercentile(<br class="">
> int PercentileCutoff, const Function *F, BlockFrequencyInfo &BFI) {<br class="">
> @@ -399,6 +408,12 @@ bool ProfileSummaryInfo::isColdCallSite(const CallBase &CB,<br class="">
> return hasSampleProfile() && CB.getCaller()->hasProfileData();<br class="">
> }<br class="">
> <br class="">
> +bool ProfileSummaryInfo::hasPartialSampleProfile() {<br class="">
> + return hasProfileSummary() &&<br class="">
> + Summary->getKind() == ProfileSummary::PSK_Sample &&<br class="">
> + (PartialProfile || Summary->isPartialProfile());<br class="">
> +}<br class="">
> +<br class="">
> INITIALIZE_PASS(ProfileSummaryInfoWrapperPass, "profile-summary-info",<br class="">
> "Profile summary info", false, true)<br class="">
> <br class="">
> <br class="">
> diff --git a/llvm/lib/CodeGen/CodeGenPrepare.cpp b/llvm/lib/CodeGen/CodeGenPrepare.cpp<br class="">
> index 359618d6669a..eceee30c532f 100644<br class="">
> --- a/llvm/lib/CodeGen/CodeGenPrepare.cpp<br class="">
> +++ b/llvm/lib/CodeGen/CodeGenPrepare.cpp<br class="">
> @@ -177,6 +177,17 @@ static cl::opt<bool> ProfileGuidedSectionPrefix(<br class="">
> "profile-guided-section-prefix", cl::Hidden, cl::init(true), cl::ZeroOrMore,<br class="">
> cl::desc("Use profile info to add section prefix for hot/cold functions"));<br class="">
> <br class="">
> +static cl::opt<bool> ProfileUnknownInSpecialSection(<br class="">
> + "profile-unknown-in-special-section", cl::Hidden, cl::init(false),<br class="">
> + cl::ZeroOrMore,<br class="">
> + cl::desc("In profiling mode like sampleFDO, if a function doesn't have "<br class="">
> + "profile, we cannot tell the function is cold for sure because "<br class="">
> + "it may be a function newly added without ever being sampled. "<br class="">
> + "With the flag enabled, compiler can put such profile unknown "<br class="">
> + "functions into a special section, so runtime system can choose "<br class="">
> + "to handle it in a <br class="">
> diff erent way than .text section, to save "<br class="">
> + "RAM for example. "));<br class="">
> +<br class="">
> static cl::opt<unsigned> FreqRatioToSkipMerge(<br class="">
> "cgp-freq-ratio-to-skip-merge", cl::Hidden, cl::init(2),<br class="">
> cl::desc("Skip merging empty blocks if (frequency of empty block) / "<br class="">
> @@ -452,6 +463,9 @@ bool CodeGenPrepare::runOnFunction(Function &F) {<br class="">
> F.setSectionPrefix(".hot");<br class="">
> else if (PSI->isFunctionColdInCallGraph(&F, *BFI))<br class="">
> F.setSectionPrefix(".unlikely");<br class="">
> + else if (ProfileUnknownInSpecialSection && PSI->hasPartialSampleProfile() &&<br class="">
> + PSI->isFunctionHotnessUnknown(F))<br class="">
> + F.setSectionPrefix(".unknown");<br class="">
> }<br class="">
> <br class="">
> /// This optimization identifies DIV instructions that can be<br class="">
> <br class="">
> diff --git a/llvm/test/Transforms/SampleProfile/section-accurate-samplepgo.ll b/llvm/test/Transforms/SampleProfile/section-accurate-samplepgo.ll<br class="">
> index 82c407ebb9d2..6ac39a167689 100644<br class="">
> --- a/llvm/test/Transforms/SampleProfile/section-accurate-samplepgo.ll<br class="">
> +++ b/llvm/test/Transforms/SampleProfile/section-accurate-samplepgo.ll<br class="">
> @@ -1,5 +1,6 @@<br class="">
> ; REQUIRES: x86-registered-target<br class="">
> ; RUN: opt < %s -sample-profile -sample-profile-file=%S/Inputs/inline.prof -codegenprepare -S | FileCheck %s<br class="">
> +; RUN: opt < %s -sample-profile -sample-profile-file=%S/Inputs/inline.prof -codegenprepare -profile-unknown-in-special-section -partial-profile -S | FileCheck %s --check-prefix UNKNOWN<br class="">
> ; RUN: opt < %s -sample-profile -sample-profile-file=%S/Inputs/inline.prof -codegenprepare -profile-sample-accurate -S | FileCheck %s --check-prefix ACCURATE<br class="">
> <br class="">
> target triple = "x86_64-pc-linux-gnu"<br class="">
> @@ -11,7 +12,8 @@ target triple = "x86_64-pc-linux-gnu"<br class="">
> declare void @hot_func()<br class="">
> <br class="">
> ; CHECK-NOT: foo_not_in_profile{{.*}}!section_prefix<br class="">
> -; CHECK: foo_not_in_profile{{.*}}!prof ![[UNKNOWN_ID:[0-9]+]]<br class="">
> +; CHECK: foo_not_in_profile{{.*}}!prof ![[NOPROFILE_ID:[0-9]+]]<br class="">
> +; UNKNOWN: foo_not_in_profile{{.*}}!prof ![[NOPROFILE_ID:[0-9]+]] !section_prefix ![[UNKNOWN_ID:[0-9]+]]<br class="">
> ; ACCURATE: foo_not_in_profile{{.*}}!prof ![[ZERO_ID:[0-9]+]] !section_prefix ![[COLD_ID:[0-9]+]]<br class="">
> ; The function not appearing in profile is cold when -profile-sample-accurate<br class="">
> ; is on.<br class="">
> @@ -31,9 +33,11 @@ define void @bar_not_in_profile() #0 {<br class="">
> <br class="">
> attributes #0 = { "profile-sample-accurate" }<br class="">
> <br class="">
> -; CHECK: ![[UNKNOWN_ID]] = !{!"function_entry_count", i64 -1}<br class="">
> +; CHECK: ![[NOPROFILE_ID]] = !{!"function_entry_count", i64 -1}<br class="">
> ; CHECK: ![[ZERO_ID]] = !{!"function_entry_count", i64 0}<br class="">
> ; CHECK: ![[COLD_ID]] = !{!"function_section_prefix", !".unlikely"}<br class="">
> +; UNKNOWN: ![[NOPROFILE_ID]] = !{!"function_entry_count", i64 -1}<br class="">
> +; UNKNOWN: ![[UNKNOWN_ID]] = !{!"function_section_prefix", !".unknown"}<br class="">
> ; ACCURATE: ![[ZERO_ID]] = !{!"function_entry_count", i64 0}<br class="">
> ; ACCURATE: ![[COLD_ID]] = !{!"function_section_prefix", !".unlikely"}<br class="">
> !llvm.module.flags = !{!1}<br class="">
> <br class="">
> <br class="">
> <br class="">
> _______________________________________________<br class="">
> llvm-commits mailing list<br class="">
> <a href="mailto:llvm-commits@lists.llvm.org" target="_blank" class="">llvm-commits@lists.llvm.org</a><br class="">
> <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank" class="">
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br class="">
<br class="">
</blockquote>
</div>
</div>
</blockquote>
</div>
</div>
</div>
</blockquote>
</div>
<br class="">
</div>
</body>
</html>