[Mlir-commits] [mlir] [MLIR][Linalg] Remove/update failing obsolete OpDSL tests for linalg.matmul. (PR #115319)
Renato Golin
llvmlistbot at llvm.org
Tue Nov 12 04:39:19 PST 2024
rengolin wrote:
> It can - there are "binded" builders for every op in every dialect - eg `linalg.MatMulOp` or `linalg.FillOp` etc. The snippet above shows that it can be done (I landed exactly this ["fix" in mlir-python-wheels](https://github.com/makslevental/mlir-python-extras/pull/105)).
If this sounds like the right fix upstream, we should merge it and do the same for the upcoming ops. (@shahidact, @javedabsar, @adam-smnk).
> What makes the current state onerous is that for whatever reason (no doubt due to changes going on in this stream), suddenly you need to explicitly pass the indexing maps even for `linalg.MatMulOp` whereas before you did not. Thus you have to build the `indexing_maps_attr` by [hand](https://github.com/llvm/llvm-project/blob/5dd9867e2d1e698fee980e31da114a37e4c7f612/mlir/python/mlir/dialects/linalg/opdsl/lang/emitter.py#L40-L111) and then call the binded builder.
This was not the intention. The parser accepts plain syntax and builds the affine map by default. We need to find a way for that not to be necessary when the maps are standard. You can keep that logic to support broadcast/transpose maps, though (which is the new thing).
> I highly doubt anyone is using Python to emit `linalg` other than me so I wouldn't call this a catastrophe.
This gives us time to fix it properly.
> I'm not sure there is another path open to the project which has costs that anyone will pay.
If there is, I can't see it either. Neither could anyone else on the original discussion. So let's just all commit to this path and fix all the problems we find along the way.
https://github.com/llvm/llvm-project/pull/115319
More information about the Mlir-commits
mailing list