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

[llvmlistbot at llvm.org]

srcarroll wrote:

@MaheshRavishankar 
>@srcarroll , first up, id suggest you leave IREE out of the discussion. It usually ends up muddling the conversation. I am not going to suggest anything related to what IREE does here (which is something completely different and that we found works best for our stack).

I'm just using it as an example to demonstrate how these changes would affect a downstream user.  I don't care about IREE itself.  Since you aren't giving concrete reasons for why this would "break the world", I'm offering a concrete example of why it doesn't.

>The crux of the difference here is that the current fuseElementwiseOps expects to work on linalg.generic

It's only expected now because that's how it used to be defined. Now the definition will be different.  Definitions change all the time and it's not unreasonable to expect users to adapt.

> The concern I have is that doing this change is effectively combining two fairly substantial transformations, (1) generalization of named ops, and (2) elementwise op fusion into one

I already explained [here](https://github.com/llvm/llvm-project/pull/144922#issuecomment-3491857788) why that's wrong.

> I am not opposed to having such a functionality, but this needs to be opt-in, since it is two steps combined into one. There are ways we could make this an opt-in, but as structured, this patch does not allow that

Yes it does. The `fuseElementwiseOps ` function is already is opt-in on a per op basis since you choose whether you call it or not with some `Operation`s `OpOperand`.  If you don't want to fuse a named op then don't call this function when an `OpOperand`s owner is a named op. It's that simple.  And as explained, just define the control function for to opt in to whatever you want with `populateElementwiseOpsFusionPatterns`.  So, yes, with these changes as is you have options and can still retain previous functionality if desired.  No separate entry point necessary.

As far as I am concerned, I've shown why your concerns are wrong.  I haven't heard any convincing arguments from you yet.  All I've heard are claims backed up by nothing.  I think we need input from others. @ftynse @rengolin @Groverkss @javedabsar1 @adam-smnk 







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