<div dir="ltr">It was introduced in r153812 by Chandler and AFAICT, even then the break from the loop would eventually result in returning false. </div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Feb 26, 2016 at 5:05 PM, Xinliang David Li <span dir="ltr"><<a href="mailto:davidxl@google.com" target="_blank">davidxl@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><div><div class="h5">On Fri, Feb 26, 2016 at 5:04 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>
<span><br>
================<br>
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>
   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>
-    if (Cost > Threshold)<br>
-      break;<br>
-<br>
     BasicBlock *BB = BBWorklist[Idx];<br>
     if (BB->empty())<br>
----------------<br>
</span><span>davidxl wrote:<br>
> why is the early bail out removed?<br>
</span>Cost cannot be greater than Threshold in the first iteration - since there is a similar check in line 1245 above. Subsequently, if Cost exceeds Threshold, that's caught in analyzeBB which returns false and the bailout happens in that case.<br>
<span><br>
================<br>
Comment at: lib/Analysis/InlineCost.cpp:1328<br>
@@ -1327,3 @@<br>
-    if (!analyzeBlock(BB, EphValues)) {<br>
-      if (IsRecursiveCall || ExposesReturnsTwice || HasDynamicAlloca ||<br>
-          HasIndirectBr || HasFrameEscape)<br>
----------------<br>
</span><span>davidxl wrote:<br>
> Why are these checks removed? Are they redundant?  They do look like to be in  the wrong place. Can you find out the original intention of the code? Is there a regression test somewhere?<br>
</span>I don't know why they were in the first place. If analyzeBlock returns false and it reaches the break stmt, it only means Cost > Threshold and we should be bailing out. There is no reason to break out of loop and return false at the end.<br></blockquote><div><br></div></div></div><div>Can you do a svn blame to find more details ?</div><div><br></div><div>David </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<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></div>
</blockquote></div><br></div>