!fixup address latest comments, thanks

Author: fhahn

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

@@ -2883,8 +2883,11 @@ void InnerLoopVectorizer::fixNonInductionPHIs(VPTransformState &State) {
       PHINode *NewPhi = cast<PHINode>(State.get(VPPhi));
       // Make sure the builder has a valid insert point.
       Builder.SetInsertPoint(NewPhi);
-      for (const auto &[Inc, VPBB] : VPPhi->incoming_values_and_blocks())
+      for (unsigned Idx = 0; Idx < VPPhi->getNumIncomingValues(); ++Idx) {
+        VPValue *Inc = VPPhi->getIncomingValue(Idx);
+        const VPBasicBlock *VPBB = VPPhi->getIncomingBlock(Idx);
         NewPhi->addIncoming(State.get(Inc), State.CFG.VPBB2IRBB[VPBB]);
+      }
     }
   }
 }

llvm/lib/Transforms/Vectorize/VPlan.cpp

@@ -790,6 +790,26 @@ InstructionCost VPBasicBlock::cost(ElementCount VF, VPCostContext &Ctx) {
   return Cost;
 }
 
+const VPBasicBlock *VPBasicBlock::getCFGPredecessor(unsigned Idx) const {
+  const VPBlockBase *Pred = nullptr;
+  if (getNumPredecessors() > 0) {
+    Pred = getPredecessors()[Idx];
+  } else {
+    auto *Region = getParent();
+    assert(Region && !Region->isReplicator() && Region->getEntry() == this &&
+           "must be in the entry block of a non-replicate region");
+    assert(
+        Idx < 2 && Region->getNumPredecessors() == 1 &&
+        "when placed in an entry block, only 2 incoming blocks are available");
+
+    // Idx ==  0 selects the predecessor of the region, Idx == 1 selects the
+    // region itself whose exiting block feeds the phi across the backedge.
+    Pred = Idx == 0 ? Region->getSinglePredecessor() : Region;
+  }
+
+  return Pred->getExitingBasicBlock();
+}
+
 InstructionCost VPRegionBlock::cost(ElementCount VF, VPCostContext &Ctx) {
   if (!isReplicator()) {
     InstructionCost Cost = 0;

llvm/lib/Transforms/Vectorize/VPlan.h

@@ -1180,42 +1180,13 @@ template <typename RecipeTy> class VPPhiAccessors {
     return getAsRecipe()->getOperand(Idx);
   }
 
-  /// Returns an interator range over the incoming values
-  VPUser::const_operand_range incoming_values() const {
-    return getAsRecipe()->operands();
-  }
-
   /// Returns the incoming block with index \p Idx.
-  const VPBasicBlock *getIncomingBlock(unsigned Idx) const;
-
-  using const_incoming_block_iterator =
-      mapped_iterator<detail::index_iterator,
-                      std::function<const VPBasicBlock *(size_t)>>;
-  using const_incoming_blocks_range =
-      iterator_range<const_incoming_block_iterator>;
-
-  const_incoming_block_iterator incoming_block_begin() const {
-    return const_incoming_block_iterator(
-        detail::index_iterator(0),
-        [this](size_t Idx) { return getIncomingBlock(Idx); });
-  }
-  const_incoming_block_iterator incoming_block_end() const {
-    return const_incoming_block_iterator(
-        detail::index_iterator(getAsRecipe()->getNumOperands()),
-        [this](size_t Idx) { return getIncomingBlock(Idx); });
+  const VPBasicBlock *getIncomingBlock(unsigned Idx) const {
+    return getAsRecipe()->getParent()->getCFGPredecessor(Idx);
   }
 
-  /// Returns an iterator range over the incoming blocks.
-  const_incoming_blocks_range incoming_blocks() const {
-    return make_range(incoming_block_begin(), incoming_block_end());
-  }
-
-  /// Returns an iterator range over pairs of incoming values and corresponding
-  /// incoming blocks.
-  detail::zippy<llvm::detail::zip_shortest, VPUser::const_operand_range,
-                const_incoming_blocks_range>
-  incoming_values_and_blocks() const {
-    return zip(incoming_values(), incoming_blocks());
+  unsigned getNumIncomingValues() const {
+    return getAsRecipe()->getNumOperands();
   }
 };
 
@@ -3372,6 +3343,12 @@ class VPBasicBlock : public VPBlockBase {
   /// the cloned recipes.
   VPBasicBlock *clone() override;
 
+  /// Returns the predecessor block at index \p Idx with the predecessors as per
+  /// the corresponding plain CFG. If the block is an entry block to a region,
+  /// the first predecessor is the single predecessor of a region, and the
+  /// second predecessor is the exiting block of the region.
+  const VPBasicBlock *getCFGPredecessor(unsigned Idx) const;
+
 protected:
   /// Execute the recipes in the IR basic block \p BB.
   void executeRecipes(VPTransformState *State, BasicBlock *BB);

llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp

@@ -1205,32 +1205,6 @@ void VPIRPhi::print(raw_ostream &O, const Twine &Indent,
 }
 #endif
 
-/// Returns the incoming block at index \p Idx for \p R. This handles both
-/// recipes placed in entry blocks of loop regions (incoming blocks are the
-/// region's predecessor and the region's exit) and other locations (incoming
-/// blocks are the direct predecessors).
-static const VPBasicBlock *getIncomingBlockForRecipe(const VPRecipeBase *R,
-                                                     unsigned Idx) {
-  const VPBasicBlock *Parent = R->getParent();
-  const VPBlockBase *Pred = nullptr;
-  if (Parent->getNumPredecessors() > 0) {
-    Pred = Parent->getPredecessors()[Idx];
-  } else {
-    auto *Region = Parent->getParent();
-    assert(Region && !Region->isReplicator() && Region->getEntry() == Parent &&
-           "must be in the entry block of a non-replicate region");
-    assert(
-        Idx < 2 && R->getNumOperands() == 2 &&
-        "when placed in an entry block, only 2 incoming blocks are available");
-
-    // Idx ==  0 selects the predecessor of the region, Idx == 1 selects the
-    // region itself whose exiting block feeds the phi across the backedge.
-    Pred = Idx == 0 ? Region->getSinglePredecessor() : Region;
-  }
-
-  return Pred->getExitingBasicBlock();
-}
-
 void VPIRMetadata::applyMetadata(Instruction &I) const {
   for (const auto &[Kind, Node] : Metadata)
     I.setMetadata(Kind, Node);
@@ -3712,12 +3686,6 @@ void VPReductionPHIRecipe::print(raw_ostream &O, const Twine &Indent,
 }
 #endif
 
-template <>
-const VPBasicBlock *
-VPPhiAccessors<VPWidenPHIRecipe>::getIncomingBlock(unsigned Idx) const {
-  return getIncomingBlockForRecipe(getAsRecipe(), Idx);
-}
-
 void VPWidenPHIRecipe::execute(VPTransformState &State) {
   assert(EnableVPlanNativePath &&
          "Non-native vplans are not expected to have VPWidenPHIRecipes.");