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

[llvmlistbot at llvm.org]

srcarroll wrote:

> I'm fine with the change, but do we really have a need for this?
@hanhanW To be honest, I personally don't need it. this was just a follow up to the discussion in this PR https://github.com/llvm/llvm-project/pull/76003 . Which I also don't personally need. 

> 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.
seems reasonable to me. by the time i started touching this code there was already a precedence to detect UB, so I was just trying to finish that off

> 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.

if i'm not mistaken, the proposed changes should cover all the cases

> We would probably be better off with an "in_bounds" style attribute for this.
I'm not opposed to that, but honestly don't know how that would work yet.

> I wonder what is the value of requiring an actual value at run time. IIUC, the padding is just the remainder of the division of the run time dimension by the run time tiling factor, on the last tile. Anything else would make no sense. Requiring users/lowering to add a mod op for the padding value seems redundant. Just a flag to pad to the nearest multiple on the last tile would do, probably.

you are referring to the padding size, not the padding value (the value filling the padded regions). A padding value is always required when padding is needed. and padding is needed when the the tile size doesn't divide the input.



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