[PATCH] D67764: [LV] Forced vectorization with runtime checks and OptForSize

[Ayal Zaks via Phabricator via llvm-commits]

Ayal added a comment.

In D67764#1678098 <https://reviews.llvm.org/D67764#1678098>, @SjoerdMeijer wrote:

> Ah, I had completely missed that this is about the interaction of `OptForSize` and the `loop.vectorize` loop hint!
>  Vectorization is forced, which means that we vectorize even though `OptForSize` is set and we need to emit runtime checks.
>
> The symptom is that we run in this assert in `emitMemRuntimeChecks()`:
>
>   assert(!BB->getParent()->hasOptSize() &&
>        "Cannot emit memory checks when optimizing for size");
>   
>
> In `getScalarEpilogueLowering()`, we do **not** set CM_ScalarEpilogueNotAllowedOptSize because of the loop hint:
>
>   if (Hints.getForce() != LoopVectorizeHints::FK_Enabled &&
>       (F->hasOptSize() ||
>        llvm::shouldOptimizeForSize(L->getHeader(), PSI, BFI))) {
>     SEL = CM_ScalarEpilogueNotAllowedOptSize;
>   
>
> And because `CM_ScalarEpilogueNotAllowedOptSize` is not set, we do not check `runtimeChecksRequired`,  so that we end up with code growth under OptForSize because of the loop hint.
>
> If we agree that this is desired behaviour, which looks reasonable to me because of this condition, I am going to remove the assert as that is not valid.


Better to make the assert valid by checking !forcedToVectorize in addition to !OptForSize.


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

https://reviews.llvm.org/D67764