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

[Petr Kurapov]

kurapov-peter wrote:

> Does this PR change what Ops `linalg.sotfmax` is decomposed into? As in, do the semantics of `linalg.softmax` change? Looking at [mlir/test/Dialect/Linalg/transform-op-decompose.mlir](https://github.com/llvm/llvm-project/pull/97582/files#diff-4ae355ef381291c83e76cefbeb8de9aea58d99b19969dff635e1ec2c87f2c278), I feel that the main difference was:

The decomposition follows the op description (see LinalgOps.td) and specifies its semantics via the implementation. The implementation ends up generating the semantically the same code as the previous decomposition implementation (with minor deviation as you noted). The transform test demonstrates the change in the generalized code.
 
> Also, what's the difference between "transform-op-decompose.mlir" and "decompose-named-ops.mlir"? Is it just "how" the decomposition is "driven"? (TD vs Pass) Couldn't that be one test instead?

The first one tests the new pass. The second one uses the transform interpreter and the decompose_interface op (which happen to partially rely on the same code now).

> If this claims alignment with e.g. PyTorch (or, more specifically, torch-mlir), shouldn't there be a link to torch-mlir docs/issues/code/test instead?

The IR presented is the IR you get by lowering PyTorch to torch-mlir.

> Are you saying that this PR is changing the semantics of softmax in Linalg?

I'd say it sets it. The implementation follows the op description, so there's no real 'change'.

> > now aggregate ops are decomposed first and then generalized
>
> I am a bit confused, there's `-linalg-decompose-named-ops` and `-linalg-generalize-named-ops` - which one are you referring to?

Decomposition is performed by the newly introduced `-linalg-decompose-named-ops` (as the name suggests). Generalization is done by the default `-linalg-generalize-named-ops`.

> The first one would only decompose and the latter would only generalise, right?

Correct.



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