[Mlir-commits] [mlir] 0816b96 - Allow same memory space for SRC and DST of dma_start operations

[Stella Stamenova]

Author: Ian Bearman
Date: 2021-05-14T10:40:15-07:00
New Revision: 0816b96a10b8ec77cc4f663eb209356456e951de

URL: https://github.com/llvm/llvm-project/commit/0816b96a10b8ec77cc4f663eb209356456e951de
DIFF: https://github.com/llvm/llvm-project/commit/0816b96a10b8ec77cc4f663eb209356456e951de.diff

LOG: Allow same memory space for SRC and DST of dma_start operations

    This change allows the SRC and DST of dma_start operations to be located in the
    same memory space. This applies to both the Affine dialect and Memref dialect
    versions of these Ops. The documention has been updated to reflect this by
    explicitly stating overlapping memory locations are not supported (undefined
    behavior).

Reviewed By: bondhugula

Differential Revision: https://reviews.llvm.org/D102274

Added: 
    

Modified: 
    mlir/include/mlir/Dialect/Affine/IR/AffineOps.h
    mlir/include/mlir/Dialect/MemRef/IR/MemRef.h
    mlir/lib/Dialect/Affine/IR/AffineOps.cpp
    mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
    mlir/test/IR/invalid-ops.mlir

Removed: 
    


################################################################################
diff  --git a/mlir/include/mlir/Dialect/Affine/IR/AffineOps.h b/mlir/include/mlir/Dialect/Affine/IR/AffineOps.h
index fc4c49b05a98a..a614ec61c0b30 100644
--- a/mlir/include/mlir/Dialect/Affine/IR/AffineOps.h
+++ b/mlir/include/mlir/Dialect/Affine/IR/AffineOps.h
@@ -47,7 +47,8 @@ bool isTopLevelValue(Value value);
 /// id), transferring chunks of number_of_elements_per_stride every stride until
 /// %num_elements are transferred. Either both or no stride arguments should be
 /// specified. The value of 'num_elements' must be a multiple of
-/// 'number_of_elements_per_stride'.
+/// 'number_of_elements_per_stride'. If the source and destination locations
+/// overlap the behavior of this operation is not defined.
 //
 // For example, an AffineDmaStartOp operation that transfers 256 elements of a
 // memref '%src' in memory space 0 at indices [%i + 3, %j] to memref '%dst' in

diff  --git a/mlir/include/mlir/Dialect/MemRef/IR/MemRef.h b/mlir/include/mlir/Dialect/MemRef/IR/MemRef.h
index 0542423977835..13d27466c2719 100644
--- a/mlir/include/mlir/Dialect/MemRef/IR/MemRef.h
+++ b/mlir/include/mlir/Dialect/MemRef/IR/MemRef.h
@@ -62,7 +62,9 @@ namespace memref {
 // 'index' type, and specify a stride for the slower memory space (memory space
 // with a lower memory space id), transferring chunks of
 // number_of_elements_per_stride every stride until %num_elements are
-// transferred. Either both or no stride arguments should be specified.
+// transferred. Either both or no stride arguments should be specified. If the
+// source and destination locations overlap the behavior of this operation is
+// not defined.
 //
 // For example, a DmaStartOp operation that transfers 256 elements of a memref
 // '%src' in memory space 0 at indices [%i, %j] to memref '%dst' in memory space

diff  --git a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
index 62e1a66353458..6063bb5d835de 100644
--- a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
+++ b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
@@ -1078,10 +1078,6 @@ LogicalResult AffineDmaStartOp::verify() {
   if (!getOperand(getTagMemRefOperandIndex()).getType().isa<MemRefType>())
     return emitOpError("expected DMA tag to be of memref type");
 
-  // DMAs from 
diff erent memory spaces supported.
-  if (getSrcMemorySpace() == getDstMemorySpace()) {
-    return emitOpError("DMA should be between 
diff erent memory spaces");
-  }
   unsigned numInputsAllMaps = getSrcMap().getNumInputs() +
                               getDstMap().getNumInputs() +
                               getTagMap().getNumInputs();

diff  --git a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
index 9a502780134a6..8125c053e3945 100644
--- a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
+++ b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
@@ -943,10 +943,6 @@ LogicalResult DmaStartOp::verify() {
                     [](Type t) { return t.isIndex(); }))
     return emitOpError("expected tag indices to be of index type");
 
-  // DMAs from 
diff erent memory spaces supported.
-  if (getSrcMemorySpace() == getDstMemorySpace())
-    return emitOpError("DMA should be between 
diff erent memory spaces");
-
   // Optional stride-related operands must be either both present or both
   // absent.
   if (numOperands != numExpectedOperands &&

diff  --git a/mlir/test/IR/invalid-ops.mlir b/mlir/test/IR/invalid-ops.mlir
index a3240704f4310..145566ce4c5dd 100644
--- a/mlir/test/IR/invalid-ops.mlir
+++ b/mlir/test/IR/invalid-ops.mlir
@@ -388,15 +388,6 @@ func @dma_start_dst_index_wrong_type(
 
 // -----
 
-func @dma_start_same_space(
-    %src: memref<2x2xf32>, %idx: index, %dst: memref<2xf32>,
-    %tag: memref<i32,2>) {
-  // expected-error at +1 {{DMA should be between 
diff erent memory spaces}}
-  memref.dma_start %src[%idx, %idx], %dst[%idx], %idx, %tag[] : memref<2x2xf32>, memref<2xf32>, memref<i32,2>
-}
-
-// -----
-
 func @dma_start_too_many_operands(
     %src: memref<2x2xf32>, %idx: index, %dst: memref<2xf32,1>,
     %tag: memref<i32,2>) {