[PATCH] D60740: [InlineCost] cleanup calculations of Cost and Threshold

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added a comment.

Pointing out the (serious) bug in this change below.



================
Comment at: llvm/trunk/lib/Analysis/InlineCost.cpp:1337
+    // Make sure the bonus doesn't go below zero.
+    Threshold += std::max(0, CA.getThreshold() - CA.getCost());
   }
----------------
I think this is the source of the (huge) behavior change.

We reduced the cost to apply some forms of bonus because it only decreases the *cost* of inlining, but doesn't increase the *benefit* of inlining. Now that is reversed.

So in this case, if the inner call is to a function where this is the last call, the threshold is now *HUGE*, and the cost is small. We take that huge threshold, and apply it blindly to our threshold here.

This will cause us to inline waaay too much along call stacks that (eventually) have an reachable, inlinable call to function which is the last call to that function.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60740/new/

https://reviews.llvm.org/D60740