From 6a3312f3269ca57d94542b8f65b67e4b23e84533 Mon Sep 17 00:00:00 2001
From: Valentin Clement <clementval@gmail.com>
Date: Wed, 24 May 2023 07:57:38 -0700
Subject: [PATCH] [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
---
 mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td |  3 ++-
 mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp         |  3 +++
 mlir/test/Dialect/OpenACC/ops.mlir              | 25 +++++++++++++++++++++++++
 3 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td b/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
index 315b945..2edc8bb 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 9923176..ac33c3a 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 407b2d9..032e09d 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
-- 
2.7.4