[llvm] d9f8316 - [VPlan] Ensure start value of phis is the first op at construction (NFC)

[Florian Hahn via llvm-commits]

Author: Florian Hahn
Date: 2023-09-22T21:24:15+01:00
New Revision: d9f83169d141c2deb29612e7831f25b1e0f8c006

URL: https://github.com/llvm/llvm-project/commit/d9f83169d141c2deb29612e7831f25b1e0f8c006
DIFF: https://github.com/llvm/llvm-project/commit/d9f83169d141c2deb29612e7831f25b1e0f8c006.diff

LOG: [VPlan] Ensure start value of phis is the first op at construction (NFC)

Header phi recipes have the start value (incoming from outside the loop)
as first operand. This wasn't the case for VPWidenPHIRecipes. Instead
the start value was picked during execute() by doing extra work.

To be in line with other recipes, ensure the operand order is as
expected during construction.

Added: 
    

Modified: 
    llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp
    llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp b/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp
index 1184f8b34afb2d9..f5e94edc319848c 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp
@@ -89,6 +89,10 @@ void PlainCFGBuilder::setVPBBPredsFromBB(VPBasicBlock *VPBB, BasicBlock *BB) {
   VPBB->setPredecessors(VPBBPreds);
 }
 
+static bool isHeaderBB(BasicBlock *BB, Loop *L) {
+  return L && BB == L->getHeader();
+}
+
 // Add operands to VPInstructions representing phi nodes from the input IR.
 void PlainCFGBuilder::fixPhiNodes() {
   for (auto *Phi : PhisToFix) {
@@ -100,6 +104,22 @@ void PlainCFGBuilder::fixPhiNodes() {
     assert(VPPhi->getNumOperands() == 0 &&
            "Expected VPInstruction with no operands.");
 
+    Loop *L = LI->getLoopFor(Phi->getParent());
+    if (isHeaderBB(Phi->getParent(), L)) {
+      // For header phis, make sure the incoming value from the loop
+      // predecessor is the first operand of the recipe.
+      assert(Phi->getNumOperands() == 2);
+      BasicBlock *LoopPred = L->getLoopPredecessor();
+      VPPhi->addIncoming(
+          getOrCreateVPOperand(Phi->getIncomingValueForBlock(LoopPred)),
+          BB2VPBB[LoopPred]);
+      BasicBlock *LoopLatch = L->getLoopLatch();
+      VPPhi->addIncoming(
+          getOrCreateVPOperand(Phi->getIncomingValueForBlock(LoopLatch)),
+          BB2VPBB[LoopLatch]);
+      continue;
+    }
+
     for (unsigned I = 0; I != Phi->getNumOperands(); ++I)
       VPPhi->addIncoming(getOrCreateVPOperand(Phi->getIncomingValue(I)),
                          BB2VPBB[Phi->getIncomingBlock(I)]);

diff  --git a/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
index d507f436feb8153..2a1213a98095907 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
@@ -1626,23 +1626,7 @@ void VPWidenPHIRecipe::execute(VPTransformState &State) {
   assert(EnableVPlanNativePath &&
          "Non-native vplans are not expected to have VPWidenPHIRecipes.");
 
-  // Currently we enter here in the VPlan-native path for non-induction
-  // PHIs where all control flow is uniform. We simply widen these PHIs.
-  // Create a vector phi with no operands - the vector phi operands will be
-  // set at the end of vector code generation.
-  VPBasicBlock *Parent = getParent();
-  VPRegionBlock *LoopRegion = Parent->getEnclosingLoopRegion();
-  unsigned StartIdx = 0;
-  // For phis in header blocks of loop regions, use the index of the value
-  // coming from the preheader.
-  if (LoopRegion->getEntryBasicBlock() == Parent) {
-    for (unsigned I = 0; I < getNumOperands(); ++I) {
-      if (getIncomingBlock(I) ==
-          LoopRegion->getSinglePredecessor()->getExitingBasicBlock())
-        StartIdx = I;
-    }
-  }
-  Value *Op0 = State.get(getOperand(StartIdx), 0);
+  Value *Op0 = State.get(getOperand(0), 0);
   Type *VecTy = Op0->getType();
   Value *VecPhi = State.Builder.CreatePHI(VecTy, 2, "vec.phi");
   State.set(this, VecPhi, 0);