[Mlir-commits] [mlir] [MLIR][Linalg] Add aggregate ops decomposition pass and softmax decom… (PR #97582)

[Renato Golin]

rengolin wrote:

> FWIW (without links to documentation), that IR is just an implementation detail of torch-mlir. In this PR we are discussing an implementation detail of Linalg. Are you saying that the implementation in Linalg should match torch-mlir? Why? What if the implementation in torch-mlir changes?

PyTorch & torch-mlir are red herrings. This PR is about softmax.

The current lowering of softmax into generics is unsurprisingly the same as both PyTorch and Tensorflow expect it to be. This is related but distinc to lowering softmax to named ops. So, let's close this tangent, as it is irrelevant.

> I know what the options are, thanks. To me, your comment implies that `-linalg-decompose-named-ops` is meant to be followed by `-linalg-generalize-named-ops` ("aggregate ops are _decomposed_ first and then _generalized_")? Is that what you had in mind?

Absolutely not.

The test where we lower to named ops was created in this PR. We verify that it does the "right thing".

The old test checks the generic lowering (because it was the only thing we had), so @kurapov-peter added the generalization pass to match the old expectation. This provides us with a clear "apples to apples" comparison, and shows that the old output can no longer be attained. Most importantly, you don't get a mix of named + generics. You either get all named or all generic. 

This seems to be a problem to @MaheshRavishankar and I want to understand it better. My guess is that there are pattern matchers that won't work with the generic version of `fill` (and why we want named ops in the first place).

An option is to make some "special" ops never to generalize, for example `linalg.fill`, by the generalize pattern. Or to have a flag in the generalize pass that does that, but without the option, it converts all. An alternative option is to piece-wise generalize downstream. It depends on how the matcher expects the code to be.

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