[PATCH] D147964: [VPlan] Introduce new entry block to VPlan for early SCEV expansion.

[Ayal Zaks via Phabricator via llvm-commits]

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

Looks good to me, ship it!
Summary should be updated a bit.



================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.cpp:1139-1140
   else {
-    VPBasicBlock *Preheader = Plan.getEntry();
     Expanded = new VPExpandSCEVRecipe(Expr, SE);
-    Preheader->appendRecipe(Expanded->getDefiningRecipe());
+    Plan.getPreheader()->appendRecipe(Expanded->getDefiningRecipe());
   }
----------------
fhahn wrote:
> Ayal wrote:
> > nit: can cast instead of getting defined recipe.
> Will address separately.
All ExpandSCEVRecipes are placed in preheader, rather than vector preheader / entry, right?


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.cpp:603
+  VPBasicBlock *Preheader = new VPBasicBlock("ph");
+
+  VPBasicBlock *VecPreheader = new VPBasicBlock("vector.ph");
----------------
nit: redundant empty line


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.cpp:606-608
+  VPValue *TripCountVPV =
+      vputils::getOrCreateVPValueForSCEVExpr(*Plan, TripCount, SE);
+  Plan->TripCount = TripCountVPV;
----------------



================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.cpp:1118
       RPOT(VPBlockDeepTraversalWrapper<const VPBlockBase *>(Plan.getEntry()));
   for (const VPBasicBlock *VPBB :
        VPBlockUtils::blocksOnly<const VPBasicBlock>(RPOT))
----------------
nit: auto? (Independent of this patch)


================
Comment at: llvm/test/Transforms/LoopVectorize/pointer-induction-unroll.ll:27
 ; STRIDED-NEXT:    [[MUL:%.*]] = shl i32 [[TMP0:%.*]], 1
+; STRIDED-NEXT:    [[TMP1:%.*]] = sext i32 [[MUL]] to i64
 ; STRIDED-NEXT:    br i1 false, label [[SCALAR_PH:%.*]], label [[VECTOR_PH:%.*]]
----------------
Note the hoist of sext from vector.ph to entry/ph, hopefully to be sunk later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147964