[PATCH] D92132: [LV] Support widened induction variables in epilogue vectorization.

[Ayal Zaks via Phabricator via llvm-commits]

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

Ship it!
This looks even better with the added tests, and the last issue @venkataramanan.kumar.llvm raised regarding replicated end values seems acceptable.
SCEV expanding the end values is worth a TODO? Added a couple of thoughts that can be addressed separately.



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:7734
   // can hit the same issue for any SCEV, or ValueTracking query done during
   // mutation.  See PR49900.
   getOrCreateTripCount(OrigLoop->getLoopPreheader());
----------------
Should this caching of trip count be taken are of by executePlan() prior to creating the skeleton, as raised in optimizeForVFAndUF()? (Independent of this patch.)


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:10521
+            }
+            if (!IndPhi) {
+              assert(isa<VPCanonicalIVPHIRecipe>(&R) &&
----------------
Ayal wrote:
> nit: `} else {` ?
Or early continue, along with above nit:

```
  if (isa<VPCanonicalIVPHIRecipe>(&R))
    continue;
  if (auto *ReductionPhi = dyn_cast<VPReductionPHIRecipe>(&R))
    ResumeV = MainILV.getReductionResumeValue(ReductionPhi->getRecurrenceDescriptor());
  else {
    auto *Ind = cast<VPWidenInductionRecipe>(&R);
    PHINode *IndPhi = Ind->getPHINode();
    const InductionDescriptor *ID = &Ind->getInductionDescriptor();
    ResumeV = MainILV.createInductionResumeValue(IndPhi, *ID, {EPI.MainLoopIterationCountCheck});
  }
```


================
Comment at: llvm/test/Transforms/LoopVectorize/optimal-epilog-vectorization-limitations.ll:12
 define signext i32 @f2(i8* noalias %A, i32 signext %n) {
+; CHECK-LABEL: @f2(
+; CHECK-NEXT:  entry:
----------------
Ayal wrote:
> Ayal wrote:
> > Adding assertions to show we can vectorize main loop but not epilog?
> Clarify why assertions were added here now?
assertions gone...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92132