[PATCH] D128755: [VPlan] Make sure optimizeInductions removes wide ind from scalar plan.

[Ayal Zaks via Phabricator via llvm-commits]

Ayal added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp:384
   VPBasicBlock *HeaderVPBB = Plan.getVectorLoopRegion()->getEntryBasicBlock();
+  bool NeedsScalarSteps = Plan.hasVF(ElementCount::getFixed(1));
   for (VPRecipeBase &Phi : HeaderVPBB->phis()) {
----------------
How about `AllUseScalars`, or `ScalarVF`? "NeedsScalarSteps" combines the existence of a true usesScalars user together with a scalar VF.

Worth clarifying that usesScalars() assumes a VF>1 vector is given, as when VF=1 all users are provided a scalar and use it. I.e., "usesScalarsOfAVector()".


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp:389
+        (all_of(IV->users(), [IV](VPUser *U) { return !U->usesScalars(IV); }) &&
+         !NeedsScalarSteps))
       continue;
----------------
Worth reversing the condition, to early-continue if (!(IV && (NeedsScalarSteps || any_of(usesScalars(IV))?
(BTW, none_of() seems clearer than all_of(!)).


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp:405
     for (VPUser *U : Users) {
-      if (!U->usesScalars(IV))
+      if (!U->usesScalars(IV) && !NeedsScalarSteps)
         continue;
----------------
Worth reversing the condition, to early-continue if (!(NeedsScalarSteps || usesScalars(IV))?


================
Comment at: llvm/test/Transforms/LoopVectorize/interleave-and-scalarize-only.ll:172
+
+
+; DBG-LABEL: 'first_order_recurrence_using_induction'
----------------
Explain what this test is designed to check? (Worth pre-committing it to demonstrate failure?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128755