[Mlir-commits] [mlir] [mlir][linalg] Extend `FuseElementwiseOps` pattern to work with named ops (PR #144922)

[llvmlistbot at llvm.org]

srcarroll wrote:

> > If `matmul` + some other op obey those rules, then it should be fused. If that's undesirable, then the pattern logic should change, but that's not really an issue specific to enabling the pattern to any `LinalgOp`.
> 
> Not directly, I agree, but the current state would limit the damage, if this was really a problem. Widening the scope needs to be done with care. @ftynse @nicolasvasilache, any insight here?
> 
> One way to at least document the intention is to actually add tests for the things that make sense (to work and not to work) in this PR, so that when this breaks something, people have something to go on.

Sure I'll add some more test cases. Is there anything in particular you would like to see, in case I miss something of real concern?  

Also, to be clear, do you think enabling fusion for, say, `linalg.matmul` is a bad thing or it's ok?  I personally think it's a good thing.  However, only in the context of having more options.  With the pattern only defined on `linalg.generic`, that limits my options.  On the other hand, fusing ANY op that satisfies conditions is also limiting choices.  But the nice thing is this pattern allows for custom control logic via the `controlFn` callback.  I dont actually use the builtin passes, rather I make my own pass by reusing the pattern with my own control logic.  So for example, if I dont want to fuse any reduction consumers then I can do
```
    // Don't allow elementwise + reduction fusion
    auto controlFn = [](OpOperand *operand) {
      auto owner = cast<LinalgOp>(operand->getOwner());
      return !llvm::any_of(owner.getIteratorTypesArray(), isReductionIterator);
    };
    RewritePatternSet fusionPatterns(ctx);
    populateElementwiseOpsFusionPatterns(fusionPatterns, controlFn);
 ```
 So, this together with enabling the pattern with `LinalgOp` gives us the most options.

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