[PATCH] D97278: [VPlan] Support to widen call intructions in VPlan native path

[Andrei Elovikov via Phabricator via llvm-commits]

a.elovikov added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp:75
+        } else if (CallInst *CI = dyn_cast<CallInst>(Inst)) {
+          NewRecipe = new VPWidenCallRecipe(
+              *CI, Plan->mapToVPValues(CI->arg_operands()));
----------------
fhahn wrote:
> Kazhuu wrote:
> > a.elovikov wrote:
> > > For my education, how do we know that such wide version is available? What if the vectorization was forced by OMP pragma (so bypass any dependence analysis legality checks), but the function being called is some arbitrary function that doesn't have a wide variant (i.e. not an intrinsic and no OMP's vector variant available)?
> > > 
> > > Also, what was the previous behavior? Did we serialize the call?
> > This is actually a good point. What I've checked the code the inner loop vectorizer use this function `VPRecipeBuilder::tryToWidenCall()` to try to widen the call. I guess something similar should be implemented here instead of trying to widen all calls. But of course avoid code duplication. Now this works because I've been only testing with simple intrinsic calls.
> > For my education, how do we know that such wide version is available? 
> 
> The short answer is we don't and we never have. This patch should restore the previous behavior, which regressed after splitting up VPWidenRecipe.
> 
> In its current state, the VPlan native path is missing a lot of legality checks. Currently it's the responsibility of the user to make sure it is only used for supported loops. Most of the recent work has been spent working on moving more bits of the inner loop vectorizer to VPlan, which in turn should eventually be useable for the VPlan native path as well. 
Thank you for the detailed response!

> This patch should restore the previous behavior

That makes sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97278