[PATCH] D17584: Cleanup inline cost analysis code

[Easwaran Raman via llvm-commits]

eraman added inline comments.

================
Comment at: lib/Analysis/InlineCost.cpp:1319-1320
@@ -1305,9 +1318,4 @@
   // Note that we *must not* cache the size, this loop grows the worklist.
   for (unsigned Idx = 0; Idx != BBWorklist.size(); ++Idx) {
-    // Bail out the moment we cross the threshold. This means we'll under-count
-    // the cost, but only when undercounting doesn't matter.
-    if (Cost > Threshold)
-      break;
-
     BasicBlock *BB = BBWorklist[Idx];
     if (BB->empty())
----------------
davidxl wrote:
> why is the early bail out removed?
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. 

================
Comment at: lib/Analysis/InlineCost.cpp:1328
@@ -1327,3 @@
-    if (!analyzeBlock(BB, EphValues)) {
-      if (IsRecursiveCall || ExposesReturnsTwice || HasDynamicAlloca ||
-          HasIndirectBr || HasFrameEscape)
----------------
davidxl wrote:
> 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?
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. 


http://reviews.llvm.org/D17584