[Mlir-commits] [mlir] [mlir][transform] Use LoopInvariant passes in the tileConsumerAndFuseProducersUsingSCF (PR #163222)

[llvmlistbot at llvm.org]

https://github.com/MaheshRavishankar requested changes to this pull request.

Ok, I looked at this again. I do have concerns with this change. The expectation of tile and fuse is that it creates 
(a) a perfectly nested loop that iterates over the loops
(b) it fused into the inner most loop of the given tiled loops.

This transformation is already fairly complex. Adding hoisting in between here is further adding to the complexity. It makes it much more complex to reason about what the tile and fuse methods do if the code suddenly fuses somewhere unexpected.

Apart from complexity, the hoisting is a "heuristic". Depending on the use case, hoisting might be beneficial, and might not be. That really comes down to a cost model. If we add hoisting, what the rationale for a later change to not do "sinking". The best approach as far as I can see is to keep to the steps (a) and (b) I highlighted above. Any susbsequent hoisting/sinking etc should be done as a post processing after tile and fuse.

https://github.com/llvm/llvm-project/pull/163222