[Mlir-commits] [mlir] [mlir][scf] Extend consumer fusion to multiple tilable users (PR #111955)

[llvmlistbot at llvm.org]

Yun-Fly wrote:

Thanks for your time! This formalization is fairly constructive and makes the solution more clear. I have some question before reimplementation.

> The issue is that
> 
> ```
> ... = firstUserOfLoop
> 
> ... = defnOfConsumer
> ```
> 
> During transformation, the loop is moved just before the `defnOfConsumer` (note that it is unnecessary to say "lastDefOfConsumer", there is only one def). and this movement causes use-def chain violation.

As for statement `there is only one def`, don't we need to check each defineOp of all operands of consumerOp? Please correct me if I have misunderstood..

> 2. Compute a backward slice of `defnOfConsumer` (with the op included) but cut the slice to not include any operation that `firstUserOfLoop` already dominates.
> 4. Move the slice computed (including `defnOfConsumer` before `firstUserOfLoop`.

I am sorry that I need to figure out what `defnOfConsumer` exactly represents here before grasping the details here.


> 1. Check that `firstUserOfLoop` and `defnOfConsumer` are in the same block

There exists one corner case. As I have already commented in the current change, it is possible that one of defOfConsumer is outside the block of `firstUserOfLoop`. E.g.

```
%a = ...
{
%loopOp = scf.for
....
%b = consumerOp ins(%loopOp, %a)
}
```
where `%a` dominates the block of `firstUserOfLoop` ==> it must dominate `firstUserOfLoop` ==> we can skip subsequent check instead of bailing.

> 3. The first user of loop should not be in the slice computed. If it does, then bail/look for next consumer.

Based on current consumer dispatching i.e. which one of multiple consumers would be fused. it would always grab the first user among the use list. So, could we resort the use list order by `shuffleUseList` to put the next consumer ahead?

BTW, during the transformation, the order of operations generated before fusion maybe changed. I am afraid if developer have some strong assumption or expectation on these order.  How about add another option like `bool intrusive`(or any other name) to control this behavior. If the user does not want to modify any order caused by fusion, we just bail rather than move those def.

Looking forward your opinions, thanks!





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