[Mlir-commits] [mlir] [mlir][tensor] Generalize/restrict `GeneralizeOuterUnitDimsPackOpPattern` (PR #114315)

[Andrzej Warzyński]

banach-space wrote:

> I believe this is a case where the term outer is slightly unclear

Indeed. In fact, I've implemented #109642 so that we can be clearer about the intent in the future. But this logic predates that PR.

> I don't quite follow how exclusion of non-unit untiled outer dimensions simplifies shape calculation logic here (I suspect it could have been more a matter of the previous implementation being more convoluted than necessary).

Yes, it's rather convoluted and also incomplete (hence this PR). And then there's [getPackOpSourceOrPaddedSource](https://github.com/llvm/llvm-project/blob/32473864cb4631780095e25a0378663b98a11188/mlir/lib/Dialect/Linalg/Transforms/Transforms.cpp#L1019-L1081) that _does_ require _all outer dims_ to be unit  - at least when the pad value is specified. This specifcially makes me believe that this is neither needed nor used?

As for simplifying calculation, if we do allow non-unit untiled dims, then we need to track 3 sets of dims (inner, outer tiled, outer untiled). Sure, that can be done, but given there are other issues here ...

I am happy to restore that functionality, but if it's not really required by anyone (that part is still unclear to me), then keeping things simple might be to our overall benefit.

What are your thoughts?

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