!fixup address latest comments, thanks

Author: fhahn

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

@@ -2760,15 +2760,6 @@ LoopVectorizationCostModel::getVectorIntrinsicCost(CallInst *CI,
   return TTI.getIntrinsicInstrCost(CostAttrs, CostKind);
 }
 
-static VPBasicBlock *getHeaderForMainVectorLoop(VPlan &Plan,
-                                                VPDominatorTree &VPDT) {
-  return find_singleton<VPBasicBlock>(
-      vp_depth_first_shallow(Plan.getEntry()), [&VPDT](VPBlockBase *VPB, bool) {
-        auto *VPBB = dyn_cast<VPBasicBlock>(VPB);
-        return VPBB && VPBB->isHeader(VPDT) ? VPBB : nullptr;
-      });
-}
-
 void InnerLoopVectorizer::fixVectorizedLoop(VPTransformState &State) {
   // Fix widened non-induction PHIs by setting up the PHI operands.
   if (EnableVPlanNativePath)
@@ -2787,10 +2778,10 @@ void InnerLoopVectorizer::fixVectorizedLoop(VPTransformState &State) {
   PSE.getSE()->forgetLoop(OrigLoop);
   PSE.getSE()->forgetBlockAndLoopDispositions();
 
-  // Don't apply optimizations below when no vector loop remains, as they all
+  // Don't apply optimizations below when no (vector) loop remains, as they all
   // require one at the moment.
   VPBasicBlock *HeaderVPBB =
-      getHeaderForMainVectorLoop(*State.Plan, State.VPDT);
+      vputils::getTopLevelVectorLoopHeader(*State.Plan, State.VPDT);
   if (!HeaderVPBB)
     return;
 
@@ -7811,6 +7802,7 @@ DenseMap<const SCEV *, Value *> LoopVectorizationPlanner::executePlan(
 
   VPBasicBlock *MiddleVPBB =
       BestVPlan.getVectorLoopRegion() ? BestVPlan.getMiddleBlock() : nullptr;
+  VPlanTransforms::disolveLoopRegions(BestVPlan);
   VPlanTransforms::convertToConcreteRecipes(BestVPlan,
                                             *Legal->getWidestInductionType());
 
@@ -7906,7 +7898,8 @@ 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).
-  VPBasicBlock *HeaderVPBB = getHeaderForMainVectorLoop(BestVPlan, State.VPDT);
+  VPBasicBlock *HeaderVPBB =
+      vputils::getTopLevelVectorLoopHeader(BestVPlan, State.VPDT);
   if (HeaderVPBB) {
     MDNode *OrigLoopID = OrigLoop->getLoopID();
 

llvm/lib/Transforms/Vectorize/VPlan.cpp

@@ -207,9 +207,17 @@ VPBlockBase *VPBlockBase::getEnclosingBlockWithPredecessors() {
   return Parent->getEnclosingBlockWithPredecessors();
 }
 
-bool VPBasicBlock::isHeader(const VPDominatorTree &VPDT) const {
-  return getPredecessors().size() == 2 &&
-         VPDT.dominates(this, getPredecessors()[1]);
+bool VPBlockUtils::isHeader(const VPBlockBase *VPB,
+                            const VPDominatorTree &VPDT) {
+  auto *VPBB = dyn_cast<VPBasicBlock>(VPB);
+  if (!VPBB)
+    return false;
+  if (auto *R = VPBB->getParent())
+    return !R->isReplicator() && VPBB->getNumPredecessors() == 0;
+
+  assert(!VPB->getParent() && "checking blocks in regions not implemented yet");
+  return VPB->getPredecessors().size() == 2 &&
+         VPDT.dominates(VPB, VPB->getPredecessors()[1]);
 }
 
 VPBasicBlock::iterator VPBasicBlock::getFirstNonPhi() {
@@ -425,22 +433,23 @@ void VPBasicBlock::connectToPredecessors(VPTransformState &State) {
   if (ParentLoop && !State.LI->getLoopFor(NewBB))
     ParentLoop->addBasicBlockToLoop(NewBB, *State.LI);
 
+  auto Preds = to_vector(getHierarchicalPredecessors());
+  if (VPBlockUtils::isHeader(this, State.VPDT)) {
+    // There's no block yet for the latch, don't try to connect it yet.
+    Preds = {Preds[0]};
+  }
+
   // Hook up the new basic block to its predecessors.
-  for (VPBlockBase *PredVPBlock : getHierarchicalPredecessors()) {
+  for (VPBlockBase *PredVPBlock : Preds) {
     VPBasicBlock *PredVPBB = PredVPBlock->getExitingBasicBlock();
     auto &PredVPSuccessors = PredVPBB->getHierarchicalSuccessors();
     BasicBlock *PredBB = CFG.VPBB2IRBB.lookup(PredVPBB);
-    if (!PredBB)
-      continue;
-
     assert(PredBB && "Predecessor basic-block not found building successor.");
     auto *PredBBTerminator = PredBB->getTerminator();
     LLVM_DEBUG(dbgs() << "LV: draw edge from" << PredBB->getName() << '\n');
 
     auto *TermBr = dyn_cast<BranchInst>(PredBBTerminator);
     if (isa<UnreachableInst>(PredBBTerminator)) {
-      if (PredVPSuccessors.size() == 2)
-        continue;
       assert(PredVPSuccessors.size() == 1 &&
              "Predecessor ending w/o branch must have single successor.");
       DebugLoc DL = PredBBTerminator->getDebugLoc();
@@ -496,7 +505,7 @@ void VPBasicBlock::execute(VPTransformState *State) {
   bool Replica = bool(State->Lane);
   BasicBlock *NewBB = State->CFG.PrevBB; // Reuse it if possible.
 
-  if (isHeader(State->VPDT)) {
+  if (VPBlockUtils::isHeader(this, State->VPDT)) {
     // Create and register the new vector loop.
     Loop *PrevParentLoop = State->CurrentParentLoop;
     State->CurrentParentLoop = State->LI->AllocateLoop();
@@ -539,11 +548,8 @@ void VPBasicBlock::execute(VPTransformState *State) {
   executeRecipes(State, NewBB);
 
   // If this block is a latch, update CurrentParentLoop.
-  if (any_of(getSuccessors(), [State, this](VPBlockBase *Succ) {
-        auto *VPBB = dyn_cast<VPBasicBlock>(Succ);
-        return VPBB && VPBB->isHeader(State->VPDT) &&
-               State->VPDT.dominates(Succ, this);
-      }))
+  if (getNumSuccessors() == 2 &&
+      VPBlockUtils::isHeader(getSuccessors()[1], State->VPDT))
     State->CurrentParentLoop = State->CurrentParentLoop->getParentLoop();
 }
 
@@ -866,10 +872,11 @@ void VPRegionBlock::removeRegion() {
   VPBlockUtils::disconnectBlocks(this, Middle);
 
   for (VPBlockBase *VPB : vp_depth_first_shallow(Entry))
-    VPB->setParent(nullptr);
+    VPB->setParent(getParent());
 
   VPBlockUtils::connectBlocks(Preheader, Header);
   VPBlockUtils::connectBlocks(Exiting, Middle);
+  VPBlockUtils::connectBlocks(Exiting, Header);
 }
 
 VPlan::VPlan(Loop *L) {
@@ -981,57 +988,57 @@ void VPlan::execute(VPTransformState *State) {
   for (VPBlockBase *Block : RPOT)
     Block->execute(State);
 
+  VPBasicBlock *Header =
+      vputils::getTopLevelVectorLoopHeader(*this, State->VPDT);
+  if (!Header)
+    return;
+
+  auto *LatchVPBB = cast<VPBasicBlock>(Header->getPredecessors()[1]);
+  BasicBlock *VectorLatchBB = State->CFG.VPBB2IRBB[LatchVPBB];
+
   // Fix the latch value of canonical, reduction and first-order recurrences
   // phis in the vector loop.
-  for (VPBasicBlock *Header :
-       VPBlockUtils::blocksOnly<VPBasicBlock>(vp_depth_first_shallow(Entry))) {
-    if (!Header->isHeader(State->VPDT))
+  for (VPRecipeBase &R : Header->phis()) {
+    // Skip phi-like recipes that generate their backedege values themselves.
+    if (isa<VPWidenPHIRecipe>(&R))
       continue;
-    for (VPRecipeBase &R : Header->phis()) {
-      if (isa<VPWidenPHIRecipe>(&R))
-        continue;
 
-      auto *LatchVPBB = cast<VPBasicBlock>(Header->getPredecessors()[1]);
-      BasicBlock *VectorLatchBB = State->CFG.VPBB2IRBB[LatchVPBB];
-
-      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(
-            std::prev(VectorLatchBB->getTerminator()->getIterator()));
-
-        // 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;
+    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<VPSingleDefRecipe>(&R);
-      // VPInstructions currently model scalar Phis only.
-      bool NeedsScalar = isa<VPInstruction>(PhiR) ||
-                         (isa<VPReductionPHIRecipe>(PhiR) &&
-                          cast<VPReductionPHIRecipe>(PhiR)->isInLoop());
-
-      Value *Phi = State->get(PhiR, NeedsScalar);
-      // VPHeaderPHIRecipe supports getBackedgeValue() but VPInstruction does
-      // not.
-      Value *Val = State->get(PhiR->getOperand(1), 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(std::prev(VectorLatchBB->getTerminator()->getIterator()));
+
+      // 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;
     }
+
+    auto *PhiR = cast<VPSingleDefRecipe>(&R);
+    // VPInstructions currently model scalar Phis only.
+    bool NeedsScalar = isa<VPInstruction>(PhiR) ||
+                       (isa<VPReductionPHIRecipe>(PhiR) &&
+                        cast<VPReductionPHIRecipe>(PhiR)->isInLoop());
+
+    Value *Phi = State->get(PhiR, NeedsScalar);
+    // VPHeaderPHIRecipe supports getBackedgeValue() but VPInstruction does
+    // not.
+    Value *Val = State->get(PhiR->getOperand(1), NeedsScalar);
+    cast<PHINode>(Phi)->addIncoming(Val, VectorLatchBB);
   }
 }
 
@@ -1390,17 +1397,18 @@ void VPlanPrinter::dumpRegion(const VPRegionBlock *Region) {
 
 #endif
 
-bool VPValue::isDefinedOutsideLoopRegions() const {
+bool VPValue::isDefinedOutsideLoop() const {
   auto *DefR = getDefiningRecipe();
   if (!DefR)
     return true;
 
+  // For non-live-ins, check if is in a region only if the top-level loop region
+  // still exits.
   const VPBasicBlock *DefVPBB = DefR->getParent();
   auto *Plan = DefVPBB->getPlan();
-  if (Plan->getVectorLoopRegion())
-    return !DefR->getParent()->getEnclosingLoopRegion();
-  return DefVPBB == Plan->getEntry();
+  return Plan->getVectorLoopRegion() && !DefVPBB->getEnclosingLoopRegion();
 }
+
 void VPValue::replaceAllUsesWith(VPValue *New) {
   replaceUsesWithIf(New, [](VPUser &, unsigned) { return true; });
 }

llvm/lib/Transforms/Vectorize/VPlan.h

@@ -1589,9 +1589,7 @@ struct VPWidenSelectRecipe : public VPRecipeWithIRFlags, public VPIRMetadata {
     return getOperand(0);
   }
 
-  bool isInvariantCond() const {
-    return getCond()->isDefinedOutsideLoopRegions();
-  }
+  bool isInvariantCond() const { return getCond()->isDefinedOutsideLoop(); }
 
   /// Returns true if the recipe only uses the first lane of operand \p Op.
   bool onlyFirstLaneUsed(const VPValue *Op) const override {
@@ -1604,17 +1602,16 @@ struct VPWidenSelectRecipe : public VPRecipeWithIRFlags, public VPIRMetadata {
 /// A recipe for handling GEP instructions.
 class VPWidenGEPRecipe : public VPRecipeWithIRFlags {
   bool isPointerLoopInvariant() const {
-    return getOperand(0)->isDefinedOutsideLoopRegions();
+    return getOperand(0)->isDefinedOutsideLoop();
   }
 
   bool isIndexLoopInvariant(unsigned I) const {
-    return getOperand(I + 1)->isDefinedOutsideLoopRegions();
+    return getOperand(I + 1)->isDefinedOutsideLoop();
   }
 
   bool areAllOperandsInvariant() const {
-    return all_of(operands(), [](VPValue *Op) {
-      return Op->isDefinedOutsideLoopRegions();
-    });
+    return all_of(operands(),
+                  [](VPValue *Op) { return Op->isDefinedOutsideLoop(); });
   }
 
 public:
@@ -3415,9 +3412,6 @@ class VPBasicBlock : public VPBlockBase {
   /// second predecessor is the exiting block of the region.
   const VPBasicBlock *getCFGPredecessor(unsigned Idx) const;
 
-  /// Returns true if the block is a loop header in a plain-CFG VPlan.
-  bool isHeader(const VPDominatorTree &VPDT) const;
-
 protected:
   /// Execute the recipes in the IR basic block \p BB.
   void executeRecipes(VPTransformState *State, BasicBlock *BB);

llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp

@@ -576,8 +576,8 @@ Value *VPInstruction::generate(VPTransformState &State) {
   case VPInstruction::BranchOnCond: {
     Value *Cond = State.get(getOperand(0), VPLane(0));
     // Replace the temporary unreachable terminator with a new conditional
-    // branch, hooking it up to backward destination for exiting blocks now and
-    // to forward destination(s) later when they are created.
+    // branch, hooking it up to backward destination (header) for latch blocks
+    // now to forward destination(s) later when they are created.
     BranchInst *CondBr =
         Builder.CreateCondBr(Cond, Builder.GetInsertBlock(), nullptr);
     CondBr->setSuccessor(0, nullptr);
@@ -600,10 +600,10 @@ Value *VPInstruction::generate(VPTransformState &State) {
     VPBasicBlock *Header = cast<VPBasicBlock>(getParent()->getSuccessors()[1]);
 
     // Replace the temporary unreachable terminator with a new conditional
-    // branch, hooking it up to backward destination (the header) now and to the
-    // forward destination (the exit/middle block) later when it is created.
-    // Note that CreateCondBr expects a valid BB as first argument, so we need
-    // to set it to nullptr later.
+    // branch, hooking it up to backward destination (the header) for latch
+    // blocks now forward destination (the exit/middle block) later when it is
+    // created. Note that CreateCondBr expects a valid BB as first argument, so
+    // we need to set it to nullptr later.
     BranchInst *CondBr = Builder.CreateCondBr(Cond, Builder.GetInsertBlock(),
                                               State.CFG.VPBB2IRBB[Header]);
     CondBr->setSuccessor(0, nullptr);
@@ -1560,7 +1560,7 @@ void VPWidenSelectRecipe::execute(VPTransformState &State) {
 InstructionCost VPWidenSelectRecipe::computeCost(ElementCount VF,
                                                  VPCostContext &Ctx) const {
   SelectInst *SI = cast<SelectInst>(getUnderlyingValue());
-  bool ScalarCond = getOperand(0)->isDefinedOutsideLoopRegions();
+  bool ScalarCond = getOperand(0)->isDefinedOutsideLoop();
   Type *ScalarTy = Ctx.Types.inferScalarType(this);
   Type *VectorTy = toVectorTy(Ctx.Types.inferScalarType(this), VF);
 
@@ -1784,7 +1784,7 @@ InstructionCost VPWidenRecipe::computeCost(ElementCount VF,
       RHSInfo = Ctx.TTI.getOperandInfo(RHS->getLiveInIRValue());
 
     if (RHSInfo.Kind == TargetTransformInfo::OK_AnyValue &&
-        getOperand(1)->isDefinedOutsideLoopRegions())
+        getOperand(1)->isDefinedOutsideLoop())
       RHSInfo.Kind = TargetTransformInfo::OK_UniformValue;
     Type *VectorTy = toVectorTy(Ctx.Types.inferScalarType(this), VF);
     Instruction *CtxI = dyn_cast_or_null<Instruction>(getUnderlyingValue());
@@ -2634,13 +2634,12 @@ static void scalarizeInstruction(const Instruction *Instr,
   if (auto *II = dyn_cast<AssumeInst>(Cloned))
     State.AC->registerAssumption(II);
 
-  assert(
-      (RepRecipe->getParent()->getParent() ||
-       !RepRecipe->getParent()->getPlan()->getVectorLoopRegion() ||
-       all_of(RepRecipe->operands(),
-              [](VPValue *Op) { return Op->isDefinedOutsideLoopRegions(); })) &&
-      "Expected a recipe is either within a region or all of its operands "
-      "are defined outside the vectorized region.");
+  assert((RepRecipe->getParent()->getParent() ||
+          !RepRecipe->getParent()->getPlan()->getVectorLoopRegion() ||
+          all_of(RepRecipe->operands(),
+                 [](VPValue *Op) { return Op->isDefinedOutsideLoop(); })) &&
+         "Expected a recipe is either within a region or all of its operands "
+         "are defined outside the vectorized region.");
 }
 
 void VPReplicateRecipe::execute(VPTransformState &State) {