[PATCH] D75965: [mlir] Add a simplifying wrapper for generateCopy and expose it.

[Uday Bondhugula via Phabricator via llvm-commits]

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