[PATCH] D53612: [LV] Avoid vectorizing loops under opt for size that involve SCEV checks

[Ayal Zaks via Phabricator via llvm-commits]

Ayal added a comment.

In https://reviews.llvm.org/D53612#1275437, @bjope wrote:

> ... 
>  One thing I noticed is that if I use the test case from PR39417 and add `-vectorizer-min-trip-count=3`, to avoid the detection of a "very small trip count", the loop will be vectorized with VF=16. That is also what happened when we triggered the assert (without this patch). Shouldn't the VF be clamped to the trip count? 
>  It seems like the vectorizer detects that the trip count is tiny (trip count is 3), but it vectorize using VF=16 but then the vectorized loop is skipped since we emit `  br i1 true, label %scalar.ph, label %vector.scevcheck`. So all the hard work with vectorizing the loop is just a waste of time, or could it be beneficial to have `VF > tripcount` in some cases?
>
> If the actual problem is that VF should be clamped to the trip count, then maybe this patch just hides that problem in certain cases (when having OptForSize).


Interesting :-)

If tail **is** folded by masking, then VF's greater than the trip count are potentially relevant.

If tail is **not** folded by masking, it's indeed futile to use any VF(*UF) greater than the trip count. LV doesn't really notice this; IRBuilder does, when `emitMinimumIterationCountCheck()` asks it to

  CheckMinIters = Builder.CreateICmp(
      P, Count, ConstantInt::get(Count->getType(), VF * UF),
      "min.iters.check");

it simply sets `CheckMinIters` to 'true'.

Another aspect related to known trip counts (smaller than VF): they should be used when comparing VectorCost to ScalarCost, instead of taking

  float VectorCost = C.first / (float)i;

In any case, this patch deals with trip counts only indirectly, as they trigger OptForSize when small. Indeed worthy of a separate ticket/patch.


Repository:
  rL LLVM

https://reviews.llvm.org/D53612