[PATCH] D104197: [VPlan] Track both incoming values for first-order recurrence phis.

[Ayal Zaks via Phabricator via llvm-commits]

Ayal added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:4768
         ScalarPHI ? PN->getType() : VectorType::get(PN->getType(), State.VF);
 
     for (unsigned Part = 0; Part < State.UF; ++Part) {
----------------
(Unrelated to this patch, but setting the loop's upper bound in advance to `unsigned LastPartForNewPhi = IsOrdered ? 1 : State.UF;` seems clearer than breaking inside the loop. Here and below.)


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:4781
+
+  if (RdxDesc) {
+    VPValue *StartVPV = PhiR->getStartValue();
----------------
Instead of asking if (RdxDesc) here, move the following block earlier to after FOR(P)'s return; i.e., have the "if (RdxDesc || FOR(P))" fully handle both?

This will also allow to keep the definitions of ScalarPHI and VecTy inside "if (RdxDesc || FOR(P))".


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8980
+      } else {
+        PhiRecipe = new VPWidenPHIRecipe(Phi);
+        PhiRecipe->addOperand(StartV);
----------------
Add a VPWidenPHIRecipe(Phi, *StartV) constructor? (If not split into VPWidenReductionPHIRecipe, VPWidenFORPHIRecipe, VPWidenPointerIVPHIRecipe.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104197