<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, May 5, 2017 at 4:08 PM, Dehao Chen <span dir="ltr"><<a href="mailto:danielcdh@gmail.com" target="_blank">danielcdh@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Fri, May 5, 2017 at 4:03 PM, Teresa Johnson via Phabricator<br>
<<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br>
> tejohnson added a comment.<br>
><br>
> In <a href="https://reviews.llvm.org/D32877#747704" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D32877#747704</a>, @danielcdh wrote:<br>
><br>
>> In <a href="https://reviews.llvm.org/D32877#747667" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D32877#747667</a>, @tejohnson wrote:<br>
>><br>
>> > In <a href="https://reviews.llvm.org/D32877#747647" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D32877#747647</a>, @eraman wrote:<br>
>> ><br>
>> > > I feel the underlying problem here is the use of VP metadata to get counts in sample PGO mode.  Dehao, even when VP metadata is available, can we add branch_weights to the call? Then, only sample PGO will annotate branch weights to calls and that won't affect instrumented PGO.<br>
>> ><br>
>> ><br>
>> > That sounds like potentially a good longer term change. But I'd still like for this to go in as an immediate fix for the issue, which is causing a noticeable impact on instrumentation PGO performance, and because there is no reason AFAIK for anything other than Sample PGO to need to look at call metadata for determining hotness.<br>
>><br>
>><br>
>> Just to confirm, with <a href="https://reviews.llvm.org/D32773" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D32773</a>, we should have recovered all the instrumented PGO performance, isn't it?<br>
><br>
><br>
> You're right, I suspect that it may (will be checking after it is fully integrated here), but my concern is that there may still be other transformations that don't update the VP metadata properly (I mentioned earlier this week that e.g. it appears the metadata isn't being dropped when calls are devirtualized after constant prop, although I don't think this causes the performance issue). These things should be fixed for other reasons including Sample PGO accuracy, but it is not needed for instrumentation PGO in the first place - is there any reason for anything other than Sample PGO to look at this instead of the branch weights?<br>
><br>
><br>
><br>
> ================<br>
> Comment at: lib/Analysis/<wbr>ProfileSummaryInfo.cpp:88<br>
> +  else if (auto Kind = getKind(Inst->getModule()))<br>
> +    IsSamplePGO = Kind.getValue() == ProfileSummary::PSK_Sample;<br>
> +  if (IsSamplePGO) {<br>
> ----------------<br>
> danielcdh wrote:<br>
>> Looks like you can always get the Kind from Inst, why would you want to pass in the Summary?<br>
> It involves doing some work that is already done when the summary is available, so this was added just for the case where it is invoked without a summary.<br>
<br>
</span>I mean can we remove the "Summary" parameter of this function, and<br>
always compute this from getKind(Inst->getModule())?<br></blockquote><div><br></div><div>I know that's what you were suggesting, but my point is that that requires doing some additional work for every instruction (this routine will be called many times), when we already have that in many cases cached on the ProfileSummary object.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Dehao<br>
<br>
><br>
><br>
> <a href="https://reviews.llvm.org/D32877" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D32877</a><br>
><br>
><br>
><br>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature" data-smartmail="gmail_signature"><span style="font-family:Times;font-size:medium"><table cellspacing="0" cellpadding="0"><tbody><tr style="color:rgb(85,85,85);font-family:sans-serif;font-size:small"><td nowrap style="border-top-style:solid;border-top-color:rgb(213,15,37);border-top-width:2px">Teresa Johnson |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(51,105,232);border-top-width:2px"> Software Engineer |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(0,153,57);border-top-width:2px"> <a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.com</a> |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(238,178,17);border-top-width:2px"> 408-460-2413</td></tr></tbody></table></span></div>
</div></div>