[PATCH] D100113: [LV] Move reduction PHI node fixup to VPlan::execute (NFC).

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.cpp:824
+      // For first-order recurrences, only a single part is generated,
+      // regardless of the UF.
       auto *VecPhi = cast<PHINode>(State->get(FOR, 0));
----------------
Ayal wrote:
> > The isOrdered case does behave quite similar to FOR phi's.
> How about having a common treatment, e.g.,
> 
> ```
>   for (VPRecipeBase &R : Header->phis()) {
>     auto *PhiR = dyn_cast<VPWidenPHIRecipe>(&R);
>     if (!PhiR)
>       continue;
>     // For first-order recurrences and in-order reduction phis, only a single part
>     // is generated, which provides the last part from the previous iteration.
>     // Otherwise all UF parts are generated.
>     bool IsOrdered = (isa<VPFirstOrderRecurrencePHIRecipe>(&R)
>                       || cast<VPReductionPHIRecipe>(&R)->isOrdered());
>     unsigned LastPartForNewPhi = IsOrdered ? 1 : State->UF;
>     for (unsigned Part = 0; Part < LastPartForNewPhi; ++Part) {
>       Value *VecPhi = State->get(PhiR->getVPSingleValue(), Part);
>         // Or suffice State->get(PhiR, Part)?
>       Value *Val = State->get(PhiR->getBackedgeValue(),
>                               IsOrdered ? State->UF - 1 : Part);
>       cast<PHINode>(VecPhi)->addIncoming(Val, VectorLatchBB);
>     }
> ```
> ?
Looks good, updated with a small re-naming. of `IsOrdered` -> `SinglePartNeeded` and a check to limit to FOR and reduction PHIs only.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100113