[PATCH] D104533: [LoopVectorize] Fix strict reductions where VF = 1

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9266
   // Adjust the recipes for any inloop reductions.
-  if (Range.Start.isVector())
-    adjustRecipesForInLoopReductions(Plan, RecipeBuilder);
+  adjustRecipesForInLoopReductions(Plan, RecipeBuilder, Range.Start);
 
----------------
Is this a valid change for in-loop reductions that are not in-order? Or does this now break things for VF=1, UF>1 for in-loop reductions?


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9340
 void LoopVectorizationPlanner::adjustRecipesForInLoopReductions(
-    VPlanPtr &Plan, VPRecipeBuilder &RecipeBuilder) {
+    VPlanPtr &Plan, VPRecipeBuilder &RecipeBuilder, ElementCount VF) {
   for (auto &Reduction : CM.getInLoopReductionChains()) {
----------------
nit: MinVF ?


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9362-9363
       } else {
-        assert(isa<VPWidenRecipe>(WidenRecipe) &&
-               "Expected to replace a VPWidenSC");
+        if (VF.isVector())
+          assert(isa<VPWidenRecipe>(WidenRecipe) &&
+                "Expected to replace a VPWidenSC");
----------------
nit: `assert ((VF.isScalar() || isa<VPWidenRecipe>(WidenRecipe)) && ...)`


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9386
             RecipeBuilder.getRecipe(cast<Instruction>(R->getOperand(0)));
         assert(isa<VPWidenRecipe>(CompareRecipe) &&
                "Expected to replace a VPWidenSC");
----------------
Should these asserts be guarded as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104533