[Mlir-commits] [mlir] [MLIR][NFC] Add allow Insert/extract slice option to pack/unpack op (PR #117340)

[Zhuoran Yin]

jerryyin wrote:

@banach-space Thanks for the detailed review and suggestions. It's very helpful to see so much details in your review!

For suggestion 1, please see commit 671829ee1c89b1d5ff82c866f7074a969414698f that adds back the unit test with attributes. Since you insist unit test for this PR is needed, I'm also amending the title of the PR to get rid of `[NFC]` as it touched function interfaces. While I can understand those additional `CHECK` after the `CHECK-NOT` adds the readability of the unit test, I still think those CHECKs are duplicated coverage with previous test cases. I'm following your suggestion anyways for this standalone case that I wouldn't have strong opinion anyways.

For suggestion 2, option 1, see commit 3ad8cd64f687afa5cadc57e40aa228d6a587ed03 for your suggested test case to exercise consumer and producer fusion. I picked Option 1, and created the bare minimal example to illustrate the additional fusion that can be done with new attributes.

Note that I am skipping the comments (Like suggestion 2 main body) around the test cases that already exist before the PR, as I'd like this PR to be focused about the new attributes added, and not distracted with additional refactoring that we may discover. I'd have no problem filing additional PRs dedicated for refactoring though.

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