[PATCH] D57837: [LV] Prevent interleaving if computeMaxVF returned None.

[Ayal Zaks via Phabricator via llvm-commits]

Ayal added a comment.

In D57837#1387764 <https://reviews.llvm.org/D57837#1387764>, @hsaito wrote:

> I have a little mental barrier in accepting this change as is. I think this feeling of mine is mainly due to the name and the "stated" functionality of computeMaxVF and "indirect inference" towards using it for suppressing interleaving. Maybe I'm just being too picky here. If so, my apologies ahead of time.
>
> Other than just the function name and the associated comment:
>
> 1. runtime ptr check && hasBranchDivergence                   interleave w/ VF=1 still okay?
> 2. For interleaving, the remaining checks other than TC==0/1 (for better diagnostics) and OptForSize appear useless. Even if TC is evenly divisible by VF or canFoldTailByMasking, we still can't interleave under OptForSize.
>
>   I think we should start thinking in terms of separate "is vectorization feasible" and "is interleaving feasible" (plus possibly "is unrolling feasible"), even if we evaluate those feasibility in one function.
>
>   Possibly going to tangent, but while we look at computeMaxVF, at the bottom, we say use pragma, but we bail out of plan() w/o checking whether UserVF exists or not.
>
>   Hopefully, this conveys enough about my feeling of uneasiness.


This patch tries only to avoid running into the "no-VPlan-was-built-for-interleaving" problem, w/o modifying any other behavior or performing any refactoring. The latter indeed deserve to be addressed, but in separate patches.

For 1), a same runtime ptr check is needed for both vectorization and interleaving, so seems it should prevent both if one wishes to avoid introducing such branches before a loop when hasBranchDivergence. But I can't find a review explaining r309890.



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:7325
+  unsigned IC = 1;
+  unsigned UserIC = 1;
+  if (MaybeVF) {
----------------
fhahn wrote:
> Ayal wrote:
> > UserIC should always be set to Hints.getInterleave().
> Ah I thought you meant that we should ignore UserIC, in case computeMaxVF returned None, similar to the way we ignore UserVF in that case. Did I miss something?
UserIC should remain intact. Only IC must decide not to interleave if no (suitable) VPlan was built.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57837