[PATCH] D120828: [LV] Create & use VPScalarIVSteps for all scalar users.

[Ayal Zaks via Phabricator via llvm-commits]

Ayal added a comment.

This nicely removes from ILV both widenIntOrFpInduction() along with createVectorIntOrFpInductionPHI() which it calls, inlining them into VPlan's VPWidenIntOrFpInductionRecipe::execute(), while postponing buildScalarSteps() for all potential scalar users of the vectorized IV to a VPlan2VPlan transformation/optimization.

Is VPWidenIntOrFpInductionRecipe still created when there are only scalar users, and discarded later by useless recipe removal?



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9565
+  // required to be loop-invariant
+  auto CreateStepValue = [&](const SCEV *Step) -> Value * {
+    if (SE.isSCEVable(IV->getType())) {
----------------
We're losing the assertion for loop-invariance of step here. Worth retaining, somewhere else perhaps?


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp:387
 
 void VPlanTransforms::optimizeInductions(VPlan &Plan, ScalarEvolution &SE) {
   SmallVector<VPRecipeBase *> ToRemove;
----------------
Update documentation of optimizeInductions()?
This is an optional performance optimization, to save extracts?


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp:427
+          continue;
+        if (R->onlyScalarsUsed(IV) || R->onlyFirstLaneUsed(IV))
+          R->setOperand(I, Steps);
----------------
Checking if R->onlyFirstLaneUsed(IV) is redundant - suffice to check if onlyScalarsUsed(IV) as the latter covers the former?

First check if only scalars are used, then if so go look for the operand to replace, preferably using some "findOperand" lambda? If not some "replaceAllScalarUsesWith(IV, Steps)"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120828