[PATCH] D97378: [VPlan] Support to vectorize inner loops with VPlan native path enabled

[Andrei Elovikov via Phabricator via llvm-commits]

a.elovikov added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:4443
   PHINode *P = cast<PHINode>(PN);
-  if (EnableVPlanNativePath) {
-    // Currently we enter here in the VPlan-native path for non-induction
-    // PHIs where all control flow is uniform. We simply widen these PHIs.
+  if (EnableVPlanNativePath && !OrigLoop->isInnermost()) {
+    // We enter here in the VPlan-native path and when the loop is not the
----------------
Kazhuu wrote:
> fhahn wrote:
> > I think we should probably have a `UseVPlanNativePath` variable in `ILV`, which is true if `EnableVPlanNativePath && !OrigLoop->isInnerMost()` and replace all checks of `EnableVPlanNativePath` with checking `UseVPlanNativePath`.
> Added the boolean to `LoopVectorizationCostModel` class instead because the flags were used there as well and to avoid having the same flag in both ILV and `LoopVectorizationCostModel` classes.
For my education, why shouldn't we use VPlan native path for inner loop vectorization? Isn't it a simpler way to make the path stable enough (it's already covered by an options that is disabled by default and is served for development purposes only)? I'd expect inner loop vectorization should be a subset of what it should be able to do.

In other words, do we expect it to be a temporary workaround to enable more work on VPlan native path right now, or do I miss some design decision that would explain why this should be a long term fix?

To clarify - I'm not going to block the review/request changes to the patch because of this. It is solely for the purpose of me getting a better understanding of the ongoing VPlan development and future plans.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97378