[PATCH] D127966: [LV] Move LoopVersioning creation to LVP::execute.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

In D127966#3614898 <https://reviews.llvm.org/D127966#3614898>, @Ayal wrote:

> In D127966#3612813 <https://reviews.llvm.org/D127966#3612813>, @fhahn wrote:
>
>> In D127966#3612708 <https://reviews.llvm.org/D127966#3612708>, @Ayal wrote:
>>
>>> In D127966#3611652 <https://reviews.llvm.org/D127966#3611652>, @fhahn wrote:
>>>
>>>> Add extra check if there are any runtime checks before creating LoopVersioning, move code up.
>>>
>>> Great, thanks!
>>> Currently noalias is introduced by emitMemRuntimeChecks() when called from ILV::createVectorizedLoopSkeleton() and from its overriding EpilogueVectorizerMainLoop::createEpilogueVectorizedLoopSkeleton(), but is missed by its overriding EpilogueVectorizerEpilogueLoop::createEpilogueVectorizedLoopSkeleton(). How about placing it in completeLoopSkeleton(), which all three skeleton creators call? It does, after all, seem to belong to ILV* than to LVP?
>>
>> The follow-up patches move `addMetadata` to `VPTransformState`, which requires `VPTransformState` to have access to `LoopVersioning`. If we move the code to `completeLoopSkeleton`, it would have to be updated to return the `LoopVersioning`. Please let me know if you prefer it that way and I'll updated the patch :)
>
> OK, and having LVP take care of creating LVer (after it creates State and calls ILV* to create the skeleton) and passing it into State is one way of achieving this.
> We should find a better way for State to take care of all metadata, and if/how `LoopVersioning` is reused for this; would be good to leave behind a TODO.

Thanks, I added a TODO in the committed version!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127966