[PATCH] D155845: [VPlan] Fix in-loop reduction chains using VPlan def-use chains (NFCI)

[Ayal Zaks via Phabricator via llvm-commits]

Ayal accepted this revision.
Ayal added a comment.
This revision is now accepted and ready to land.

This looks good to me, thanks!
Adding last minor nits.

Would be good to see how adjustRecipesForReductions() may become a VPlan2VPlan pass, and apply fmuladd decomposition / minmax composition as separate, preparatory steps.



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9221
+                   "need to have the compare of the select");
+            continue;
+          }
----------------
Note: in this case PreviousLink stays the recipe before the compare, feeding one of the operands of the select.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9242
+                   PreviousLinkV &&
+               "PreviousLinkV must be the other operand than VecOp");
       }
----------------



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9244
+      VPRecipeBase *CompareRecipe =
+          CurrentLink->getOperand(0)->getDefiningRecipe();
+
----------------
fhahn wrote:
> Ayal wrote:
> > fhahn wrote:
> > > Ayal wrote:
> > > > Could we use PreviousLink serve as/instead of CompareRecipe?
> > > Unfortunately I don't think so, but we can let dead-recipe removal take care of this removal. I removed the code.
> > If PreviousLink is set to Cmp before early-continuing above?
> > 
> > OTOH, dead-recipe removal could also reclaim CurrentLink.
> Yep, updated, thanks!
nit: may be worth noting somewhere that this transformation may leave dead recipes for a subsequent pass to clean up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155845