[PATCH] D75980: [LV] Generate RT checks up-front and remove them if required. (WIP)

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

In D75980#2165968 <https://reviews.llvm.org/D75980#2165968>, @ebrevnov wrote:

>




> I think the topic deserves a dedicated discussion on dev list in a wider audience because it is not trivial one.

Agreed, it would be good to make sure there are no major objections on the general approach. We already have a few cases of 'prior art' in the code base, but this patch might take things a little further than in some other places.

> In fact I have an experience of doing similar thing in SLP and it was mainly negative. Things look simple in the beginning and we went ahead and implemented this approach. The problem was that over the time it became hard to manage pre-generated instructions correctly and we started having nasty bugs. Everything is simple until you don't have sharing. If you have references from multiple places (both from inside IR and internal data structures) things become complex and error prone. I would not like to go this direction again until we 1000% sure things will remain simple.

>From what I've seen so far, the major concerns is around SCEV expansion. I started some patches solidifying the way we clean up after SCEV expansion (D84327 <https://reviews.llvm.org/D84327>), which is already done in LoopIdiomRecognize. I think we need to properly address this first, before continuing here.

I am not too concerned about interactions with other LV internal data structures, as the runtime checks are pretty independent of vector code generation/legality, as they are outside of the loop and nothing in the vector loop should be aware of the checks.

> One more thing is concerning me in this particular implementation. I think the way we are trying to "hide" Checks.TmpBlock from all global analysis and data structures is fragile and can be source of bugs in future. In my opinion it would be much easier and cleaner to make pre-generated code runtime dead (by changing guarding condition to be unconditionally false) and let dedicated cleanup passes make their job.

That's an interesting suggestion and would certainly simplify things. My main concern with leaving cleanup to another pass is that it  would mean that LV would always modify functions with loops that need runtime checks, even if the checks are not used. Not sure how big of an issue that would be in terms of compile-time caused by the additional invalidations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75980