[PATCH] D115793: [VPlan] Create header & latch blocks for plan skeleton up front (NFC).

[Ayal Zaks via Phabricator via llvm-commits]

Ayal added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8798
   VPBlockUtils::insertBlockAfter(RegSucc, Region);
+  VPBlockUtils::connectBlocks(RegSucc, SingleSucc);
   return RegSucc;
----------------
Perhaps something like VPBlockUtils::insertBlockOnEdge() would help take care of ensuring single successor, disconnecting, reconnecting? Admittedly two blocks are being inserted here.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9022
+  VPBasicBlock *HeaderVPBB = new VPBasicBlock(OrigLoop->getHeader()->getName());
+  auto *TopRegion = new VPRegionBlock("vector loop");
+  TopRegion->setEntry(HeaderVPBB);
----------------
Feed Entry and Exit to constructor of VPRegionBlock instead of setting them explicitly?
Feed Entry to constructor of VPlan instead of setting it explicitly?


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9045
+    else
+      FirstIter = false;
     VPBB = FirstVPBBForBB;
----------------
Can be simplified into:

  if (FillHeaderVPBB)
    FillHeaderVPBB = false;
  else {
    auto *FirstVPBBForBB = new VPBasicBlock(BB->getName());
    VPBlockUtils::insertBlockAfter(FirstVPBBForBB, VPBB);
    VPBB = FirstVPBBForBB;
  }

?

(Always generating a new VPBB and fusing the empty "dummy" header later, along with fusing latch below, would be reverting D111299?)


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.h:2370
+    SmallVector<VPBlockBase *> Succs(BlockPtr->getSuccessors().begin(),
+                                     BlockPtr->getSuccessors().end());
+    for (VPBlockBase *Succ : Succs) {
----------------
Something early_inc_range could handle?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115793