[Mlir-commits] [mlir] [mlir][affine] Preserve used sibling loop results during fusion (PR #216564)

[Kunal Dubey]

xakep8 wrote:

> Left a couple of comments, but didn't read all of the PR thoroughly.
> 
> I think this it is excessively complicated and mixes several things together. Specifically, it should just clone the body of the second loop into the first and append its iter_args/results. Other transformations should and likely will take care of removing iter_args/results that became dead, it's not a concern for fusion itself. If not, we should have a separate transformation or pattern for that, in a separate PR.
> 
> Similarly, I don't quite see the need to reason about the "promoted slice". Just clone the whole body with an IRMapping, or `replaceAllUsesIn` a specific region and have DCE/canonicalizers do the rest.
> 
> The dominance logic is also excessively complex. If we are fusing a loop that comes later in a block into a loop that comes earlier, we should check that all `iter_args` of the later loop are dominating the earlier loop (DominanceInfo should be sufficient). If we are going in opposite direction, we should check that the later loop domiates all uses of the results of the earlier loop. If there is a strict deterministic order in which we process candidates, e.g. we only consider fusing an earlier loop into a later loop and not, one check is sufficient. There shouldn't be a need to compute a slice, it's expensive and wasteful here.

we need dominance logic because the merging is top down not bottom up, so a src loop is a loop above the definition of dst loop, if the approach was to be reverse then it makes sense to ignore dominance entirely.

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