[Mlir-commits] [mlir] [mlir][linalg] Fix `linalg.matmul_transpose_a` def. (PR #97690)

Jerry Shih llvmlistbot at llvm.org
Sun Jul 7 19:42:02 PDT 2024


JerryShih wrote:

> > The `matmul_transpose_a` input data format should be `KxM * KxN`.
> 
> That's already the case, isn't it? IIUC, this PR is merely changing the variable names in the YAML spec file. Making all definitions use consistent naming is definitely desirable, but the suggested change looks also inconsistent with other example.

Here is the `original` matmul_transpose_a def:
```
def matmul_transpose_a(
    A=TensorDef(T1, S.K, S.N),
    B=TensorDef(T2, S.K, S.M),
    C=TensorDef(U, S.M, S.N, output=True),
    cast=TypeFnAttrDef(default=TypeFn.cast_signed),
):
```
It uses KxN for matrix A and KxM for matrix B.

And please check the `batch_matmul_transpose_a` and `matmul` def:
```
@linalg_structured_op
def matmul(
    A=TensorDef(T1, S.M, S.K),
    B=TensorDef(T2, S.K, S.N),
    C=TensorDef(U, S.M, S.N, output=True),
    cast=TypeFnAttrDef(default=TypeFn.cast_signed),
):

@linalg_structured_op
def batch_matmul_transpose_a(
    A=TensorDef(T1, Batch, S.K, S.M),
    B=TensorDef(T2, Batch, S.K, S.N),
    C=TensorDef(U, Batch, S.M, S.N, output=True),
):
```

The current matmul_transpose_a def is inconsistent with other ops def.
And the matmul_transpose_a's output is MxN.
>From the current matmul_transpose_a def, the output shape should be:
`A^T*B = NxK * KxM = NxM`

> See my suggestion inline.

I don't see the inlined suggestion at github? Do we have another review system for llvm project?

> 
> Also, this looks like an NFC change, so please update the name and the summary.

Yes, the input/output's affine_map doesn't used in llvm code-gen tools now. I will update the patch name/summary.


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


More information about the Mlir-commits mailing list