[PATCH] D45420: [NFC] [LoopUtil] Moved RecurrenceDescriptor/LoopDescriptor from Transform/Utils/LoopUtils.* to Analysis tree

[Renato Golin via Phabricator via llvm-commits]

rengolin added a comment.

In https://reviews.llvm.org/D45420#1070587, @hsaito wrote:

> Side effect of that is https://reviews.llvm.org/D45552 will have to land in Transform first and then move to Analysis when the descriptors move.


Should be fine.

> For me, placing the code in the right location is one of the simplest things to avoid duplicate efforts and improve reusability. Having said that, our near-term reuse of Legal (https://reviews.llvm.org/D45552) change still works even if it lands in Transform. So, I won't insist like a purist. Then, I no longer have a good motivation to split the LoopUtil, but since I don't like to do the same thing twice, I might as well finish the splitting work.

I see. Usually, when we move code, we make sure that the use case for being in a more generic part of the tree has a definite improvement, not just being a logical move.

This is mostly because we want to keep the code that is only used for transforms inside its own directory, even if it's an "analysis" per se.

A strong case for the move would be having another part of the code, not transform, needing that analysis. Once we have one, then move becomes obvious.

> So, at the moment, what we need to settle are 1) file names to use and 2) move to Analysis or keep it under Transform.

We should keep inside Transform for now, until we have non-Transform code using it.

I agree not splitting LoopUtils is probably the simplest way forward. I don't like it as well, but I'd like to avoid code motion that can move again once we have a clear view, making it hard to git bisect changes (especially in a time of VPlan potentially breaking the world. :)

cheers,
--renato


Repository:
  rL LLVM

https://reviews.llvm.org/D45420