[Mlir-commits] [mlir] 6a3312f - [mlir][openacc] destroy region on firstprivate.recipe is optional

[Valentin Clement]

Author: Valentin Clement
Date: 2023-05-24T07:58:06-07:00
New Revision: 6a3312f3269ca57d94542b8f65b67e4b23e84533

URL: https://github.com/llvm/llvm-project/commit/6a3312f3269ca57d94542b8f65b67e4b23e84533
DIFF: https://github.com/llvm/llvm-project/commit/6a3312f3269ca57d94542b8f65b67e4b23e84533.diff

LOG: [mlir][openacc] destroy region on firstprivate.recipe is optional

The destroy region is optional but the verifier was enforcing it.
Update the verifier and make it clear in the definition.

Reviewed By: razvanlupusoru

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

Added: 
    

Modified: 
    mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
    mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
    mlir/test/Dialect/OpenACC/ops.mlir

Removed: 
    


################################################################################
diff  --git a/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td b/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
index 315b945ce5fde..2edc8bb969af1 100644
--- a/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
+++ b/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
@@ -481,7 +481,8 @@ def OpenACC_FirstprivateRecipeOp : OpenACC_Op<"firstprivate.recipe",
          created private value. It takes the initial value and the privatized
          value as arguments.
       3. The destroy region specifies how to destruct the value when it reaches
-         its end of life. It takes the privatized value as argument.
+         its end of life. It takes the privatized value as argument. It is
+         optional.
 
     A single privatization recipe can be used for multiple operand if they have
     the same type and do not require a specific default initialization.

diff  --git a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
index 992317628d87f..ac33c3a84e00b 100644
--- a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
+++ b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
@@ -398,6 +398,9 @@ LogicalResult acc::FirstprivateRecipeOp::verifyRegions() {
     return emitOpError() << "expects copy region with two arguments of the "
                             "privatization type";
 
+  if (getDestroyRegion().empty())
+    return success();
+
   if (failed(verifyInitLikeSingleArgRegion(*this, getDestroyRegion(),
                                            "privatization", "destroy",
                                            getType(), /*verifyYield=*/false)))

diff  --git a/mlir/test/Dialect/OpenACC/ops.mlir b/mlir/test/Dialect/OpenACC/ops.mlir
index 407b2d9f4d84f..032e09dc4e3e1 100644
--- a/mlir/test/Dialect/OpenACC/ops.mlir
+++ b/mlir/test/Dialect/OpenACC/ops.mlir
@@ -510,6 +510,31 @@ acc.private.recipe @privatization_memref_10_10_f32 : memref<10x10xf32> init {
   acc.terminator
 }
 
+acc.firstprivate.recipe @privatization_memref_10xf32 : memref<10xf32> init {
+^bb0(%arg0: memref<10xf32>):
+  %0 = memref.alloc() : memref<10xf32>
+  acc.yield %0 : memref<10xf32>
+} copy {
+^bb0(%arg0: memref<10xf32>, %arg1: memref<10xf32>):
+  acc.terminator
+} destroy {
+^bb0(%arg0: memref<10xf32>):
+  memref.dealloc %arg0 : memref<10xf32> 
+  acc.terminator
+}
+
+// Test optional destroy region
+acc.firstprivate.recipe @privatization_memref_20xf32 : memref<20xf32> init {
+^bb0(%arg0: memref<20xf32>):
+  %0 = memref.alloc() : memref<20xf32>
+  acc.yield %0 : memref<20xf32>
+} copy {
+^bb0(%arg0: memref<20xf32>, %arg1: memref<20xf32>):
+  acc.terminator
+}
+
+// CHECK-LABEL: acc.firstprivate.recipe @privatization_memref_20xf32 : memref<20xf32> init
+
 func.func @testserialop(%a: memref<10xf32>, %b: memref<10xf32>, %c: memref<10x10xf32>) -> () {
   %i64value = arith.constant 1 : i64
   %i32value = arith.constant 1 : i32