[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