[PATCH] D100102: [VPlan] Iterate over phi recipes to detect reductions to fix.

[Ayal Zaks via Phabricator via llvm-commits]

Ayal added a comment.

This does look pretty simpler now :-). Adding some more thoughts, could be addressed separately.



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9404
   // Adjust the recipes for any inloop reductions.
   adjustRecipesForInLoopReductions(Plan, RecipeBuilder, Range.Start);
 
----------------
Perhaps wrap the following insert-select-for-non-in-loop-reduction-under-fold-tail along with the above, under adjustRecipesForReductions()? buildVPlanWithVPRecipes() has grown out of propotion.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9416
         continue;
-      VPValue *Phi = Plan->getOrAddVPValue(Reduction.first);
-      VPValue *Red = Plan->getOrAddVPValue(Reduction.second.getLoopExitInstr());
-      Builder.createNaryOp(Instruction::Select, {Cond, Red, Phi});
+      if (!Cond)
+        Cond = RecipeBuilder.createBlockInMask(OrigLoop->getHeader(), Plan);
----------------
Checking if (!Cond) seems redundant; if we want to avoid creating BlockInMask for the header block here unless there is at-least one reduction (although it will probably be created anyhow, especially under foldTail), may as well call createBlockInMask() repeatedly - it provides internal caching. I.e., should be renamed more accurately getOrCreateBlockInMask().


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9419
+      VPValue *Red = Plan->getOrAddVPValue(
+          PhiR->getRecurrenceDescriptor().getLoopExitInstr());
+      Builder.createNaryOp(Instruction::Select, {Cond, Red, PhiR});
----------------
`VPValue *Red = PhiR->getBackedgeValue ();`?

Would be good to note that (and remember how ;-) ILV::fixReduction() later hooks up to this select, as it seems pretty useless here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100102