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

[Ayal Zaks via Phabricator via llvm-commits]

Ayal accepted this revision.
Ayal added a comment.
This revision is now accepted and ready to land.

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127966