[PATCH] D123541: [VPlan] Replace remaining use of needsScalarIV.

Ayal Zaks via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 13 13:27:58 PDT 2022


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

Looks good to me, curious if it may lead to any regression...
Adding minor nits.



================
Comment at: llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp:415
 
 void VPlanTransforms::optimizeInductions(VPlan &Plan, ScalarEvolution &SE) {
   SmallVector<VPRecipeBase *> ToRemove;
----------------
nit (irrespective of this patch): would be good to have a more descriptive name, e.g., optimizeScalarInductions().


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp:419
   for (VPRecipeBase &Phi : HeaderVPBB->phis()) {
     auto *IV = dyn_cast<VPWidenIntOrFpInductionRecipe>(&Phi);
+    if (!IV || all_of(IV->users(), [IV](VPUser *U) {
----------------
nit: a comment should explain that if no user of IV uses scalars, there's no need for an optimized scalar induction.
nit: may be clearer to use none_of() instead of all_of() with !


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp:434
 
     // If there are no vector users of IV, simply update all users to use Step
     // instead.
----------------
nit: "Step" >> "Steps"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123541



More information about the llvm-commits mailing list