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

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
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());
   }
----------------
Ayal wrote:
> 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?
Yep!


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


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.cpp:606-608
+  VPValue *TripCountVPV =
+      vputils::getOrCreateVPValueForSCEVExpr(*Plan, TripCount, SE);
+  Plan->TripCount = TripCountVPV;
----------------
Ayal wrote:
> 
Simplified in the committed version, thanks!


================
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:%.*]]
----------------
Ayal wrote:
> Note the hoist of sext from vector.ph to entry/ph, hopefully to be sunk later.
Yes, the backend at the latest should be able to sink it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147964