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

[Xinliang David Li via llvm-commits]

On Tue, Jan 24, 2017 at 5:19 PM, Kyle Butt via Phabricator <
reviews at reviews.llvm.org> wrote:

> iteratee added inline comments.
>
>
> ================
> Comment at: lib/CodeGen/MachineBlockPlacement.cpp:1086
> +  if (DupCandidates.size() != 0)
> +    std::make_heap(DupCandidates.begin(), DupCandidates.end());
> +  while (DupCandidates.size() != 0) {
> ----------------
> davidxl wrote:
> > Why not just stable sort it? The vector should be of size 1 for most of
> the cases.  Also why do you need position ?
> Will just sort the vector.
>
> Position is because we rely on the successor order being stable and the
> first successor being a subtle hint. Without the position, we lose track of
> whether the block in the vector came before or after the block we picked
> without tail duplication.
>
>
would stable_sort not work for this case?

David


>
> ================
> Comment at: lib/CodeGen/MachineBlockPlacement.cpp:1098
> +    if (canTailDuplicateUnplacedPreds(BB, Succ, Chain, BlockFilter)
> +        && isProfitableToTailDup(BB, Succ, AdjustedSumProb, Chain,
> +                                     BlockFilter)) {
> ----------------
> davidxl wrote:
> > isProfitableToTailDup assumes the baseline layout does not pick Succ.
> The assumption may not be true here as there are other two possibilities:
> > 1) Succ == BestSucc.BB in the base layout
> > 2) BestSucc.BB == null in the base layout (all BB's successors have
> conflicts).
> >
> > In such two cases, isProfitable check should probably be skipped (as it
> is benefitial)
> 1.
> Succ won't equal BestSucc.BB because of the continue. These blocks were
> not chosen by the first loop by construction.
>
> 2.
> Good catch. I'll add that.
>
>
> https://reviews.llvm.org/D28583
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170125/510cf125/attachment.html>