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

[Ege Beysel]

egebeysel wrote:

I've read through the discussion here, and first of all, thank you for the detailed examples, grounded arguments and really taking the time to clearly state the problem and your motivation.


Though, I happen to agree with Hanhan on this. From my point of view, the problem that this PR is addressing is rather a _practical_ one that though changes the _semantics_ of the operation, and it is bringing us to an ambiguous state:

> My thinking was that extra pad tiles would be allowed 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 free of extra padding tiles, and thus they could be tiled as in the existing TileAndFuse implementation.
Do you think this would solve the issue?

After this, the semantics of the operation itself becomes two-fold: static shapes are allowed to have extra padding, but dynamic shapes do not. I believe in this case, extending the semantics in favor of practicality is not feasible and would rather be avoided.

Although your use-case is totally valid, and I would definitely be in favor of having tests upstream that validate that the `pad + pack` representation works as intended. 

One thing that I want to note, and please correct me on this one if I'm wrong, is that the `extract_slice + unpack` case, although it represents the extra padded case, cannot be fused into it's producer loop at the moment. We can possibly implement that case and make sure it works both ways.

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