[PATCH] D133985: [mlir][transform] Add multi-buffering to the transform dialect

[Alex Zinenko via Phabricator via llvm-commits]

ftynse accepted this revision.
ftynse added a comment.
This revision is now accepted and ready to land.
Herald added a reviewer: nicolasvasilache.

LGTM with comments addressed.



================
Comment at: llvm-project/mlir/include/mlir/Dialect/MemRef/Transforms/Passes.h:60
 /// on the temporary allocation between consecutive loop iterations.
-/// It return success if the allocation was multi-buffered and returns failure()
-/// otherwise.
+/// It return the new allocation if the original allocation was multi-buffered
+/// and returns failure() otherwise.
----------------
typo


================
Comment at: llvm-project/mlir/lib/Dialect/MemRef/TransformOps/MemRefTransformOps.cpp:49
+  void init() {
+    declareDependentDialect<pdl::PDLDialect>();
+
----------------
This also needs to declare that it generates the affine dialect so it is loaded appropriately by passes. I checked the implementation of multibuffering and it seems to only generate memref and affine, but in case I missed something, this needs to declare as generated any dialect that the transform application may produce except for the dialect that contains the target payload IR op. This is similar to how passes must declare "dependent" dialects, and is practically used for that purpose.


================
Comment at: llvm-project/mlir/test/Dialect/MemRef/transform-ops.mlir:4
+// CHECK-DAG: #[[$MAP0:.*]] = affine_map<(d0, d1, d2) -> (((d0 - d1) floordiv d2) mod 2)>
+// CHECK-DAG: #[[$MAP1:.*]] = affine_map<(d0)[s0] -> (d0 + s0)>
+
----------------
Nit: I landed the diff that changes the default subview behavior to use the strided form instead of this, make sure to rebase before landing.


================
Comment at: llvm-project/mlir/test/Dialect/MemRef/transform-ops.mlir:31
+
+transform.with_pdl_patterns {
+^bb0(%arg0: !pdl.operation):
----------------
There is no need to wrap the transform snippet in `with_pdl_patterns` if it does not contain pdl patterns.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133985/new/

https://reviews.llvm.org/D133985