[PATCH] D99294: [VPlan] Representing backedge def-use feeding reduction phis.

[Ayal Zaks via Phabricator via llvm-commits]

Ayal added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8683
+  for (PHINode *PN : PhisToFix) {
+    VPWidenPHIRecipe *R = cast<VPWidenPHIRecipe>(getRecipe(PN));
+    VPRecipeBase *IncR =
----------------
can alternatively traverse over  `R : PhiRecipesToFix`, and use R's underlying ingredient to retrieve its PN, etc.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8799
+      PhisToFix.push_back(Phi);
+      recordRecipeOf(Phi);
+      recordRecipeOf(cast<Instruction>(
----------------
Instead of recording the recipe of Phi, which the next line creates, can do

```
  auto *PhiRecipe = new VPWidenPHIRecipe(Phi, RdxDesc, *StartV);
  PhiRecipesToFix.push_back(PhiRecipe);
  return toVPRecipeResult(PhiRecipe);
```


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.h:991
+/// descriptor, the start value is the first operand of the recipe and the
+/// incoming value from the backedge is the second operand. In the VPlan native
+/// path, all incoming VPValues & VPBasicBlock pairs are managed in the recipe
----------------
fhahn wrote:
> Ayal wrote:
> > Worth supplying another interface to retrieve 'preheadered' and 'latched' operands of a header phi recipe (by setting them as 0 and 1 enum values?) to increase the readability of 'getOperand(0)' and 'getOperand(1)'?
> I added a `VPWidenPHIRecipe::getBackedgeValue()`, similar to `getStartValue`. WDYT?
Great!


================
Comment at: llvm/test/Transforms/LoopVectorize/vplan-printing.ll:82
 ; CHECK-NEXT:   WIDEN-INDUCTION %iv = phi %iv.next, 0
-; CHECK-NEXT:   WIDEN-PHI %red = phi %red.next, 0.000000e+00
+; CHECK-NEXT:   WIDEN-PHI ir<%red> = phi ir<0.000000e+00>, ir<%red.next>
 ; CHECK-NEXT:   CLONE ir<%arrayidx> = getelementptr ir<%y>, ir<%iv>
----------------
fhahn wrote:
> Ayal wrote:
> > Clean up the TODO in VPlan.cpp/ VPWidenPHIRecipe::print() ?
> Unfortunately there are still cases where we do not track the value from the back-edge (like first-order recurrences).
Ah, right.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99294