[llvm] [VPlan] Use VPlan predecessors in VPWidenPHIRecipe (NFC). (PR #126388)

via llvm-commits llvm-commits at lists.llvm.org
Sat Feb 8 13:14:14 PST 2025


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-vectorizers

Author: Florian Hahn (fhahn)

<details>
<summary>Changes</summary>

Update VPWidenPHIRecipe to use the predecessors in VPlan to determine the incoming blocks instead of tracking them separately. This brings VPWidenPHIrecipe in line with the other phi recipes.

---
Full diff: https://github.com/llvm/llvm-project/pull/126388.diff


6 Files Affected:

- (modified) llvm/lib/Transforms/Vectorize/VPlan.h (+6-18) 
- (modified) llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp (+12-9) 
- (modified) llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp (+15) 
- (modified) llvm/lib/Transforms/Vectorize/VPlanUtils.h (+1-9) 
- (modified) llvm/test/Transforms/LoopVectorize/outer-loop-wide-phis.ll (+1-1) 
- (modified) llvm/unittests/Transforms/Vectorize/VPlanTest.cpp (+2-2) 


``````````diff
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h
index 3816e1b61576a3d..f2f118f5d2980c1 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.h
+++ b/llvm/lib/Transforms/Vectorize/VPlan.h
@@ -1958,13 +1958,12 @@ class VPScalarPHIRecipe : public VPHeaderPHIRecipe {
 #endif
 };
 
-/// A recipe for handling phis that are widened in the vector loop.
-/// In the VPlan native path, all incoming VPValues & VPBasicBlock pairs are
-/// managed in the recipe directly.
+/// A recipe for widened phis. Incoming values are operands of the recipe and
+/// their operand index corresponds to the incoming predeocessor block. If the
+/// recipe is placed in an entry block to a (non-replicate) region, it must have
+/// exactly 2 incoming values, from from the predecessors of the region and one
+/// from the exiting block of the region.
 class VPWidenPHIRecipe : public VPSingleDefRecipe {
-  /// List of incoming blocks. Only used in the VPlan native path.
-  SmallVector<VPBasicBlock *, 2> IncomingBlocks;
-
 public:
   /// Create a new VPWidenPHIRecipe for \p Phi with start value \p Start and
   /// debug location \p DL.
@@ -1991,19 +1990,8 @@ class VPWidenPHIRecipe : public VPSingleDefRecipe {
              VPSlotTracker &SlotTracker) const override;
 #endif
 
-  /// Adds a pair (\p IncomingV, \p IncomingBlock) to the phi.
-  void addIncoming(VPValue *IncomingV, VPBasicBlock *IncomingBlock) {
-    addOperand(IncomingV);
-    IncomingBlocks.push_back(IncomingBlock);
-  }
-
   /// Returns the \p I th incoming VPBasicBlock.
-  VPBasicBlock *getIncomingBlock(unsigned I) { return IncomingBlocks[I]; }
-
-  /// Set the \p I th incoming VPBasicBlock to \p IncomingBlock.
-  void setIncomingBlock(unsigned I, VPBasicBlock *IncomingBlock) {
-    IncomingBlocks[I] = IncomingBlock;
-  }
+  VPBasicBlock *getIncomingBlock(unsigned I);
 
   /// Returns the \p I th incoming VPValue.
   VPValue *getIncomingValue(unsigned I) { return getOperand(I); }
diff --git a/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp b/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp
index 5a2e5d7cfee48d0..c94ea35e8112e25 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp
@@ -136,19 +136,22 @@ void PlainCFGBuilder::fixPhiNodes() {
       // predecessor is the first operand of the recipe.
       assert(Phi->getNumOperands() == 2);
       BasicBlock *LoopPred = L->getLoopPredecessor();
-      VPPhi->addIncoming(
-          getOrCreateVPOperand(Phi->getIncomingValueForBlock(LoopPred)),
-          BB2VPBB[LoopPred]);
+      VPPhi->addOperand(
+          getOrCreateVPOperand(Phi->getIncomingValueForBlock(LoopPred)));
       BasicBlock *LoopLatch = L->getLoopLatch();
-      VPPhi->addIncoming(
-          getOrCreateVPOperand(Phi->getIncomingValueForBlock(LoopLatch)),
-          BB2VPBB[LoopLatch]);
+      VPPhi->addOperand(
+          getOrCreateVPOperand(Phi->getIncomingValueForBlock(LoopLatch)));
       continue;
     }
 
-    for (unsigned I = 0; I != Phi->getNumOperands(); ++I)
-      VPPhi->addIncoming(getOrCreateVPOperand(Phi->getIncomingValue(I)),
-                         BB2VPBB[Phi->getIncomingBlock(I)]);
+    // Add operands for VPPhi in the order matching its predecessors in VPlan.
+    DenseMap<const VPBasicBlock *, VPValue *> IncomingValues;
+    for (unsigned I = 0; I != Phi->getNumOperands(); ++I) {
+      IncomingValues[BB2VPBB[Phi->getIncomingBlock(I)]] =
+          getOrCreateVPOperand(Phi->getIncomingValue(I));
+    }
+    for (VPBlockBase *Pred : VPPhi->getParent()->getPredecessors())
+      VPPhi->addOperand(IncomingValues.lookup(Pred->getExitingBasicBlock()));
   }
 }
 
diff --git a/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
index bc80c5ea0b1b2af..b86b6285fc50183 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
@@ -3577,6 +3577,21 @@ void VPReductionPHIRecipe::print(raw_ostream &O, const Twine &Indent,
 }
 #endif
 
+VPBasicBlock *VPWidenPHIRecipe::getIncomingBlock(unsigned I) {
+  VPBasicBlock *Parent = getParent();
+  VPBlockBase *Pred = nullptr;
+  if (Parent->getNumPredecessors() == 0) {
+    auto *R = Parent->getParent();
+    assert(R && R->getEntry() == Parent);
+    assert(I < 2);
+    Pred = I == 0 ? R->getSinglePredecessor() : R;
+  } else {
+    Pred = Parent->getPredecessors()[I];
+  }
+
+  return Pred->getExitingBasicBlock();
+}
+
 void VPWidenPHIRecipe::execute(VPTransformState &State) {
   assert(EnableVPlanNativePath &&
          "Non-native vplans are not expected to have VPWidenPHIRecipes.");
diff --git a/llvm/lib/Transforms/Vectorize/VPlanUtils.h b/llvm/lib/Transforms/Vectorize/VPlanUtils.h
index ac5e1978fcfbe00..6ddb88308955f1a 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanUtils.h
+++ b/llvm/lib/Transforms/Vectorize/VPlanUtils.h
@@ -169,16 +169,8 @@ class VPBlockUtils {
   static void reassociateBlocks(VPBlockBase *Old, VPBlockBase *New) {
     for (auto *Pred : to_vector(Old->getPredecessors()))
       Pred->replaceSuccessor(Old, New);
-    for (auto *Succ : to_vector(Old->getSuccessors())) {
+    for (auto *Succ : to_vector(Old->getSuccessors()))
       Succ->replacePredecessor(Old, New);
-
-      // Replace any references to Old in widened phi incoming blocks.
-      for (auto &R : Succ->getEntryBasicBlock()->phis())
-        if (auto *WidenPhiR = dyn_cast<VPWidenPHIRecipe>(&R))
-          for (unsigned I = 0; I < WidenPhiR->getNumOperands(); I++)
-            if (WidenPhiR->getIncomingBlock(I) == Old)
-              WidenPhiR->setIncomingBlock(I, cast<VPBasicBlock>(New));
-    }
     New->setPredecessors(Old->getPredecessors());
     New->setSuccessors(Old->getSuccessors());
     Old->clearPredecessors();
diff --git a/llvm/test/Transforms/LoopVectorize/outer-loop-wide-phis.ll b/llvm/test/Transforms/LoopVectorize/outer-loop-wide-phis.ll
index 3f81c0f5c822a2e..c5d2f6acf85b380 100644
--- a/llvm/test/Transforms/LoopVectorize/outer-loop-wide-phis.ll
+++ b/llvm/test/Transforms/LoopVectorize/outer-loop-wide-phis.ll
@@ -134,7 +134,7 @@ define void @wide_phi_2_predecessors_phi_ops_swapped(ptr noalias %A, ptr noalias
 ; CHECK-NEXT:    [[WIDE_MASKED_GATHER:%.*]] = call <4 x i64> @llvm.masked.gather.v4i64.v4p0(<4 x ptr> [[TMP1]], i32 8, <4 x i1> splat (i1 true), <4 x i64> poison)
 ; CHECK-NEXT:    br label %[[INNER_LATCH4]]
 ; CHECK:       [[INNER_LATCH4]]:
-; CHECK-NEXT:    [[VEC_PHI5:%.*]] = phi <4 x i64> [ zeroinitializer, %[[INNER_HEADER1]] ], [ [[WIDE_MASKED_GATHER]], %[[THEN3]] ]
+; CHECK-NEXT:    [[VEC_PHI5:%.*]] = phi <4 x i64> [ [[WIDE_MASKED_GATHER]], %[[THEN3]] ], [ zeroinitializer, %[[INNER_HEADER1]] ]
 ; CHECK-NEXT:    [[TMP2:%.*]] = add nsw <4 x i64> [[VEC_PHI5]], [[VEC_IND]]
 ; CHECK-NEXT:    [[TMP3]] = add nsw <4 x i64> [[TMP2]], [[VEC_PHI2]]
 ; CHECK-NEXT:    [[TMP4]] = add nuw nsw <4 x i64> [[VEC_PHI]], splat (i64 1)
diff --git a/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp b/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp
index 2f37c08bd9f117d..b17725382e33c56 100644
--- a/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp
+++ b/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp
@@ -672,7 +672,7 @@ TEST_F(VPBasicBlockTest, reassociateBlocks) {
     auto *WidenPhi = new VPWidenPHIRecipe(nullptr);
     IntegerType *Int32 = IntegerType::get(C, 32);
     VPValue *Val = Plan.getOrAddLiveIn(ConstantInt::get(Int32, 1));
-    WidenPhi->addIncoming(Val, VPBB1);
+    WidenPhi->addOperand(Val);
     VPBB2->appendRecipe(WidenPhi);
 
     VPBasicBlock *VPBBNew = Plan.createVPBasicBlock("VPBBNew");
@@ -693,7 +693,7 @@ TEST_F(VPBasicBlockTest, reassociateBlocks) {
     auto *WidenPhi = new VPWidenPHIRecipe(nullptr);
     IntegerType *Int32 = IntegerType::get(C, 32);
     VPValue *Val = Plan.getOrAddLiveIn(ConstantInt::get(Int32, 1));
-    WidenPhi->addIncoming(Val, VPBB1);
+    WidenPhi->addOperand(Val);
     VPBB2->appendRecipe(WidenPhi);
 
     VPBasicBlock *VPBBNew = Plan.createVPBasicBlock("VPBBNew");

``````````

</details>


https://github.com/llvm/llvm-project/pull/126388


More information about the llvm-commits mailing list