[PATCH] D17584: Cleanup inline cost analysis code

[Easwaran Raman via llvm-commits]

Ping

On Mon, Mar 14, 2016 at 1:41 PM, Easwaran Raman <eraman at google.com> wrote:

> 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())
> ----------------
> There is a subtle bug in my reasoning that is caught by adding the assert
> as suggested by Philip. The invariant is maintained at the beginning of the
> first iteration. In the first iteration, if analyzeBlock returned true
> (Cost <= Threshold), Cost could still end up > Threshold when SingleBBBonus
> is subtracted, violating the invariant in the second iteration. What I have
> done now is to add a if (Cost > Threshold) return false immediately after
> the SingleBBBonus is subtracted. I think this is a bit more readable since
> the check is present immediately after the bonus is subtracted.
>
>
> http://reviews.llvm.org/D17584
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160318/41f6597e/attachment.html>