<div dir="ltr"><div dir="ltr"><div class="gmail_default" style="font-family:monospace;font-size:small;color:#000000"><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Jul 22, 2020 at 5:49 PM Wei Mi <<a href="mailto:wmi@google.com">wmi@google.com</a>> wrote:<br></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"><div dir="ltr"><div style="font-family:"times new roman",serif"><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Jul 22, 2020 at 3:57 PM David Li via Phabricator <<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">davidxl added a comment.<br>
<br>
Inlining won't be helped unless there is a hot callsite to the all-zero count function -- but this should not exist. </blockquote><div><br></div><div><div style="font-family:"times new roman",serif">Inlining may be helped because of the hot callee heuristic. The threshold will be higher (325) but not be as high as hot callsite (3000). </div></div></div></div></blockquote><div><br></div><div><br></div><div class="gmail_default" style="font-family:monospace;font-size:small;color:rgb(0,0,0)">The cold callsite heuristic will supersede, so hot callee heuristic won't kick in.</div><div class="gmail_default" style="font-family:monospace;font-size:small;color:rgb(0,0,0)"></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"><div class="gmail_quote"><div><div style="font-family:"times new roman",serif">And the callsites inside of the function may have more inlining because of hot callsite heuristic. How hot a callsite inside of the function is depends on the entry count value of the function.</div><br></div><div> </div></div></div></blockquote><div><br></div><div><div class="gmail_default" style="font-family:monospace;font-size:small;color:rgb(0,0,0)">Since this is approximate, the scaling factor of internal block counts can still be determined by relative hotness of function in sample profile if that helps, but it is hard to measure the secondary effects compared with the cost of putting it wrongly in .text.unlikely.</div><br></div><div><br></div><div> </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"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">I think the major performance hit comes from 1) text.unlikely which may not be mlocked; and 2) all unbiased branches due to zero weights. So doing this depending it on entry count existence is fine, but we still to teach PGOUse to drop the body. I think a simpler design would be<br>
<br>
At llvm_profdata side:<br>
<br>
1. if the instrumentation cold function has enough internal counts, just scale up the max internal counts to be a multiple of hot threshold</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
2. if the cold function has all zero counts or we believe all their internal counts are not trustworthy (basically ignore step 1) with an option), we can simply discard the function entry completely (to signal this function is actually hot, but we don't know internal counts)<br>
<br>
At PGOUse side:<br>
<br>
if we don't find counters for a function, set the function's entry value to be above hot threshold (a function statically linked in should always have counts. If there are not counts, it means it is corrected by llvm-profdata).<br></blockquote><div><br></div><div><div style="font-family:"times new roman",serif">Ok, in this way we keep a valid entry count in the metadata during optimized build without depending on the profile format always having an entry count in each function.</div></div></div></div></blockquote><div><br></div><div class="gmail_default" style="font-family:monospace;font-size:small;color:rgb(0,0,0)">yes.</div><div class="gmail_default" style="font-family:monospace;font-size:small;color:rgb(0,0,0)"></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"><div class="gmail_quote"><div><div style="font-family:"times new roman",serif"> However in this way, we won't be able to treat warm and hot functions differently. We will need to treat all the functions as hot even if they are only warm in production (I think we want to move both hot and warm functions out of .text.unlikely section). <span class="gmail_default" style="font-family:monospace;font-size:small;color:rgb(0,0,0)"> </span></div></div></div></div></blockquote><div><br></div><div class="gmail_default" style="font-family:monospace;font-size:small;color:rgb(0,0,0)">Number of functions like these are assumed to be small. If there are large number of such functions, the training data should really be fixed up instead of relying on this method.</div><div class="gmail_default" style="font-family:monospace;font-size:small;color:rgb(0,0,0)"><br></div><div class="gmail_default" style="font-family:monospace;font-size:small;color:rgb(0,0,0)">David</div><div class="gmail_default" style="font-family:monospace;font-size:small;color:rgb(0,0,0)"></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"><div class="gmail_quote"><div><div style="font-family:"times new roman",serif"><span class="gmail_default" style="font-family:monospace;font-size:small;color:rgb(0,0,0)"></span></div></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
<br>
Repository:<br>
rL LLVM<br>
<br>
CHANGES SINCE LAST ACTION<br>
<a href="https://reviews.llvm.org/D81981/new/" rel="noreferrer" target="_blank">https://reviews.llvm.org/D81981/new/</a><br>
<br>
<a href="https://reviews.llvm.org/D81981" rel="noreferrer" target="_blank">https://reviews.llvm.org/D81981</a><br>
<br>
<br>
<br>
</blockquote></div></div>
</blockquote></div></div>