[PATCH] D79976: [LV] Handle Fold-Tail of loops with vectorizarion factor (VF) equal to 1

[Ayal Zaks via Phabricator via llvm-commits]

Ayal added a comment.

In D79976#2045766 <https://reviews.llvm.org/D79976#2045766>, @anhtuyen wrote:

> In D79976#2045738 <https://reviews.llvm.org/D79976#2045738>, @anhtuyen wrote:
>
> > In D79976#2045007 <https://reviews.llvm.org/D79976#2045007>, @Ayal wrote:
> >
> > > This looks good to me, thanks! Please wait a day or so if @fhahn has further comments.
> > >
> > > As noted, it would now be good to also extend pr45679-fold-tail-by-masking.ll with an additional RUN of -force-vector-width=1 and -force-vector-interleave=4.
> >
> >
> > It is very kind of you, Ayal @Ayal , and I thank you very much. As you have mentioned, I surely will wait a day or two to see whether Florian and Bardia might have further comments.
> >  About pr45679-fold-tail-by-masking.ll, do you want me to add it for completeness? 
> >  With VF=1, the testcase does not go through the code changed by this patch (b/c its BackedgeTakenCount is null).
>
>
> Oh, I think I know what you meant now: adding the test pr45679-fold-tail-by-masking.ll **AFTER** applying your patch https://reviews.llvm.org/D80085. I will do.


Yes, thanks.
Sorry for the confusion; before D80085 <https://reviews.llvm.org/D80085> this test generated wrong code with VF=1, afterwards the compiler crashes, with this patch it generates correct code.



================
Comment at: llvm/test/Transforms/LoopVectorize/tail-folding-vectorization-factor-1.ll:107
+for.body:
+  %addr = phi double* [ %ptr, %for.body ], [ %ptr1, %entry ]
+  %ptr = getelementptr inbounds double, double* %addr, i64 1
----------------
anhtuyen wrote:
> Ayal wrote:
> > Interesting; this test w/o a primary IV seems to work ok w/o the patch; comparing pairs of vectors of size 1...
> > 
> > 
> Yes, this test should pass without this patch b/c it compares a pair of vectors size 1. When we decided to replace VTCMO with TCMO when State->VF == 1, we also updated VPWidenCanonicalIVRecipe::execute() , and it now compares two scalars. Did I mis-understand your above comment about its interesting-ness?
Right. Just noted that the original motivation was to fix crashing cases, and this test shows it also helps to improve correct code cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79976