[Mlir-commits] [mlir] [mlir][vector] Refactor vector-transfer-flatten.mlir (nfc) (1/n) (PR #95743)

Andrzej Warzyński llvmlistbot at llvm.org
Wed Jun 19 08:40:51 PDT 2024


================
@@ -568,6 +568,7 @@ namespace {
 /// memref.collapse_shape on the source so that the resulting
 /// vector.transfer_read has a 1D source. Requires the source shape to be
 /// already reduced i.e. without unit dims.
+///
----------------
banach-space wrote:

In principle:
* one patch == one change,
* avoid unrelated changes.

In this patch, I am violating these rules. First, I am making 2 changes:
* Refactoring vector-transfer-flatten.mlir
* Adding small NFCs in [VectorTransferOpTransforms.cpp](https://github.com/llvm/llvm-project/pull/95743/files#diff-9e617f6fb3ac9281831590661eb2f7e69d0c32936f11f02151f229ab5257b78b)

Second, this particular change qualifies as "unrelated". So, if we were to go by the book, I should split it into a separate PR. I am happy to do that, but I am also mindful that I'm generating a lot of PR traffic and want to reduce noise 😅 

In situation like this, I try to make the intent clear in the summary:

> Finally, changes in "VectorTransferOpTransforms.cpp" are merely meant to
> unify comments and logic between
> 
> FlattenContiguousRowMajorTransferWritePattern and
> FlattenContiguousRowMajorTransferReadPattern.

... and then follow the reviewers recommendation. If I was reviewing this, I'd say "move to a different patch - I'll happily review that" ;-)

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


More information about the Mlir-commits mailing list