[PATCH] D113223: [VPlan] Add VPCanonicalIVRecipe, partly retire createInductionVariable.

[Ayal Zaks via Phabricator via llvm-commits]

Ayal added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:578
 
-  /// Create a new induction variable inside L.
-  PHINode *createInductionVariable(Loop *L, Value *Start, Value *End,
-                                   Value *Step, Instruction *DL);
+  /// Connect the header to the header (backedge) and exit blocks of \p L.
+  void createHeaderBranch(Loop *L);
----------------
More accurately, something like:
/// Introduce a conditional branch (on true, condition to be set later) at the end of the header=latch connecting it to itself (across the backedge) and to the exit block of \p L.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8916
+// VPInstruction to increment it by VF * UF to the header block.
+static void addCanonicalIVPHIRecipes(VPlan &Plan, Type *IdxTy, DebugLoc DL,
+                                     bool HasNUW, bool IsVPlanNative) {
----------------
addCanonicalIVPHIRecipes >> addCanonicalIVRecipes

> ... to the header block.
The phi is added to the header block, the increment is added to the latch block.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.cpp:780
+  case VPInstruction::CanonicalIVIncrement:
+    O << "canonical induction increment";
+    break;
----------------
How about printing something like " VF*UF + " instead?


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.cpp:927
+    auto *PhiR = dyn_cast<VPHeaderPHIRecipe>(&R);
+    if (!PhiR || isa<VPWidenPHIRecipe>(&R))
+      continue;
----------------
`if (isa_and_nonnull<VPWidenPHIRecipe>(PhiR))` ?

Use PhiR instead of &R below?


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.cpp:948
+  auto *CanonicalIV = cast<VPCanonicalIVPHIRecipe>(&*Header->begin());
+  // TODO: Model compare and branch explicitly in VPlan as recipes.
+  auto *Next = State->get(CanonicalIV->getBackedgeValue(), 0);
----------------
call getCanonicalIV(), or call getCanonicalIVIncrement() which should take care of looking up the backedge value of the canonical IV - or retrieving the last recipe in the latch directly?


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp:178
   }
 
   for (const VPRegionBlock *Region :
----------------
Verify that the last recipe in Exit is a CanonicalIVIncrement VPInstruction?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113223