[PATCH] D104631: [LoopVersioning] Allow versionLoop to create plain branch inst when no runtime check is specified

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

In D104631#3185655 <https://reviews.llvm.org/D104631#3185655>, @eopXD wrote:

>> IIUC you only need 3). Personally it seems like extending versionLoop to only do 3) makes the interface more complicated than it needs to be. Instead, would it be possible to extract the code to do 3) into a utility function that returns the conditional branch to select the loop? Then you could use that in LoopIdiomRecognize directly.
>
> Hi @fhahn,
>
> Sorry for the late reply.
> I think I have made minimum change for `versionLoop` and only exposed utility functions (`getRuntimeCheckBB` and `getRuntimeCheckBI`) for use. 
> The usage of these two functions are inside D104636 <https://reviews.llvm.org/D104636>.

FWIW I still think the interface should be adopted to make this more useable, rather than just adding on additional state that makes it even harder to make further changes. While the changes here are indeed a bit smaller than improving the interface, the additional state that needs maintained increases the complexity of the class even further, which has a cost down the road.

I *think* the patch should only be committed once  D104636 <https://reviews.llvm.org/D104636> has also been accepted; at the moment there's nothing to test the new functionality.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104631