!fixup address latest comments, thanks

Author: fhahn

llvm/lib/Transforms/Vectorize/LoopVectorizationPlanner.h

@@ -334,7 +334,7 @@ class VPBuilder {
         FPBinOp ? FPBinOp->getFastMathFlags() : FastMathFlags(), DL));
   }
 
-  VPExpandSCEVRecipe *expandSCEV(const SCEV *Expr, ScalarEvolution &SE) {
+  VPExpandSCEVRecipe *createExpandSCEV(const SCEV *Expr, ScalarEvolution &SE) {
     return tryInsertInstruction(new VPExpandSCEVRecipe(Expr, SE));
   }
 
@@ -563,7 +563,8 @@ class LoopVectorizationPlanner {
   /// Emit remarks for recipes with invalid costs in the available VPlans.
   void emitInvalidCostRemarks(OptimizationRemarkEmitter *ORE);
 
-  /// Create a check to \p Plan to see if the vector loop should be executed.
+  /// Create a check to \p Plan to see if the vector loop should be executed
+  /// based on its trip count.
   void addMinimumIterationCheck(VPlan &Plan, ElementCount VF, unsigned UF,
                                 ElementCount MinProfitableTripCount) const;
 

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

@@ -544,7 +544,8 @@ class InnerLoopVectorizer {
 protected:
   friend class LoopVectorizationPlanner;
 
-  /// Create a new IR basic block for the scalar preheader.
+  /// Create a new IR basic block for the scalar preheader whose name is
+  /// prefixed with \p Prefix.
   void createScalarPreheader(StringRef Prefix);
 
   /// Allow subclasses to override and print debug traces before/after vplan
@@ -669,17 +670,6 @@ class InnerLoopAndEpilogueVectorizer : public InnerLoopVectorizer {
                             UnrollFactor, CM, BFI, PSI, Checks, Plan),
         EPI(EPI), MinProfitableTripCount(MinProfitableTripCount) {}
 
-  // Override this function to handle the more complex control flow around the
-  // three loops.
-  BasicBlock *createVectorizedLoopSkeleton() final {
-    return createEpilogueVectorizedLoopSkeleton();
-  }
-
-  /// The interface for creating a vectorized skeleton using one of two
-  /// different strategies, each corresponding to one execution of the vplan
-  /// as described above.
-  virtual BasicBlock *createEpilogueVectorizedLoopSkeleton() = 0;
-
   /// Holds and updates state information required to vectorize the main loop
   /// and its epilogue in two separate passes. This setup helps us avoid
   /// regenerating and recomputing runtime safety checks. It also helps us to
@@ -710,10 +700,10 @@ class EpilogueVectorizerMainLoop : public InnerLoopAndEpilogueVectorizer {
                                        EPI.MainLoopVF, EPI.MainLoopUF) {}
   /// Implements the interface for creating a vectorized skeleton using the
   /// *main loop* strategy (ie the first pass of vplan execution).
-  BasicBlock *createEpilogueVectorizedLoopSkeleton() final;
+  BasicBlock *createVectorizedLoopSkeleton() final;
 
 protected:
-  // Create a check to see if the vector loop should be executed
+  // Create a check to see if the main vector loop should be executed
   Value *createIterationCountCheck(ElementCount VF, unsigned UF) const;
 
   /// Emits an iteration count bypass check once for the main loop (when \p
@@ -742,7 +732,7 @@ class EpilogueVectorizerEpilogueLoop : public InnerLoopAndEpilogueVectorizer {
   }
   /// Implements the interface for creating a vectorized skeleton using the
   /// *epilogue loop* strategy (ie the second pass of vplan execution).
-  BasicBlock *createEpilogueVectorizedLoopSkeleton() final;
+  BasicBlock *createVectorizedLoopSkeleton() final;
 
 protected:
   /// Emits an iteration count bypass check after the main vector loop has
@@ -7268,7 +7258,7 @@ DenseMap<const SCEV *, Value *> LoopVectorizationPlanner::executePlan(
   // at the wrong place.
   for (VPRecipeBase &R : make_early_inc_range(*Entry)) {
     if (!isa<VPIRInstruction, VPIRPhi>(&R))
-      continue;
+      break;
     R.eraseFromParent();
   }
   for (Instruction &I : drop_begin(reverse(*Entry->getIRBasicBlock())))
@@ -7415,7 +7405,7 @@ DenseMap<const SCEV *, Value *> LoopVectorizationPlanner::executePlan(
 
 /// This function is partially responsible for generating the control flow
 /// depicted in https://llvm.org/docs/Vectorizers.html#epilogue-vectorization.
-BasicBlock *EpilogueVectorizerMainLoop::createEpilogueVectorizedLoopSkeleton() {
+BasicBlock *EpilogueVectorizerMainLoop::createVectorizedLoopSkeleton() {
   createScalarPreheader("");
 
   // Generate the code to check the minimum iteration count of the vector
@@ -7501,8 +7491,7 @@ EpilogueVectorizerMainLoop::emitIterationCountCheck(BasicBlock *Bypass,
 
 /// This function is partially responsible for generating the control flow
 /// depicted in https://llvm.org/docs/Vectorizers.html#epilogue-vectorization.
-BasicBlock *
-EpilogueVectorizerEpilogueLoop::createEpilogueVectorizedLoopSkeleton() {
+BasicBlock *EpilogueVectorizerEpilogueLoop::createVectorizedLoopSkeleton() {
   createScalarPreheader("vec.epilog.");
 
   // Now, compare the remaining count and if there aren't enough iterations to
@@ -9315,18 +9304,19 @@ void LoopVectorizationPlanner::addMinimumIterationCheck(
   // vscale is not necessarily a power-of-2, which means we cannot guarantee
   // an overflow to zero when updating induction variables and so an
   // additional overflow check is required before entering the vector loop.
-  bool CheckNeededWithTailFolding =
+  bool IsIndvarOverflowCheckNeededForVF =
       VF.isScalable() && !TTI.isVScaleKnownToBeAPowerOfTwo() &&
-      !isIndvarOverflowCheckKnownFalse(&CM, VF, 1) &&
+      !isIndvarOverflowCheckKnownFalse(&CM, VF, UF) &&
       CM.getTailFoldingStyle() !=
           TailFoldingStyle::DataAndControlFlowWithoutRuntimeCheck;
+  const uint32_t *BranchWeigths =
+      hasBranchWeightMD(*OrigLoop->getLoopLatch()->getTerminator())
+          ? &MinItersBypassWeights[0]
+          : nullptr;
   VPlanTransforms::addMinimumIterationCheck(
       Plan, VF, UF, MinProfitableTripCount,
       CM.requiresScalarEpilogue(VF.isVector()), CM.foldTailByMasking(),
-      CheckNeededWithTailFolding, OrigLoop,
-      hasBranchWeightMD(*OrigLoop->getLoopLatch()->getTerminator())
-          ? &MinItersBypassWeights[0]
-          : nullptr,
+      IsIndvarOverflowCheckNeededForVF, OrigLoop, BranchWeigths,
       OrigLoop->getLoopPredecessor()->getTerminator()->getDebugLoc(),
       *PSE.getSE());
 }

llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp

@@ -680,62 +680,70 @@ void VPlanTransforms::addMinimumIterationCheck(
   // vector trip count is zero. This check also covers the case where adding one
   // to the backedge-taken count overflowed leading to an incorrect trip count
   // of zero. In this case we will also jump to the scalar loop.
-  auto P = RequiresScalarEpilogue ? ICmpInst::ICMP_ULE : ICmpInst::ICMP_ULT;
+  CmpInst::Predicate CmpPred =
+      RequiresScalarEpilogue ? ICmpInst::ICMP_ULE : ICmpInst::ICMP_ULT;
   // If tail is to be folded, vector loop takes care of all iterations.
-  const SCEV *Count = vputils::getSCEVExprForVPValue(Plan.getTripCount(), SE);
-  Type *CountTy = Count->getType();
-  auto CreateStep = [&]() -> const SCEV * {
-    const SCEV *VFxUF = SE.getElementCount(CountTy, (VF * UF), SCEV::FlagNUW);
-    // Create step with max(MinProTripCount, UF * VF).
-    if (UF * VF.getKnownMinValue() >= MinProfitableTripCount.getKnownMinValue())
+  VPValue *TripCountVPV = Plan.getTripCount();
+  const SCEV *TripCount = vputils::getSCEVExprForVPValue(TripCountVPV, SE);
+  Type *TripCountTy = TripCount->getType();
+  auto CreateMinTripCount = [&]() -> const SCEV * {
+    // Create or get max(MinProfitableTripCount, UF * VF) and return it.
+    const SCEV *VFxUF =
+        SE.getElementCount(TripCountTy, (VF * UF), SCEV::FlagNUW);
+    const SCEV *MinProfitableTripCountSCEV =
+        SE.getElementCount(TripCountTy, MinProfitableTripCount, SCEV::FlagNUW);
+    const SCEV *Max = SE.getUMaxExpr(MinProfitableTripCountSCEV, VFxUF);
+    if (!VF.isScalable())
+      return Max;
+
+    if (UF * VF.getKnownMinValue() >=
+        MinProfitableTripCount.getKnownMinValue()) {
+      // TODO: SCEV should be able to simplify test.
       return VFxUF;
+    }
 
-    const SCEV *MinProfTC =
-        SE.getElementCount(CountTy, MinProfitableTripCount, SCEV::FlagNUW);
-    if (!VF.isScalable())
-      return MinProfTC;
-    return SE.getUMaxExpr(MinProfTC, VFxUF);
+    return Max;
   };
 
   VPBasicBlock *EntryVPBB = Plan.getEntry();
   VPBuilder Builder(EntryVPBB);
-  VPValue *CheckMinIters = Plan.getFalse();
-  const SCEV *Step = CreateStep();
+  VPValue *TripCountCheck = Plan.getFalse();
+  const SCEV *Step = CreateMinTripCount();
   if (!TailFolded) {
     // TODO: Emit unconditional branch to vector preheader instead of
     // conditional branch with known condition.
-    const SCEV *TripCountSCEV = SE.applyLoopGuards(Count, OrigLoop);
+    TripCount = SE.applyLoopGuards(TripCount, OrigLoop);
     // Check if the trip count is < the step.
-    if (SE.isKnownPredicate(P, TripCountSCEV, Step)) {
+    if (SE.isKnownPredicate(CmpPred, TripCount, Step)) {
       // TODO: Ensure step is at most the trip count when determining max VF and
       // UF, w/o tail folding.
-      CheckMinIters = Plan.getTrue();
-    } else if (!SE.isKnownPredicate(CmpInst::getInversePredicate(P),
-                                    TripCountSCEV, Step)) {
+      TripCountCheck = Plan.getTrue();
+    } else if (!SE.isKnownPredicate(CmpInst::getInversePredicate(CmpPred),
+                                    TripCount, Step)) {
       // Generate the minimum iteration check only if we cannot prove the
       // check is known to be true, or known to be false.
-      CheckMinIters = Builder.createICmp(P, Plan.getTripCount(),
-                                         Builder.expandSCEV(Step, SE), DL,
-                                         "min.iters.check");
-    } // else step known to be < trip count, use CheckMinIters preset to false.
+      VPValue *MinTripCountVPV = Builder.createExpandSCEV(Step, SE);
+      TripCountCheck = Builder.createICmp(
+          CmpPred, TripCountVPV, MinTripCountVPV, DL, "min.iters.check");
+    } // else step known to be < trip count, use TripCountCheck preset to false.
   } else if (CheckNeededWithTailFolding) {
     // vscale is not necessarily a power-of-2, which means we cannot guarantee
     // an overflow to zero when updating induction variables and so an
     // additional overflow check is required before entering the vector loop.
 
     // Get the maximum unsigned value for the type.
-    VPValue *MaxUIntTripCount = Plan.getOrAddLiveIn(
-        ConstantInt::get(CountTy, cast<IntegerType>(CountTy)->getMask()));
-    VPValue *LHS = Builder.createNaryOp(Instruction::Sub,
-                                        {MaxUIntTripCount, Plan.getTripCount()},
-                                        DebugLoc::getUnknown());
+    VPValue *MaxUIntTripCount = Plan.getOrAddLiveIn(ConstantInt::get(
+        TripCountTy, cast<IntegerType>(TripCountTy)->getMask()));
+    VPValue *DistanceToMax =
+        Builder.createNaryOp(Instruction::Sub, {MaxUIntTripCount, TripCountVPV},
+                             DebugLoc::getUnknown());
 
     // Don't execute the vector loop if (UMax - n) < (VF * UF).
-    CheckMinIters = Builder.createICmp(ICmpInst::ICMP_ULT, LHS,
-                                       Builder.expandSCEV(Step, SE), DL);
+    TripCountCheck = Builder.createICmp(ICmpInst::ICMP_ULT, DistanceToMax,
+                                        Builder.createExpandSCEV(Step, SE), DL);
   }
   VPInstruction *Term =
-      Builder.createNaryOp(VPInstruction::BranchOnCond, {CheckMinIters}, DL);
+      Builder.createNaryOp(VPInstruction::BranchOnCond, {TripCountCheck}, DL);
   if (MinItersBypassWeights) {
     MDBuilder MDB(Plan.getContext());
     MDNode *BranchWeights = MDB.createBranchWeights(