[Mlir-commits] [mlir] [mlir][Tensor] Retain discardable attrs in pack(cast) folder (PR #115772)

[Andrzej Warzyński]

banach-space wrote:

I see this as a follow-up to the recent change I made in #114559.

>From what I can tell, before my change, the behavior introduced **explicitly** here was already present **implicitly** via the [clone](https://github.com/llvm/llvm-project/blob/6fe7ad8be35d054f3ba8437979b01b0aba3abd0e/mlir/include/mlir/Dialect/Utils/StructuredOpsUtils.h#L139-L148) method, which preserved these attributes (*). So, is this PR effectively restoring the previous behaviour?

To me, this feels like a temporary solution rather than a scalable one. I agree with others that preserving discardable attributes seems somewhat ad hoc. My recent PR may have uncovered this issue, and I understand that this approach is a quick patch. Sometimes, this kind of fix is indeed what’s needed to unblock people while a sounder approach is discussed.

> I think the more important thing here is that this doesn't change the semantics of the transformation one bit from upstreams perspective.

Sure, the Op transformation seems fine. But what about the attribute itself? Preserving it happens to be OK for the particular attribute in your use case (I don't know what that is), but may not make sense for other attributes that somebody else may want to attach.

(*) My understanding is that it clones "inherent" rather than "discardable" attributes, but the [underlying storage](https://github.com/llvm/llvm-project/blob/91e134ad7d162c9affe37c67afb9dec34a215b7a/mlir/include/mlir/IR/Operation.h#L1065-L1066) seems to be shared between the two?

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