[Mlir-commits] [mlir] [mlir][tensor] Preserve encoding in more canonicalizers (concat, reshape, pad) (PR #207241)

[Dmitrii Makarenko]

Devjiu wrote:

@joker-eph 

> I'm not sure what point you're trying to make actually?

Let me make it concrete with the actual downstream case, since that's where "verify() passes" and "semantically correct" diverge.

Our downstream stashes tensor bounds in the encoding - used later by bufferization to compute allocation sizes for dynamic dims. Before canonicalization:
```mlir
  %r = tensor.insert_slice %src into %dst[0, 0, 0, 0] [1, %sz1, %c16, 32] [1, 1, 1, 1]
      : tensor<1x?x?x32xf16, {bounds=[1, 8, 16, 32]}> into tensor<1x1280x32x32xf16>
```
after canonicalization today
```mlir
  %cast = tensor.cast %src : tensor<1x?x?x32xf16, {bounds=[1, 8, 16, 32]}> to tensor<1x?x16x32xf16>
  %r = tensor.insert_slice %cast into %dst[0, 0, 0, 0] [1, %sz1, %c16, 32] [1, 1, 1, 1]
      : tensor<1x?x16x32xf16> into tensor<1x1280x32x32xf16>
```
`%cast`'s result type has no encoding — the bounds are gone. `RankedTensorType::verify` has nothing to say about this: it's a well-formed type either way, an absent attribute isn't invalid IR by any check we have. But downstream, bufferization has no more bounds to compute the allocation size for the dynamic dim, and either falls back to an unbounded/incorrect allocation or fails deep inside a pass that has no idea a canonicalizer upstream silently dropped the information it needed.


>>I don't think "OK to drop when the encoding says it no longer holds" implies "OK to drop unconditionally." Those are different decision procedures with very different odds of throwing away information.

>I don't quite see why actually. I mentioned an example why this seems completely arbitrary to me and not robust.

This example is exactly why: dropping was never conditioned on anything - `{bounds=...}` doesn't implement `VerifiableTensorEncoding`, it never gets asked whether it still holds, it's just discarded because the pattern
re-derives the type from a template that doesn't carry it. The shape change here is dyn -> static, the bounds are still perfectly valid for that shape, and nothing about them became stale, yet they're dropped anyway. 

`insert_slice`/`extract_slice` are also not some rarely-used op pair — they're core to the tiling mechanism/interface that a lot of downstream lowering pipelines are built on. Silently erasing arbitrary encoding metadata in such a central, generic op feels like the wrong default, especially since different downstreams attach very different (and mutually opaque) meaning
to that attribute.

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