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

Andrzej Warzyński llvmlistbot at llvm.org
Tue Jul 9 01:44:52 PDT 2024


banach-space wrote:

> The implementation ends up generating the semantically the same code as the previous decomposition implementation (with minor deviation as you noted).

IMO this is key  - please add that in the summary.

> 
> > 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).

>From what I can tell, both tests verify the decomposition of `linalg.softmax`:
```mlir
func.func @softmax(%arg0: tensor<2x16x32xf32>, %dst: tensor<2x16x32xf32>) -> tensor<2x16x32xf32> {
  %1 = linalg.softmax dimension(2) ins(%arg0 : tensor<2x16x32xf32>) outs(%dst: tensor<2x16x32xf32>) -> tensor<2x16x32xf32>
  return %1 : tensor<2x16x32xf32>
}
```

Couldn't we re-use the input and the `CHECK` lines? To avoid duplication.

> 
> > 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.

I know where the IR is coming from.

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?

I'm trying to understand the motivation here and the overall design that we are converging towards.

> 
> > 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'.
> 

Key info, please highlight in the summary.

> > > 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`.

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?

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


More information about the Mlir-commits mailing list