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

[Ayal Zaks via Phabricator via llvm-commits]

Ayal added a comment.

Thanks for accommodating, this looks good to me, with a couple of final nits.
@rengolin, @bmahjour, @venkataramanan.kumar.llvm - any further comments or ok with you to accept?



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:10514
+            if (auto *Ind = dyn_cast<VPWidenPointerInductionRecipe>(&R)) {
+              IndPhi = cast<PHINode>(Ind->getUnderlyingValue());
+              ID = &Ind->getInductionDescriptor();
----------------
nit: may introduce getPHINode() here as well; and potentially a pure virtual VPWidenInductionRecipe base class to cover the common interface of it and getInductionDescriptor().


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:10521
+            }
+            if (!IndPhi) {
+              assert(isa<VPCanonicalIVPHIRecipe>(&R) &&
----------------
nit: `} else {` ?


================
Comment at: llvm/test/Transforms/LoopVectorize/X86/gather_scatter.ll:173
 ; AVX512-LABEL: @foo2(
-; AVX512-NEXT:  entry:
+; AVX512-NEXT:  iter.check:
 ; AVX512-NEXT:    br label [[VECTOR_BODY:%.*]]
----------------
Ayal wrote:
> nit: why this redundant label change, here and additional multiple instances below?
ok, `iter.check` is an indicator that createEpilogueVectorizedLoopSkeleton() kicked in, sigh.


================
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:
> Adding assertions to show we can vectorize main loop but not epilog?
Clarify why assertions were added here now?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92132