[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