[PATCH] D75965: [mlir] Add a simplifying wrapper for generateCopy and expose it.
Uday Bondhugula via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 11 13:02:07 PDT 2020
bondhugula accepted this revision.
bondhugula added inline comments.
This revision is now accepted and ready to land.
================
Comment at: mlir/include/mlir/Transforms/LoopUtils.h:201
+
+/// generateCopyFromMemRefRegion is similar to affineDataCopyGenerate, but with
+/// some simplifications:
----------------
bondhugula wrote:
> generateCopyFromMemRefRegion -> generateCopyForMemRegion ?
This doc comment appears to provides detail before the actual information. Perhaps just rephrase to:
/// Similar to affineDataCopyGenerate, but works on a single memref region.
You could move "The logic of "find ..." as a comment on the definition of the method.
================
Comment at: mlir/include/mlir/Transforms/LoopUtils.h:206
+/// associated loops/DMAs.
+/// * It processes a single memref denoted by `memrefRegion`.
+/// * The prologue and epilogue always surround `insertion_point`.
----------------
-> a single memref region denoted ...
================
Comment at: mlir/include/mlir/Transforms/LoopUtils.h:212
+///
+/// Also note that certain options in `copyOptions` isn't looked at anymore,
+/// like slowMemorySpace.
----------------
Nit: isn't -> aren't
================
Comment at: mlir/test/Transforms/affine-data-copy.mlir:143
// CHECK-SMALL-LABEL: func @foo
// FILTER-LABEL: func @foo
func @foo(%arg0: memref<1024x1024xf32>, %arg1: memref<1024x1024xf32>, %arg2: memref<1024x1024xf32>) -> memref<1024x1024xf32> {
----------------
// MEMREF-REGION-LABEL
here to be safe.
================
Comment at: mlir/test/Transforms/affine-data-copy.mlir:202
// FILTER-NOT: dealloc
+
+// MEMREF_REGION: alloc() : memref<1024x1024xf32>
----------------
Perhaps a comment here on what is being checked for.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75965/new/
https://reviews.llvm.org/D75965
More information about the llvm-commits
mailing list