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

[llvmlistbot at llvm.org]

https://github.com/Max191 requested changes to this pull request.

> This PR restricts GeneralizeOuterUnitDimsPackOpPattern to follow its
intended purpose (as per the documentation),

Does this PR add this restriction in the matcher for the pattern? I didn't see any update to the matching logic.

> There was one in-tree test that violated this assumption (and happened
to work) – see @simple_KCRS_to_KRSCsr in
"generalize-tensor-pack.mlir". That test has been updated to satisfy the
new requirements of the pattern.

This test has a non-unit outer dimension, but that dimension is not packed (no inner_tile for it). I think this may actually be intended behavior, since the pattern checks that `packOp.getTiledOuterDims()` are all 1. Maybe the comments are just misleading, and this case is meant to be supported.

I'm not sure what the motivation of this pattern was to begin with, so I can't say if this type of case needs to be supported, but I would be wary of removing that functionality without hearing from whoever wrote this pattern. Looks like @qedawkins added this test, so he may have better context.

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