[PATCH] D17584: Cleanup inline cost analysis code

Easwaran Raman via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 26 17:04:19 PST 2016

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. 


More information about the llvm-commits mailing list