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

Florian Hahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 21 13:41:37 PST 2022


fhahn marked 3 inline comments as done.
fhahn added inline comments.


================
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());
----------------
Ayal wrote:
> 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.)
Yes that would reduce duplication. Done in 7d8528dbf290


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:10521
+            }
+            if (!IndPhi) {
+              assert(isa<VPCanonicalIVPHIRecipe>(&R) &&
----------------
Ayal wrote:
> 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});
>   }
> ```
Added the early exit at the top of the loop, thanks!


================
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:
> > Ayal wrote:
> > > Adding assertions to show we can vectorize main loop but not epilog?
> > Clarify why assertions were added here now?
> assertions gone...
I think the `REQUIRES: asserts` is needed because the test only checks debug output.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92132



More information about the llvm-commits mailing list