<div dir="ltr">Ping</div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Mar 14, 2016 at 1:41 PM, Easwaran Raman <span dir="ltr"><<a href="mailto:eraman@google.com" target="_blank">eraman@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">eraman added inline comments.<br>
<br>
================<br>
<span class="">Comment at: lib/Analysis/InlineCost.cpp:1319-1320<br>
@@ -1305,9 +1318,4 @@<br>
   // Note that we *must not* cache the size, this loop grows the worklist.<br>
</span><span class="">   for (unsigned Idx = 0; Idx != BBWorklist.size(); ++Idx) {<br>
-    // Bail out the moment we cross the threshold. This means we'll under-count<br>
-    // the cost, but only when undercounting doesn't matter.<br>
</span><span class="">-    if (Cost > Threshold)<br>
-      break;<br>
-<br>
     BasicBlock *BB = BBWorklist[Idx];<br>
     if (BB->empty())<br>
----------------<br>
</span>There is a subtle bug in my reasoning that is caught by adding the assert as suggested by Philip. The invariant is maintained at the beginning of the first iteration. In the first iteration, if analyzeBlock returned true (Cost <= Threshold), Cost could still end up > Threshold when SingleBBBonus is subtracted, violating the invariant in the second iteration. What I have done now is to add a if (Cost > Threshold) return false immediately after the SingleBBBonus is subtracted. I think this is a bit more readable since the check is present immediately after the bonus is subtracted.<br>
<br>
<br>
<a href="http://reviews.llvm.org/D17584" rel="noreferrer" target="_blank">http://reviews.llvm.org/D17584</a><br>
<br>
<br>
<br>
</blockquote></div><br></div>