[PATCH] D96773: [VPlan] Manage pairs of incoming (VPValue, VPBB) in VPWidenPHIRecipe.

[Satish K Guggilla via Phabricator via llvm-commits]

sguggill requested changes to this revision.
sguggill added a comment.
This revision now requires changes to proceed.

Thanks for the changes Florian. Some comments:



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:4366-4367
 
     // The predecessor order is preserved and we can rely on mapping between
     // scalar and vector block predecessors.
+    for (unsigned i = 0; i < VPPhi->getNumOperands(); ++i) {
----------------
Please delete this now obsolete comment.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp:37
+      VPValue *VPV = Ingredient->getVPValue();
       // Can only handle VPInstructions.
+      Instruction *Inst = cast<Instruction>(VPV->getUnderlyingValue());
----------------
nit - please delete or update comment for VPWidenPHIRecipe case.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp:55-60
           NewRecipe = new VPWidenPHIRecipe(Phi);
-      } else if (GetElementPtrInst *GEP = dyn_cast<GetElementPtrInst>(Inst)) {
-        NewRecipe = new VPWidenGEPRecipe(
-            GEP, Plan->mapToVPValues(GEP->operands()), OrigLoop);
-      } else
-        NewRecipe =
-            new VPWidenRecipe(*Inst, Plan->mapToVPValues(Inst->operands()));
+          for (unsigned i = 0; i < VPPhi->getNumOperands(); ++i) {
+            VPValue *Inc = VPPhi->getIncomingValue(i);
+            VPBasicBlock *VPBB = VPPhi->getIncomingBlock(i);
+            cast<VPWidenPHIRecipe>(NewRecipe)->addIncoming(Inc, VPBB);
+          }
----------------
Do we need to create a new recipe here? Can we leave the original ingredient as is?


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp:76-83
+        else if (PHINode *Phi = dyn_cast<PHINode>(Inst)) {
+          InductionDescriptor II = Inductions.lookup(Phi);
+          if (II.getKind() == InductionDescriptor::IK_IntInduction ||
+              II.getKind() == InductionDescriptor::IK_FpInduction) {
+            VPValue *Start = Plan->getOrAddVPValue(II.getStartValue());
+            NewRecipe = new VPWidenIntOrFpInductionRecipe(Phi, Start, nullptr);
+          } else
----------------
Please delete.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96773/new/

https://reviews.llvm.org/D96773