[Mlir-commits] [mlir] [mlir][linalg] Genericize MapOp (PR #162742)

[llvmlistbot at llvm.org]

srcarroll wrote:

> > > A high level question - instead of changing linalg.map definition, would it not be less ruffling if instead you extended linalg-morph-ops with a pattern to rewrite linalg.map into generic?
> > 
> > 
> > In my opinion what you are suggesting is even more "ruffling", and I actually disagree this is ruffling anything anyway. These changes don't change the definition of map op. Nor does it have a negative impact on anything else as far as I can tell. A better question would be why have this specialization for the bbargs in the map op when all it does is make it harder to apply generic patterns that already exist? I'd like to hear one good reason to keep this specialization as is.
> 
> It was just a suggestion - I am not married to that approach. I am happy with your change if @matthias-springer is too.
> 
> Also maybe what I was suggesting is mis-understood. So let me rephrase - changing the op definition -- although in your change its not the op-definition but its lowering via body-- often meets resistance from folks because of impact on their pipeline. So what I was proposing was a e.g. `populateLinalgMapToGenericPattern` rewrite that takes a `linalg.map` and converts it to one or more core ops (incl. linalg-generic). `linalg.map` I have found it to be quite an odd/outlier op as other better supported ops could have been used in its place. But front-ends may generate linalg.map and so it cannot be removed easily.
> 
> I

As far as I can tell, the only affect on any pipeline these changes would have is whether `linalg.map` is converted to `linalg.generic` or not via `linalg-generalize-named-ops`.  

Any frontend that is targeting the linalg dialect for conversion can choose to use `linalg.map` or not. If they have already chosen to use `linalg.map` then nothing will change for them after these changes because neither the definition nor the builder have changed.

Now on the topic of converting to `linalg.generic`.  I can't imagine why that would be a problem for anyone.  The resulting loops will be exactly the same whether `linalg.map` is converted straight to loops or to `linalg.generic` then loops.  Anyone can choose to convert `linalg.map` to anything else before using `linalg-generalize-named-ops`.  So I'm just not seeing a situation where this change is limiting choices.

If there is some reason beyond my understanding to avoid converting to `linalg.generic`, then I personally think the best solution for that is to add an option, say `exclude-ops`, in `linalg-generalize-named-ops` that allows people to specify which ops they don't want to convert.

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