[Mlir-commits] [mlir] [MLIR][tensor] Improve `tensor.pack` verifier to catch more cases with unconditional runtime errors (PR #77217)

[Nicolas Vasilache]

nicolasvasilache wrote:

My argument goes this way:
- we can remove UB by always requiring the padding value to be specified. It generally seems desirable to me to avoid needlessly introducing UB and special runtime cases.
- in the static case, when we have enough info, we can relax this to omit the padding value iff we can statically prove what we need to.

Ultimately, this smells like the "in_bounds" attribute that LLVM has on load/store and that we also added to vector.transfer_read/write.

The situation seems close enough to me to justify a similar attribute addition.

```
 I'm fine with the change, but do we really have a need for this?

%0 = tensor.pack %input inner_dims_pos = [1, 0] inner_tiles = [%tile_size_0, %tile_size_1] into %output : tensor<256x128xf32> -> tensor<10x8x?x?xf32>
```

Not planning to use it myself right now but I can see how a transformation guaranteeing that a dynamic tile size will not result in padding seems pretty useful. We would probably be better off with an "in_bounds" style attribute for this.

```
FYI that the changes about requirePaddingValue will break all the downstream projects that use the method. 
```

This has never been, and continues to not be, a factor for upstream MLIR. If it is the right thing to do we should, if not that's ok. Arguing design positions based on breaking downstream projects is dangerous: it gives the wrong incentives as it pigeonholes evolution.


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