[PATCH] D17584: Cleanup inline cost analysis code

David Li via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 26 20:57:44 PST 2016


davidxl accepted this revision.
davidxl added a reviewer: davidxl.
davidxl added a comment.
This revision is now accepted and ready to land.

good cleanup -- and I certainly hope we can do more cleanups in the future. Given the current state, I think it is worth doing a chrome size testing before committing. lgtm

David


================
Comment at: lib/Analysis/InlineCost.cpp:1328
@@ -1327,3 @@
-    if (!analyzeBlock(BB, EphValues)) {
-      if (IsRecursiveCall || ExposesReturnsTwice || HasDynamicAlloca ||
-          HasIndirectBr || HasFrameEscape)
----------------
eraman wrote:
> 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. 
ok -- looks like a left over in the original patch.


http://reviews.llvm.org/D17584





More information about the llvm-commits mailing list