[Mlir-commits] [mlir] [mlir][linalg] Allow extra pad tiles in linalg.pack & unpack (PR #189049)

[llvmlistbot at llvm.org]

fabrizio-indirli wrote:

Thanks for taking another look at this!

> Under the new semantics, the dynamic dest can legally have extras at runtime, so fusion has no static way to decide. The static bailout you added in PackOpTiling already acknowledges fusion can't represent pure-padding tiles; the dynamic case has no corresponding bailout and no working analysis.

My thinking was that extra pad tiles would be allows only when the affected dimensions (both in the source and result tensors) are **known statically**. So, in your example, the dynamic dims could be considered to be without extra padding tiles, and thus they could be tiled as in the existing TileAndFuse implementation.
Do you think this would solve the issue?

However, I appreciate that without the extra attributes it might be less clear to the user that extra padding tiles are not allowed on dynamic dims. To make this clearer and enforce this at the verifier level, we could simply re-introduce the optional `extra-pad_tiles` attribute, and check in the verifier that the involved dims are static. Would this work? 
However, before re-introducing the attribute, could you clarify what you mean in a previous post of yours (on the 1st version of the patch, that still had the explicit attributes) when you said
> The other point is that the PR will make pack/unpack asymmetry, which sounds not good to me.
Did you mean that you'd like to see exactly the same arguments and attributes fields in the two ops `linalg.pack` and `linalg.unpack`? That is already not the case, since `linalg.pack` has the (optional) additional field `padding_value`, so I'm not sure I follow.

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