[PATCH] D44338: [LV][VPlan] Build plain CFG with simple recipes for outer loops.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:7704
+    if (VPlanBuildStressTest)
+      return {UserVF, 0};
+
----------------
dcaballe wrote:
> fhahn wrote:
> > Could we here just return NoVectorization and get rid of the additional check in LoopVectorizePass::processLoop? Also, could we move the setting of UserVF in `plan` too? Otherwise it seems harder to keep track of what's going on and we set UserVF even for inner loops.
> > 
> > Also, I think ideally we would only bail out if the outer loop is not supported, but achieving that seems more trouble than it's worth.
> > Could we here just return NoVectorization and get rid of the additional check in LoopVectorizePass::processLoop?
> 
> Ok. It sounds good to me, at least for now. Hopefully we won't introduce anything after `plan` that we must skip in stress testing mode. Thanks!
> 
> > Also, could we move the setting of UserVF in plan too? Otherwise it seems harder to keep track of what's going on and we set UserVF even for inner loops.
> 
> Could you please elaborate a bit more? I'm not sure I understand what you mean.
> 
> > Also, I think ideally we would only bail out if the outer loop is not supported, but achieving that seems more trouble than it's worth.
> 
> We can think about it in the future. We would need legality analysis for outer loops if we want to generate code.
> Ok. It sounds good to me, at least for now. Hopefully we won't introduce anything after plan that we must skip in stress testing mode. Thanks!

If we return NoVectorization, I would assume we would not use the generated plans after `plan`, as we decided to skip vectorization?

> Could you please elaborate a bit more? I'm not sure I understand what you mean.

I meant moving the code to set UserVF  = 4 if VPlanBuildStressTest into this function. That would reduce the number of functions where we have to handle VPlanBuildStressTest, which IMO makes it easier to see what's going on.



Repository:
  rL LLVM

https://reviews.llvm.org/D44338