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

[Dmitrii Makarenko]

Devjiu wrote:

> The "propagate if it does not break the verifier" is a bit weird to me

To be precise: we don't ask a generic/weak verifier, we ask the encoding's own `verifyEncoding` — the predicate its author committed to as the contract for that attribute. Its encoding's own contract, not in our canonicalizer's policy. We're not writing a new check here — we're trusting the one the encoding author already exposed. 

> this seems to acknowledge that it is OK to drop the metadata, so it
> should just always be OK to drop it

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.
Current implementation drops it every time the pattern fires, regardless of whether it's still valid; the proposed policy drops it only when the encoding's own predicate says it no longer applies (and never for opaque encodings, which don't get asked at all).

Concretely for our downstream: we stash bounds/stride metadata in the encoding. After tiling we get a dynamic-shaped tensor whose bounds are still valid and still required by a later lowering. The current unconditional-drop canonicalizer removes that metadata anyway. 
So for our downstream, the current canonicalizer produces "incorrect IR".
That's the "incorrect IR" I care about: not caught by `verify()`, but wrong all the same, and much harder to debug because the
failure surfaces far from the cause.


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