!fixup address latest comments, thanks

Author: fhahn

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

@@ -3012,6 +3012,8 @@ void InnerLoopVectorizer::fixVectorizedLoop(VPTransformState &State) {
                    getOrCreateVectorTripCount(nullptr), LoopMiddleBlock, State);
   }
 
+  // Don't apply optimizations below when no vector region remains, as they all
+  // require a vector loop at the moment.
   if (!State.Plan->getVectorLoopRegion())
     return;
 
@@ -7811,14 +7813,14 @@ DenseMap<const SCEV *, Value *> LoopVectorizationPlanner::executePlan(
   // 2.6. Maintain Loop Hints
   // Keep all loop hints from the original loop on the vector loop (we'll
   // replace the vectorizer-specific hints below).
-  MDNode *OrigLoopID = OrigLoop->getLoopID();
+  if (auto *LoopRegion = BestVPlan.getVectorLoopRegion()) {
+    MDNode *OrigLoopID = OrigLoop->getLoopID();
 
-  std::optional<MDNode *> VectorizedLoopID =
-      makeFollowupLoopID(OrigLoopID, {LLVMLoopVectorizeFollowupAll,
-                                      LLVMLoopVectorizeFollowupVectorized});
+    std::optional<MDNode *> VectorizedLoopID =
+        makeFollowupLoopID(OrigLoopID, {LLVMLoopVectorizeFollowupAll,
+                                        LLVMLoopVectorizeFollowupVectorized});
 
-  if (auto *R = BestVPlan.getVectorLoopRegion()) {
-    VPBasicBlock *HeaderVPBB = R->getEntryBasicBlock();
+    VPBasicBlock *HeaderVPBB = LoopRegion->getEntryBasicBlock();
     Loop *L = LI->getLoopFor(State.CFG.VPBB2IRBB[HeaderVPBB]);
     if (VectorizedLoopID) {
       L->setLoopID(*VectorizedLoopID);
@@ -7844,11 +7846,10 @@ DenseMap<const SCEV *, Value *> LoopVectorizationPlanner::executePlan(
   ILV.printDebugTracesAtEnd();
 
   // 4. Adjust branch weight of the branch in the middle block.
-  if (auto *R = BestVPlan.getVectorLoopRegion()) {
-    auto *ExitVPBB = cast<VPBasicBlock>(R->getSingleSuccessor());
-
+  if (BestVPlan.getVectorLoopRegion()) {
+    auto *MiddleVPBB = BestVPlan.getMiddleBlock();
     auto *MiddleTerm =
-        cast<BranchInst>(State.CFG.VPBB2IRBB[ExitVPBB]->getTerminator());
+        cast<BranchInst>(State.CFG.VPBB2IRBB[MiddleVPBB]->getTerminator());
     if (MiddleTerm->isConditional() &&
         hasBranchWeightMD(*OrigLoop->getLoopLatch()->getTerminator())) {
       // Assume that `Count % VectorTripCount` is equally distributed.

llvm/lib/Transforms/Vectorize/VPlan.cpp

@@ -554,6 +554,8 @@ VPBasicBlock *VPBasicBlock::splitAt(iterator SplitAt) {
 template <typename T> static T *getEnclosingLoopRegionForRegion(T *P) {
   if (P && P->isReplicator()) {
     P = P->getParent();
+    // Multiple loop regions can be nested, but replicate regions can only be
+    // nested inside a loop region or must be outside any other region.
     assert((!P || !cast<VPRegionBlock>(P)->isReplicator()) &&
            "unexpected nested replicate regions");
   }
@@ -933,6 +935,8 @@ void VPlan::prepareToExecute(Value *TripCountV, Value *VectorTripCountV,
 
   IRBuilder<> Builder(State.CFG.PrevBB->getTerminator());
   // FIXME: Model VF * UF computation completely in VPlan.
+  assert((!getVectorLoopRegion() || VFxUF.getNumUsers()) &&
+         "VFxUF expected to always have users");
   unsigned UF = getUF();
   if (VF.getNumUsers()) {
     Value *RuntimeVF = getRuntimeVF(Builder, TCTy, State.VF);
@@ -986,54 +990,56 @@ void VPlan::execute(VPTransformState *State) {
   for (VPBlockBase *Block : RPOT)
     Block->execute(State);
 
-  if (auto *LoopRegion = getVectorLoopRegion()) {
-    VPBasicBlock *LatchVPBB = LoopRegion->getExitingBasicBlock();
-    BasicBlock *VectorLatchBB = State->CFG.VPBB2IRBB[LatchVPBB];
+  State->CFG.DTU.flush();
 
-    // Fix the latch value of canonical, reduction and first-order recurrences
-    // phis in the vector loop.
-    VPBasicBlock *Header = LoopRegion->getEntryBasicBlock();
-    for (VPRecipeBase &R : Header->phis()) {
-      // Skip phi-like recipes that generate their backedege values themselves.
-      if (isa<VPWidenPHIRecipe>(&R))
-        continue;
+  auto *LoopRegion = getVectorLoopRegion();
+  if (!LoopRegion)
+    return;
 
-      if (isa<VPWidenInductionRecipe>(&R)) {
-        PHINode *Phi = nullptr;
-        if (isa<VPWidenIntOrFpInductionRecipe>(&R)) {
-          Phi = cast<PHINode>(State->get(R.getVPSingleValue()));
-        } else {
-          auto *WidenPhi = cast<VPWidenPointerInductionRecipe>(&R);
-          assert(!WidenPhi->onlyScalarsGenerated(State->VF.isScalable()) &&
-                 "recipe generating only scalars should have been replaced");
-          auto *GEP = cast<GetElementPtrInst>(State->get(WidenPhi));
-          Phi = cast<PHINode>(GEP->getPointerOperand());
-        }
-
-        Phi->setIncomingBlock(1, VectorLatchBB);
-
-        // Move the last step to the end of the latch block. This ensures
-        // consistent placement of all induction updates.
-        Instruction *Inc = cast<Instruction>(Phi->getIncomingValue(1));
-        Inc->moveBefore(VectorLatchBB->getTerminator()->getPrevNode());
-
-        // Use the steps for the last part as backedge value for the induction.
-        if (auto *IV = dyn_cast<VPWidenIntOrFpInductionRecipe>(&R))
-          Inc->setOperand(0, State->get(IV->getLastUnrolledPartOperand()));
-        continue;
+  VPBasicBlock *LatchVPBB = LoopRegion->getExitingBasicBlock();
+  BasicBlock *VectorLatchBB = State->CFG.VPBB2IRBB[LatchVPBB];
+
+  // Fix the latch value of canonical, reduction and first-order recurrences
+  // phis in the vector loop.
+  VPBasicBlock *Header = LoopRegion->getEntryBasicBlock();
+  for (VPRecipeBase &R : Header->phis()) {
+    // Skip phi-like recipes that generate their backedege values themselves.
+    if (isa<VPWidenPHIRecipe>(&R))
+      continue;
+
+    if (isa<VPWidenInductionRecipe>(&R)) {
+      PHINode *Phi = nullptr;
+      if (isa<VPWidenIntOrFpInductionRecipe>(&R)) {
+        Phi = cast<PHINode>(State->get(R.getVPSingleValue()));
+      } else {
+        auto *WidenPhi = cast<VPWidenPointerInductionRecipe>(&R);
+        assert(!WidenPhi->onlyScalarsGenerated(State->VF.isScalable()) &&
+               "recipe generating only scalars should have been replaced");
+        auto *GEP = cast<GetElementPtrInst>(State->get(WidenPhi));
+        Phi = cast<PHINode>(GEP->getPointerOperand());
       }
 
-      auto *PhiR = cast<VPHeaderPHIRecipe>(&R);
-      bool NeedsScalar = isa<VPScalarPHIRecipe>(PhiR) ||
-                         (isa<VPReductionPHIRecipe>(PhiR) &&
-                          cast<VPReductionPHIRecipe>(PhiR)->isInLoop());
-      Value *Phi = State->get(PhiR, NeedsScalar);
-      Value *Val = State->get(PhiR->getBackedgeValue(), NeedsScalar);
-      cast<PHINode>(Phi)->addIncoming(Val, VectorLatchBB);
+      Phi->setIncomingBlock(1, VectorLatchBB);
+
+      // Move the last step to the end of the latch block. This ensures
+      // consistent placement of all induction updates.
+      Instruction *Inc = cast<Instruction>(Phi->getIncomingValue(1));
+      Inc->moveBefore(VectorLatchBB->getTerminator()->getPrevNode());
+
+      // Use the steps for the last part as backedge value for the induction.
+      if (auto *IV = dyn_cast<VPWidenIntOrFpInductionRecipe>(&R))
+        Inc->setOperand(0, State->get(IV->getLastUnrolledPartOperand()));
+      continue;
     }
-  }
 
-  State->CFG.DTU.flush();
+    auto *PhiR = cast<VPHeaderPHIRecipe>(&R);
+    bool NeedsScalar = isa<VPScalarPHIRecipe>(PhiR) ||
+                       (isa<VPReductionPHIRecipe>(PhiR) &&
+                        cast<VPReductionPHIRecipe>(PhiR)->isInLoop());
+    Value *Phi = State->get(PhiR, NeedsScalar);
+    Value *Val = State->get(PhiR->getBackedgeValue(), NeedsScalar);
+    cast<PHINode>(Phi)->addIncoming(Val, VectorLatchBB);
+  }
 }
 
 InstructionCost VPlan::cost(ElementCount VF, VPCostContext &Ctx) {
@@ -1399,13 +1405,17 @@ void VPlanIngredient::print(raw_ostream &O) const {
 
 #endif
 
-bool VPValue::isDefinedOutsideLoopRegions() const {
-
-  return !hasDefiningRecipe() ||
-         (!getDefiningRecipe()->getParent()->getEnclosingLoopRegion() &&
-          getDefiningRecipe()->getParent()->getPlan()->getVectorLoopRegion());
+/// Returns true if there is a vector loop region and \p VPV is defined in a
+/// loop region.
+static bool isDefinedInsideLoopRegions(const VPValue *VPV) {
+  const VPRecipeBase *DefR = VPV->getDefiningRecipe();
+  return DefR && (!DefR->getParent()->getPlan()->getVectorLoopRegion() ||
+                  DefR->getParent()->getEnclosingLoopRegion());
 }
 
+bool VPValue::isDefinedOutsideLoopRegions() const {
+  return !isDefinedInsideLoopRegions(this);
+}
 void VPValue::replaceAllUsesWith(VPValue *New) {
   replaceUsesWithIf(New, [](VPUser &, unsigned) { return true; });
 }

llvm/lib/Transforms/Vectorize/VPlan.h

@@ -3809,6 +3809,8 @@ class VPlan {
 
   ~VPlan();
 
+  /// Set \p EntryBlock as the entry VPBlockBase of this VPRegionBlock. \p
+  /// EntryBlock must have no predecessors.
   void setEntry(VPBasicBlock *VPBB) {
     Entry = VPBB;
     VPBB->setPlan(this);
@@ -3842,7 +3844,8 @@ class VPlan {
   VPBasicBlock *getEntry() { return Entry; }
   const VPBasicBlock *getEntry() const { return Entry; }
 
-  /// Returns the preheader of the vector loop region.
+  /// Returns the preheader of the vector loop region, if one exists, or null
+  /// otherwise.
   VPBasicBlock *getVectorPreheader() {
     VPRegionBlock *VectorRegion = getVectorLoopRegion();
     return VectorRegion

llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp

@@ -2376,9 +2376,7 @@ void VPBranchOnMaskRecipe::execute(VPTransformState &State) {
   // Replace the temporary unreachable terminator with a new conditional branch,
   // whose two destinations will be set later when they are created.
   auto *CurrentTerminator = State.CFG.PrevBB->getTerminator();
-  assert((isa<UnreachableInst>(CurrentTerminator) ||
-          (isa<BranchInst>(CurrentTerminator) &&
-           !CurrentTerminator->getOperand(0))) &&
+  assert(isa<UnreachableInst>(CurrentTerminator) &&
          "Expected to replace unreachable terminator with conditional branch.");
   auto *CondBr = BranchInst::Create(State.CFG.PrevBB, nullptr, ConditionBit);
   CondBr->setSuccessor(0, nullptr);

llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp

@@ -794,15 +794,12 @@ static void simplifyRecipe(VPRecipeBase &R, VPTypeAnalysis &TypeInfo) {
     return R.getVPSingleValue()->replaceAllUsesWith(R.getOperand(1));
 }
 
-/// Try to simplify the recipes in \p Plan. If \p CanonicalIVTy is not nullptr,
-/// use it directly instead of retrieving the canonical IV type from the plan
-/// which may not exist any longer.
-static void simplifyRecipes(VPlan &Plan, Type *CanonicalIVTy = nullptr) {
+/// Try to simplify the recipes in \p Plan. Use \p CanonicalIVTy as type for all
+/// un-typed live-ins in VPTypeAnalysis.
+static void simplifyRecipes(VPlan &Plan, Type *CanonicalIVTy) {
   ReversePostOrderTraversal<VPBlockDeepTraversalWrapper<VPBlockBase *>> RPOT(
       Plan.getEntry());
-  Type *CanonicalIVType =
-      CanonicalIVTy ? CanonicalIVTy : Plan.getCanonicalIV()->getScalarType();
-  VPTypeAnalysis TypeInfo(CanonicalIVType);
+  VPTypeAnalysis TypeInfo(CanonicalIVTy);
   for (VPBasicBlock *VPBB : VPBlockUtils::blocksOnly<VPBasicBlock>(RPOT)) {
     for (VPRecipeBase &R : make_early_inc_range(*VPBB)) {
       simplifyRecipe(R, TypeInfo);
@@ -840,38 +837,40 @@ void VPlanTransforms::optimizeForVFAndUF(VPlan &Plan, ElementCount BestVF,
       !SE.isKnownPredicate(CmpInst::ICMP_ULE, TripCount, C))
     return;
 
+  // The vector loop region only executes once. If possible, completely remove
+  // the region, otherwise replace the terminator controlling the latch with
+  // (BranchOnCond true).
   Term->eraseFromParent();
   auto *Header = cast<VPBasicBlock>(VectorRegion->getEntry());
   auto *CanIVTy = Plan.getCanonicalIV()->getScalarType();
-  if (any_of(Header->phis(),
-             IsaPred<VPWidenIntOrFpInductionRecipe, VPReductionPHIRecipe>)) {
-    LLVMContext &Ctx = SE.getContext();
-    auto *BOC = new VPInstruction(
-        VPInstruction::BranchOnCond,
-        {Plan.getOrAddLiveIn(ConstantInt::getTrue(Ctx))}, Term->getDebugLoc());
-    ExitingVPBB->appendRecipe(BOC);
-  } else {
-    for (VPRecipeBase &R : make_early_inc_range(Header->phis())) {
-      auto *P = cast<VPHeaderPHIRecipe>(&R);
-      P->replaceAllUsesWith(P->getStartValue());
-      P->eraseFromParent();
+  if (all_of(
+          Header->phis(),
+          IsaPred<VPCanonicalIVPHIRecipe, VPFirstOrderRecurrencePHIRecipe>)) {
+    for (VPRecipeBase &HeaderR : make_early_inc_range(Header->phis())) {
+      auto *HeaderPhiR = cast<VPHeaderPHIRecipe>(&HeaderR);
+      HeaderPhiR->replaceAllUsesWith(HeaderPhiR->getStartValue());
+      HeaderPhiR->eraseFromParent();
     }
 
-    VPBlockBase *Preheader = Plan.getVectorPreheader();
-    VPBlockBase *Middle = Plan.getMiddleBlock();
+    VPBlockBase *Preheader = VectorRegion->getSinglePredecessor();
+    VPBlockBase *Exit = VectorRegion->getSingleSuccessor();
     VPBlockUtils::disconnectBlocks(Preheader, VectorRegion);
-    VPBlockUtils::disconnectBlocks(VectorRegion, Middle);
+    VPBlockUtils::disconnectBlocks(VectorRegion, Exit);
 
-    Header->setParent(nullptr);
-    ExitingVPBB->setParent(nullptr);
+    for (VPBlockBase *B : vp_depth_first_shallow(VectorRegion->getEntry()))
+      B->setParent(nullptr);
 
-    for (VPBlockBase *B : vp_depth_first_shallow(VectorRegion->getEntry())) {
-      if (isa<VPRegionBlock>(B))
-        B->setParent(nullptr);
-    }
     VPBlockUtils::connectBlocks(Preheader, Header);
-    VPBlockUtils::connectBlocks(ExitingVPBB, Middle);
+    VPBlockUtils::connectBlocks(ExitingVPBB, Exit);
     simplifyRecipes(Plan, CanIVTy);
+  } else {
+    // The vector region contains header phis for which we cannot remove the
+    // loop region yet.
+    LLVMContext &Ctx = SE.getContext();
+    auto *BOC = new VPInstruction(
+        VPInstruction::BranchOnCond,
+        {Plan.getOrAddLiveIn(ConstantInt::getTrue(Ctx))}, Term->getDebugLoc());
+    ExitingVPBB->appendRecipe(BOC);
   }
 
   VPlanTransforms::removeDeadRecipes(Plan);
@@ -1287,10 +1286,10 @@ void VPlanTransforms::optimize(VPlan &Plan) {
   removeRedundantCanonicalIVs(Plan);
   removeRedundantInductionCasts(Plan);
 
-  simplifyRecipes(Plan);
+  simplifyRecipes(Plan, Plan.getCanonicalIV()->getScalarType());
   legalizeAndOptimizeInductions(Plan);
   removeRedundantExpandSCEVRecipes(Plan);
-  simplifyRecipes(Plan);
+  simplifyRecipes(Plan, Plan.getCanonicalIV()->getScalarType());
   removeDeadRecipes(Plan);
 
   createAndOptimizeReplicateRegions(Plan);