[PATCH] D118167: [VPlan] Record whether scalar IVs are need in induction recipe. (NFC)

[Ayal Zaks via Phabricator via llvm-commits]

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

This is fine, couple of final nits; thanks for accommodating!



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:2587
+  // If only a vector induction is needed, create it and return.
+  if (Def->needsVectorIV() && !Def->needsScalarIV()) {
     createVectorIntOrFpInductionPHI(ID, Step, Start, EntryVal, Def, State);
----------------
Suffice to ask if `!Def->needsScalarIV()` as that implies `Def->needsVectorIV()` - they cannot both be false if Def is not dead - but being explicit is clearer?


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:2595
   // loop iteration.
-  if (!shouldScalarizeInstruction(EntryVal)) {
+  if (Def->needsVectorIV() && Def->needsScalarIV()) {
     createVectorIntOrFpInductionPHI(ID, Step, Start, EntryVal, Def, State);
----------------
Suffice to ask `if Def->needsVectorIV()` as we already know that `Def->needsScalarIV()` here - but being explicit is clearer?


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8525
+static VPWidenIntOrFpInductionRecipe *
+createInductionRecipe(PHINode *Phi, Instruction *PhiOrTrunc, VPValue *Start,
+                      const InductionDescriptor &IndDesc,
----------------
"create[Widen]InductionRecipe" ?


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8547
+      },
+      Range);
+  bool NeedsScalarIVOnly = LoopVectorizationPlanner::getDecisionAndClampRange(
----------------
Nice to see the above encapsulated here!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118167