<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Apr 30, 2018 at 8:51 PM, Wei Mi via Phabricator <span dir="ltr"><<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">wmi added inline comments.<br>
<span class=""><br>
<br>
================<br>
Comment at: lib/Transforms/IPO/<wbr>SampleProfile.cpp:368<br>
+  uint64_t CallsiteTotalSamples = CallsiteFS->getTotalSamples();<br>
+  if (PSI && PSI->isHotCount(<wbr>CallsiteTotalSamples))<br>
+    return true;<br>
----------------<br>
</span><span class="">danielcdh wrote:<br>
> In what situations will PSI be nullptr? If not, then please assert it instead.<br>
> <br>
> Also, I think this will overwrite the later (PercentSamples >= SampleProfileHotThreshold) heuristic, and we should remove that flag.<br>
</span>PSI will not be nullptr. Will add an assertion.<br>
<br>
It is possible that for a callsite its CallsiteTotalSamples is less than hot cutoff threshold but still have a PententSamples larger than SampleProfileHotThreshold. My original plan is if a callsite is inlined currently, the new heuristic will still keep it. <br>
<br>
But I check where SampleProfileHotThreshold is used and find it is also used to populate the InlinedGUIDs set. To make that simple and consistent, like you suggest, I may remove SampleProfileHotThreshold and related heuristic. <br></blockquote><div><br></div><div>I think you will also need to update that to make it consistent with the early inline heuristic, otherwise the thinlto compile phase may not be able to find the function to inline.</div><div><br></div><div>Dehao</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5"><br>
<br>
Repository:<br>
  rL LLVM<br>
<br>
<a href="https://reviews.llvm.org/D45377" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D45377</a><br>
<br>
<br>
<br>
</div></div></blockquote></div><br></div></div>