[PATCH] D116473: [VPlan] Don't consider VPWidenCanonicalIVRecipe phi-like.

[Ayal Zaks via Phabricator via llvm-commits]

Ayal accepted this revision.
Ayal added a comment.
This revision is now accepted and ready to land.

This looks good to me, thanks!



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8451
     // This is used instead of IV < TC because TC may wrap, unlike BTC.
     // Start by constructing the desired canonical IV in the header block.
     VPValue *IV = nullptr;
----------------
fhahn wrote:
> Ayal wrote:
> > How about ironing this a bit, while we're here, e.g.:
> > 
> > ```
> >     // Introduce the early-exit compare IV <= BTC to form header block mask.
> >     // This is used instead of IV < TC because TC may wrap, unlike BTC.
> >     // Start by constructing the desired canonical IV in the header block as its first non-phi instructions.
> >     assert(CM.foldTailByMasking() && "must fold the tail");
> >     VPBasicBlock *HeaderVPBB = Plan->getEntry()->getEntryBasicBlock();
> >     auto NewInsertionPoint = HeaderVPBB->getFirstNonPhi();
> > 
> >     VPValue *IV = nullptr;
> >     if (Legal->getPrimaryInduction())
> >       IV = Plan->getOrAddVPValue(Legal->getPrimaryInduction());
> >     else {
> >       auto *IVRecipe = new VPWidenCanonicalIVRecipe();
> >       HeaderVPBB->insert(IVRecipe, NewInsertionPoint);
> >       IV = IVRecipe;
> >     }
> > 
> >     assert(Builder.getInsertBlock() == HeaderVPBB && "..."); // if desired?
> >     VPBuilder::InsertPointGuard Guard(Builder);
> >     Builder.setInsertPoint(HeaderVPBB, NewInsertionPoint);
> > ```
> This now means that the compare recipe always gets created in the header VPBB whereas before it may be created in a different VPBB (but still reachable from the header through single branches). This causes some slight changes in the order the IR is generated, but overall seems like a nice simplification. I updated the patch.
Well, the documentation claims that the desired canonical IV is  constructed "in the header block"

The distinct VPBB's seem to be equivalent, necessarily? Such as the entry of a replicating region and its single predecessor that appear in vplan-sink-scalars-and-merge.ll, which should be fused during code-gen.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116473