[PATCH] D28583: CodeGen: Allow small copyable blocks to "break" the CFG.

[Xinliang David Li via llvm-commits]

On Mon, Jan 23, 2017 at 8:33 PM, Kyle Butt via Phabricator <
reviews at reviews.llvm.org> wrote:

> iteratee added inline comments.
>
>
> ================
> Comment at: lib/CodeGen/MachineBlockPlacement.cpp:642
> +  // increases fallthrough.
> +  if (SuccSuccs.size() == 0)
> +    return true;
> ----------------
> davidxl wrote:
> > davidxl wrote:
> > > I assume this is loop back edge source block. You need a test case to
> cover it.
> > test case for this?
> It's not just a back edge. I added a test case.
>
>
> ================
> Comment at: lib/CodeGen/MachineBlockPlacement.cpp:671
> +  BranchProbability PProb = MBPI->getEdgeProbability(BB, Succ);
> +  BlockFrequency P = BBFreq * PProb;
> +  // At this point, we don't know which block would be chosen instead of
> Succ.
> ----------------
> davidxl wrote:
> > Add a short cut here with comments:
> >
> > // If P is not larger, the best successor selection loop will eventually
> select C, not Succ (as it is not profitable to do so).
> > if (P <= Qout)
> >    return false;
> If we weren't estimating Qout, I'd agree. Instead we'll skip calling this
> altogether if we know that we won't use the result.
>

I don't understand the reply. The early check I suggested is basically:

if ( AdjustedSumProb - PProb > PProb)
   return false;

David

>
>
> https://reviews.llvm.org/D28583
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170123/ac8346f5/attachment.html>