!fixup address latest comments, thanks

Author: fhahn

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

@@ -9323,9 +9323,9 @@ LoopVectorizationPlanner::tryToBuildVPlanWithVPRecipes(VFRange &Range) {
           Range);
   auto Plan = std::make_unique<VPlan>(OrigLoop);
   // Build hierarchical CFG.
-  VPlanHCFGBuilder HCFGBuilder(OrigLoop, LI, *Plan);
-  // TODO: Convert to VPlan-transform and consoliate all transforms for VPlan
+  // Convert to VPlan-transform and consoliate all transforms for VPlan
   // creation.
+  VPlanHCFGBuilder HCFGBuilder(OrigLoop, LI, *Plan);
   HCFGBuilder.buildHierarchicalCFG();
 
   VPlanTransforms::introduceTopLevelVectorLoopRegion(

llvm/lib/Transforms/Vectorize/VPlan.h

@@ -3570,6 +3570,13 @@ class VPlan {
     return TripCount;
   }
 
+  /// Set the trip count assuming it is currently null; if it is not - use
+  /// resetTripCount().
+  void setTripCount(VPValue *NewTripCount) {
+    assert(!TripCount && NewTripCount && "TripCount should not be set yet.");
+    TripCount = NewTripCount;
+  }
+
   /// Resets the trip count for the VPlan. The caller must make sure all uses of
   /// the original trip count have been replaced.
   void resetTripCount(VPValue *NewTripCount) {
@@ -3578,14 +3585,6 @@ class VPlan {
     TripCount = NewTripCount;
   }
 
-  // Set the trip count assuming it is currently null; if it is not - use
-  // resetTripCount().
-  void setTripCount(VPValue *NewTripCount) {
-    assert(!TripCount && NewTripCount && "TripCount should not be set yet.");
-
-    TripCount = NewTripCount;
-  }
-
   /// The backedge taken count of the original loop.
   VPValue *getOrCreateBackedgeTakenCount() {
     if (!BackedgeTakenCount)

llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp

@@ -32,7 +32,8 @@ void VPlanTransforms::introduceTopLevelVectorLoopRegion(
   VPBlockUtils::disconnectBlocks(OriginalLatch, HeaderVPBB);
   VPBasicBlock *VecPreheader = Plan.createVPBasicBlock("vector.ph");
   VPBlockUtils::connectBlocks(Plan.getEntry(), VecPreheader);
-  assert(OriginalLatch->getNumSuccessors() == 0 && "expected no predecessors");
+  assert(OriginalLatch->getNumSuccessors() == 0 &&
+         "Plan should end at top level latch");
 
   // Create SCEV and VPValue for the trip count.
   // We use the symbolic max backedge-taken-count, which works also when
@@ -47,11 +48,13 @@ void VPlanTransforms::introduceTopLevelVectorLoopRegion(
       vputils::getOrCreateVPValueForSCEVExpr(Plan, TripCount, SE));
 
   // Create VPRegionBlock, with existing header and new empty latch block, to be
-  // filled
+  // filled.
   VPBasicBlock *LatchVPBB = Plan.createVPBasicBlock("vector.latch");
   VPBlockUtils::insertBlockAfter(LatchVPBB, OriginalLatch);
   auto *TopRegion = Plan.createVPRegionBlock(
       HeaderVPBB, LatchVPBB, "vector loop", false /*isReplicator*/);
+  // All VPBB's reachable shallowly from HeaderVPBB belong to top level loop,
+  // because VPlan is expected to end at top level latch.
   for (VPBlockBase *VPBB : vp_depth_first_shallow(HeaderVPBB))
     VPBB->setParent(TopRegion);
 

llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp

@@ -382,7 +382,7 @@ void PlainCFGBuilder::buildPlainCFG(
     if (!isHeaderBB(BB, LoopForBB)) {
       setVPBBPredsFromBB(VPBB, BB);
     } else if (Region) {
-      // BB is a loop header and there's a corresponding region , set the
+      // BB is a loop header and there's a corresponding region, set the
       // predecessor for it.
       setRegionPredsFromBB(Region, BB);
     }

llvm/lib/Transforms/Vectorize/VPlanTransforms.h

@@ -55,7 +55,7 @@ struct VPlanTransforms {
   /// Introduce the top-level VPRegionBlock for the main loop in \p Plan. Coming
   /// into this function, \p Plan's top-level loop is modeled using a plain CFG.
   /// This transform wraps the plain CFG of the top-level loop within a
-  /// VPRegionBlock and creates a VPValue expressions for the original trip
+  /// VPRegionBlock and creates a VPValue expression for the original trip
   /// count. It will also introduce a dedicated VPBasicBlock for the vector
   /// pre-header as well a VPBasicBlock as exit block of the region
   /// (middle.block). If a check is needed to guard executing the scalar