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

[Renato Golin]

rengolin wrote:

> Not everyone goes down named ops for all elementwise operations. Most common use case is that you use named ops for compute ops like matmul/convs/etc. and use generic ops for elementwise operations.

I'd avoid assuming user intention/behaviour. While this may be true for some, it's not true for everyone. We need to make decisions that are consistent with the dialect, not its current use. (I'm not disagreeing with you, just making clear that usage is not the only proxy for dialect design).

> That being said, since all contributors work on MLIR because they use it downstream, I think it is a common courtesy to minimize extra workload for everyone by staging the change when reasonable. The upstream committer may have to extend extra effort, but they will ultimately save time if everyone else does the same.

That's not a reasonable upstream path. You can _ask_ people to help you, and they can agree to help you, but ultimately, your downstream cost must be paid by your team. Other downstream projects already pay their own costs, and it's unreasonable to ask them to pay yours, too. This is a key assumption of the whole LLVM project since its inception and I don't see it changing now.

> It is also a matter of encouraging upstream evolution and maintenance: having to jump through too many hoops because of just "fears" of causing downstream to update some APIs is discouraging upstream contributors, in particular those who aren't necessarily paid to work upstream. Reducing the overhead/friction to improve the codebase is critical, and the willingness to break APIs (with reasons) is part of it.

I would not put it quite like @joker-eph above, but the sentiment stands: forcing people to appease to multiple downstream constraints makes it very hard to contribute to the project. This is highly discouraging and conter-productive.

> @rengolin Here's another idea. Maybe to start i could just modify the controlFn for `linalg-fuse-elementwise-ops` pass specifically to only allow `linalg.generic`. This would keep the current behavior for now, while also giving people more options with the pattern, so everyone would win. Then we can have more time think about if `linalg-fuse-elementwise-ops` should change.

Adding the `generic` patterns would be a low-key change and potentially reasonable solution to the conundrum. Remember, Linalg has non-generic operations where fusion doesn't work / makes no sense, so we need to be careful.

However, my concern still stands: if the `generic` has reduction/transpose/broadcast patterns, then we need to make sure that the fuser can handle them. The current named ops (`add`, `mul`) mandate an element-wise affine map, so they would be absolutely fine, but `generic`, `contract` and `elementwise` do allow for arbitrary affine maps, and that could be an issue.

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