[PATCH] D99750: [LV, VP]VP intrinsics support for the Loop Vectorizer

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp:986
+  auto *WideCanonicalIV =
+      cast<VPWidenCanonicalIVRecipe>(*FoundWidenCanonicalIVUser);
+  // Walk users of WideCanonicalIV and replace all compares of the form
----------------
ABataev wrote:
> fhahn wrote:
> > ABataev wrote:
> > > fhahn wrote:
> > > > ABataev wrote:
> > > > > shiva0217 wrote:
> > > > > > There is a case that VPWidenCanonicalIVRecipe didn't be generated with tail folding.
> > > > > > 
> > > > > >     int i;
> > > > > >     int foo (int q, int z)
> > > > > >     {
> > > > > >       int e = 0;
> > > > > >       while (z < 1)
> > > > > >         {
> > > > > >           q = z * 2;
> > > > > >           if (q != 0)
> > > > > >             for (i = 0; i < 2; ++i)
> > > > > >               e = 5;
> > > > > >           ++z;
> > > > > >         }
> > > > > >       return e;
> > > > > >     }
> > > > > > 
> > > > > > `for (i = 0; i < 2; ++i)` been simplifed as `store i32 2, ptr @i`.
> > > > > > Both pointer and store value are loop-invariant, so the mask(VPWidenCanonicalIVRecipe) might not be generated.
> > > > > > Should we suppress the replacement when the mask is not available?
> > > > > Fixed, added the test
> > > > Was this fixed by adding the ` bool KeepVPCanonicalWidenRecipes` flag? What's the test case for this? There's a new `no_masking` case, but it has an empty body and no vector code is generated? 
> > > > 
> > > 1. Yes.
> > > 2. Yes, this test is a reduced version of the failed case
> > Is it possible it is over-reduced? The same IR seems to be generated both with and without `KeepVPCanonicalWidenRecipes` IIUC, because the loop is not vectorized due to being empty. 
> > 
> > What's the issue if there's no mask/canonical widen recipe? Wouldn't it be fine to jus not replace anything?
> When I checked it, it crashed without this parameter, maybe there were some other changes.
Can you check if it still crashes? Would be good to understand exactly what the issue is, and if possible avoid having a separate `KeepVPCanonicalWidenRecipes` flag


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99750