[mlir][openacc] Cleanup acc.exit_data from old data clause operands

Since the new data operand operations have been added in D148389 and
adopted on acc.exit_data in D149601, the old clause operands are no longer
needed.

The LegalizeDataOpForLLVMTranslation will become obsolete when all
operations will be cleaned. For the time being only the appropriate
part are being removed.

processOperands will also receive some updates once all the operands
will be coming from an acc data operand operation.

Reviewed By: jeanPerier

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

flang/lib/Lower/OpenACC.cpp

@@ -1368,7 +1368,6 @@ genACCExitDataOp(Fortran::lower::AbstractConverter &converter,
   addOperand(operands, operandSegments, async);
   addOperand(operands, operandSegments, waitDevnum);
   addOperands(operands, operandSegments, waitOperands);
-  operandSegments.append({0, 0, 0});
   addOperands(operands, operandSegments, dataClauseOperands);
 
   mlir::acc::ExitDataOp exitDataOp = createSimpleOp<mlir::acc::ExitDataOp>(

flang/lib/Optimizer/Transforms/OpenACC/OpenACCDataOperandConversion.cpp

@@ -116,7 +116,6 @@ void OpenACCDataOperandConversion::runOnOperation() {
   fir::LLVMTypeConverter converter(
       op.getOperation()->getParentOfType<mlir::ModuleOp>(), true);
   patterns.add<LegalizeDataOpForLLVMTranslation<acc::DataOp>>(converter);
-  patterns.add<LegalizeDataOpForLLVMTranslation<acc::ExitDataOp>>(converter);
   patterns.add<LegalizeDataOpForLLVMTranslation<acc::ParallelOp>>(converter);
 
   ConversionTarget target(*context);
@@ -147,13 +146,6 @@ void OpenACCDataOperandConversion::runOnOperation() {
                allDataOperandsAreConverted(op.getAttachOperands());
       });
 
-  target.addDynamicallyLegalOp<acc::ExitDataOp>(
-      [allDataOperandsAreConverted](acc::ExitDataOp op) {
-        return allDataOperandsAreConverted(op.getCopyoutOperands()) &&
-               allDataOperandsAreConverted(op.getDeleteOperands()) &&
-               allDataOperandsAreConverted(op.getDetachOperands());
-      });
-
   target.addDynamicallyLegalOp<acc::ParallelOp>(
       [allDataOperandsAreConverted](acc::ParallelOp op) {
         return allDataOperandsAreConverted(op.getReductionOperands()) &&

flang/test/Transforms/OpenACC/convert-data-operands-to-llvmir.fir

@@ -30,28 +30,6 @@ fir.global internal @_QFEa : !fir.array<10xf32> {
   fir.has_value %0 : !fir.array<10xf32>
 }
 
-func.func @_QQsub_enter_exit() attributes {fir.bindc_name = "a"} {
-  %0 = fir.address_of(@_QFEa) : !fir.ref<!fir.array<10xf32>>
-  acc.exit_data copyout(%0 : !fir.ref<!fir.array<10xf32>>)
-  return
-}
-
-// CHECK-LABEL: func.func @_QQsub_enter_exit() attributes {fir.bindc_name = "a"} {
-// CHECK: %[[ADDR:.*]] = fir.address_of(@_QFEa) : !fir.ref<!fir.array<10xf32>>
-// CHECK: %[[CAST1:.*]] = builtin.unrealized_conversion_cast %[[ADDR]] : !fir.ref<!fir.array<10xf32>> to !llvm.ptr<array<10 x f32>>
-// CHECK: acc.exit_data copyout(%[[CAST1]] : !llvm.ptr<array<10 x f32>>)
-
-// LLVMIR-LABEL: llvm.func @_QQsub_enter_exit() attributes {fir.bindc_name = "a"} {
-// LLVMIR: %[[ADDR:.*]] = llvm.mlir.addressof @_QFEa : !llvm.ptr<array<10 x f32>>
-// LLVMIR: acc.exit_data copyout(%[[ADDR]] : !llvm.ptr<array<10 x f32>>)
-
-// -----
-
-fir.global internal @_QFEa : !fir.array<10xf32> {
-  %0 = fir.undefined !fir.array<10xf32>
-  fir.has_value %0 : !fir.array<10xf32>
-}
-
 func.func @_QQsub_parallel() attributes {fir.bindc_name = "test"} {
   %0 = fir.address_of(@_QFEa) : !fir.ref<!fir.array<10xf32>>
   %1 = fir.alloca i32 {bindc_name = "i", uniq_name = "_QFEi"}

mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td

@@ -813,9 +813,6 @@ def OpenACC_ExitDataOp : OpenACC_Op<"exit_data", [AttrSizedOperandSegments]> {
                        Optional<IntOrIndex>:$waitDevnum,
                        Variadic<IntOrIndex>:$waitOperands,
                        UnitAttr:$wait,
-                       Variadic<AnyType>:$copyoutOperands,
-                       Variadic<AnyType>:$deleteOperands,
-                       Variadic<AnyType>:$detachOperands,
                        Variadic<OpenACC_PointerLikeTypeInterface>:$dataClauseOperands,
                        UnitAttr:$finalize);
 
@@ -833,9 +830,6 @@ def OpenACC_ExitDataOp : OpenACC_Op<"exit_data", [AttrSizedOperandSegments]> {
       | `async` `(` $asyncOperand `:` type($asyncOperand) `)`
       | `wait_devnum` `(` $waitDevnum `:` type($waitDevnum) `)`
       | `wait` `(` $waitOperands `:` type($waitOperands) `)`
-      | `copyout` `(` $copyoutOperands `:` type($copyoutOperands) `)`
-      | `delete` `(` $deleteOperands `:` type($deleteOperands) `)`
-      | `detach` `(` $detachOperands `:` type($detachOperands) `)`
       | `dataOperands` `(` $dataClauseOperands `:` type($dataClauseOperands) `)`
     )
     attr-dict-with-keyword

mlir/lib/Conversion/OpenACCToLLVM/OpenACCToLLVM.cpp

@@ -156,7 +156,6 @@ class LegalizeDataOpForLLVMTranslation : public ConvertOpToLLVMPattern<Op> {
 void mlir::populateOpenACCToLLVMConversionPatterns(
     LLVMTypeConverter &converter, RewritePatternSet &patterns) {
   patterns.add<LegalizeDataOpForLLVMTranslation<acc::DataOp>>(converter);
-  patterns.add<LegalizeDataOpForLLVMTranslation<acc::ExitDataOp>>(converter);
   patterns.add<LegalizeDataOpForLLVMTranslation<acc::ParallelOp>>(converter);
 }
 
@@ -208,13 +207,6 @@ void ConvertOpenACCToLLVMPass::runOnOperation() {
                allDataOperandsAreConverted(op.getAttachOperands());
       });
 
-  target.addDynamicallyLegalOp<acc::ExitDataOp>(
-      [allDataOperandsAreConverted](acc::ExitDataOp op) {
-        return allDataOperandsAreConverted(op.getCopyoutOperands()) &&
-               allDataOperandsAreConverted(op.getDeleteOperands()) &&
-               allDataOperandsAreConverted(op.getDetachOperands());
-      });
-
   target.addDynamicallyLegalOp<acc::ParallelOp>(
       [allDataOperandsAreConverted](acc::ParallelOp op) {
         return allDataOperandsAreConverted(op.getReductionOperands()) &&

mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp

@@ -542,11 +542,9 @@ LogicalResult acc::ExitDataOp::verify() {
   // 2.6.6. Data Exit Directive restriction
   // At least one copyout, delete, or detach clause must appear on an exit data
   // directive.
-  if (getCopyoutOperands().empty() && getDeleteOperands().empty() &&
-      getDetachOperands().empty() && getDataClauseOperands().empty())
-    return emitError(
-        "at least one operand in copyout, delete or detach must appear on the "
-        "exit data operation");
+  if (getDataClauseOperands().empty())
+    return emitError("at least one operand must be present in dataOperands on "
+                     "the exit data operation");
 
   // The async attribute represent the async clause without value. Therefore the
   // attribute and operand cannot appear at the same time.
@@ -565,8 +563,7 @@ LogicalResult acc::ExitDataOp::verify() {
 }
 
 unsigned ExitDataOp::getNumDataOperands() {
-  return getCopyoutOperands().size() + getDeleteOperands().size() +
-         getDetachOperands().size() + getDataClauseOperands().size();
+  return getDataClauseOperands().size();
 }
 
 Value ExitDataOp::getDataOperand(unsigned i) {

mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp

@@ -202,17 +202,31 @@ processDataOperands(llvm::IRBuilderBase &builder,
 
   unsigned index = 0;
 
+  llvm::SmallVector<mlir::Value> deleteOperands, copyoutOperands;
+  for (mlir::Value dataOp : op.getDataClauseOperands()) {
+    if (auto devicePtrOp = mlir::dyn_cast_or_null<acc::GetDevicePtrOp>(
+            dataOp.getDefiningOp())) {
+      for (auto &u : devicePtrOp.getAccPtr().getUses()) {
+        if (mlir::dyn_cast_or_null<acc::DeleteOp>(u.getOwner()))
+          deleteOperands.push_back(devicePtrOp.getVarPtr());
+        else if (mlir::dyn_cast_or_null<acc::CopyoutOp>(u.getOwner()))
+          copyoutOperands.push_back(devicePtrOp.getVarPtr());
+      }
+    }
+  }
+
+  auto nbTotalOperands = deleteOperands.size() + copyoutOperands.size();
+
   // Delete operands are handled as `delete` call.
-  if (failed(processOperands(builder, moduleTranslation, op,
-                             op.getDeleteOperands(), op.getNumDataOperands(),
-                             kDeleteFlag, flags, names, index, mapperAllocas)))
+  if (failed(processOperands(builder, moduleTranslation, op, deleteOperands,
+                             nbTotalOperands, kDeleteFlag, flags, names, index,
+                             mapperAllocas)))
     return failure();
 
   // Copyout operands are handled as `from` call.
-  if (failed(processOperands(builder, moduleTranslation, op,
-                             op.getCopyoutOperands(), op.getNumDataOperands(),
-                             kHostCopyoutFlag, flags, names, index,
-                             mapperAllocas)))
+  if (failed(processOperands(builder, moduleTranslation, op, copyoutOperands,
+                             nbTotalOperands, kHostCopyoutFlag, flags, names,
+                             index, mapperAllocas)))
     return failure();
 
   return success();
@@ -509,7 +523,8 @@ LogicalResult OpenACCDialectLLVMIRTranslationInterface::convertOperation(
                "unexpected OpenACC terminator with operands");
         return success();
       })
-      .Case<acc::CreateOp, acc::CopyinOp, acc::UpdateDeviceOp>([](auto op) {
+      .Case<acc::CreateOp, acc::CopyinOp, acc::CopyoutOp, acc::DeleteOp,
+            acc::UpdateDeviceOp, acc::GetDevicePtrOp>([](auto op) {
         // NOP
         return success();
       })

mlir/test/Conversion/OpenACCToLLVM/convert-data-operands-to-llvmir.mlir

@@ -1,42 +1,5 @@
 // RUN: mlir-opt -convert-openacc-to-llvm='use-opaque-pointers=1' -split-input-file %s | FileCheck %s
 
-func.func @testexitdataop(%a: memref<10xf32>, %b: memref<10xf32>) -> () {
-  acc.exit_data copyout(%b : memref<10xf32>) delete(%a : memref<10xf32>)
-  return
-}
-
-// CHECK: acc.exit_data copyout(%{{.*}} : !llvm.struct<"openacc_data", (struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>, ptr, i64)>) delete(%{{.*}} : !llvm.struct<"openacc_data.1", (struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>, ptr, i64)>)
-
-// -----
-
-func.func @testexitdataop(%a: !llvm.ptr, %b: memref<10xf32>) -> () {
-  acc.exit_data copyout(%b : memref<10xf32>) delete(%a : !llvm.ptr)
-  return
-}
-
-// CHECK: acc.exit_data copyout(%{{.*}} : !llvm.struct<"openacc_data", (struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>, ptr, i64)>) delete(%{{.*}} : !llvm.ptr)
-
-// -----
-
-func.func @testexitdataop(%a: memref<10xi64>, %b: memref<10xf32>) -> () {
-  acc.exit_data copyout(%b : memref<10xf32>) delete(%a : memref<10xi64>) attributes {async}
-  return
-}
-
-// CHECK: acc.exit_data copyout(%{{.*}} : !llvm.struct<"openacc_data", (struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>, ptr, i64)>) delete(%{{.*}} : !llvm.struct<"openacc_data.1", (struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>, ptr, i64)>) attributes {async}
-
-// -----
-
-func.func @testexitdataop(%a: memref<10xf32>, %b: memref<10xf32>) -> () {
-  %ifCond = arith.constant true
-  acc.exit_data if(%ifCond) copyout(%b : memref<10xf32>) delete(%a : memref<10xf32>)
-  return
-}
-
-// CHECK: acc.exit_data if(%{{.*}}) copyout(%{{.*}} : !llvm.struct<"openacc_data", (struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>, ptr, i64)>) delete(%{{.*}} : !llvm.struct<"openacc_data.1", (struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>, ptr, i64)>)
-
-// -----
-
 func.func @testdataregion(%a: memref<10xf32>, %b: memref<10xf32>) -> () {
   acc.data copy(%b : memref<10xf32>) copyout(%a : memref<10xf32>) {
     acc.parallel {

mlir/test/Conversion/OpenACCToSCF/convert-openacc-to-scf.mlir

@@ -13,14 +13,16 @@ func.func @testenterdataop(%a: memref<f32>, %ifCond: i1) -> () {
 
 // -----
 
-func.func @testexitdataop(%a: memref<10xf32>, %ifCond: i1) -> () {
-  acc.exit_data if(%ifCond) delete(%a: memref<10xf32>)
+func.func @testexitdataop(%a: memref<f32>, %ifCond: i1) -> () {
+  %0 = acc.getdeviceptr varPtr(%a : memref<f32>) -> memref<f32>
+  acc.exit_data if(%ifCond) dataOperands(%0 : memref<f32>)
+  acc.delete accPtr(%0 : memref<f32>)
   return
 }
 
-// CHECK:      func @testexitdataop(%{{.*}}: memref<10xf32>, [[IFCOND:%.*]]: i1)
+// CHECK:      func @testexitdataop(%{{.*}}: memref<f32>, [[IFCOND:%.*]]: i1)
 // CHECK:        scf.if [[IFCOND]] {
-// CHECK-NEXT:     acc.exit_data delete(%{{.*}} : memref<10xf32>)
+// CHECK-NEXT:     acc.exit_data dataOperands(%{{.*}} : memref<f32>)
 // CHECK-NEXT:   }
 
 // -----
@@ -88,21 +90,25 @@ func.func @enter_data_false(%d1 : memref<f32>) {
 
 // -----
 
-func.func @exit_data_true(%d1 : memref<10xf32>) {
+func.func @exit_data_true(%d1 : memref<f32>) {
   %true = arith.constant true
-  acc.exit_data if(%true)  delete(%d1 : memref<10xf32>) attributes {async}
+  %0 = acc.getdeviceptr varPtr(%d1 : memref<f32>) -> memref<f32>
+  acc.exit_data if(%true) dataOperands(%0 : memref<f32>) attributes {async}
+  acc.delete accPtr(%0 : memref<f32>)
   return
 }
 
 // CHECK-LABEL: func.func @exit_data_true
 // CHECK-NOT:if
-// CHECK:acc.exit_data delete
+// CHECK:acc.exit_data dataOperands
 
 // -----
 
-func.func @exit_data_false(%d1 : memref<10xf32>) {
+func.func @exit_data_false(%d1 : memref<f32>) {
   %false = arith.constant false
-  acc.exit_data if(%false)  delete(%d1 : memref<10xf32>) attributes {async}
+  %0 = acc.getdeviceptr varPtr(%d1 : memref<f32>) -> memref<f32>
+  acc.exit_data if(%false) dataOperands(%0 : memref<f32>) attributes {async}
+  acc.delete accPtr(%0 : memref<f32>)
   return
 }
 

mlir/test/Dialect/OpenACC/canonicalize.mlir

@@ -23,19 +23,23 @@ func.func @testenterdataop(%a: memref<f32>) -> () {
 
 // -----
 
-func.func @testexitdataop(%a: memref<10xf32>) -> () {
+func.func @testexitdataop(%a: memref<f32>) -> () {
   %ifCond = arith.constant true
-  acc.exit_data if(%ifCond) delete(%a: memref<10xf32>)
+  %0 = acc.getdeviceptr varPtr(%a : memref<f32>) -> memref<f32>
+  acc.exit_data if(%ifCond) dataOperands(%0 : memref<f32>)
+  acc.delete accPtr(%0 : memref<f32>)
   return
 }
 
-// CHECK: acc.exit_data delete(%{{.*}} : memref<10xf32>)
+// CHECK: acc.exit_data dataOperands(%{{.*}} : memref<f32>)
 
 // -----
 
-func.func @testexitdataop(%a: memref<10xf32>) -> () {
+func.func @testexitdataop(%a: memref<f32>) -> () {
   %ifCond = arith.constant false
-  acc.exit_data if(%ifCond) delete(%a: memref<10xf32>)
+  %0 = acc.getdeviceptr varPtr(%a : memref<f32>) -> memref<f32>
+  acc.exit_data if(%ifCond) dataOperands(%0 : memref<f32>)
+  acc.delete accPtr(%0 : memref<f32>)
   return
 }
 
@@ -80,13 +84,15 @@ func.func @testenterdataop(%a: memref<f32>, %ifCond: i1) -> () {
 
 // -----
 
-func.func @testexitdataop(%a: memref<10xf32>, %ifCond: i1) -> () {
-  acc.exit_data if(%ifCond) delete(%a: memref<10xf32>)
+func.func @testexitdataop(%a: memref<f32>, %ifCond: i1) -> () {
+  %0 = acc.getdeviceptr varPtr(%a : memref<f32>) -> memref<f32>
+  acc.exit_data if(%ifCond) dataOperands(%0 : memref<f32>)
+  acc.delete accPtr(%0 : memref<f32>)
   return
 }
 
-// CHECK: func @testexitdataop(%{{.*}}: memref<10xf32>, [[IFCOND:%.*]]: i1)
-// CHECK:   acc.exit_data if(%{{.*}}) delete(%{{.*}} : memref<10xf32>)
+// CHECK: func @testexitdataop(%{{.*}}: memref<f32>, [[IFCOND:%.*]]: i1)
+// CHECK:   acc.exit_data if(%{{.*}}) dataOperands(%{{.*}} : memref<f32>)
 
 // -----
 

mlir/test/Dialect/OpenACC/invalid.mlir

@@ -167,22 +167,26 @@ acc.loop {
 
 // -----
 
-// expected-error@+1 {{at least one operand in copyout, delete or detach must appear on the exit data operation}}
+// expected-error@+1 {{at least one operand must be present in dataOperands on the exit data operation}}
 acc.exit_data attributes {async}
 
 // -----
 
 %cst = arith.constant 1 : index
-%value = memref.alloc() : memref<10xf32>
+%value = memref.alloc() : memref<f32>
+%0 = acc.getdeviceptr varPtr(%value : memref<f32>) -> memref<f32>
 // expected-error@+1 {{async attribute cannot appear with asyncOperand}}
-acc.exit_data async(%cst: index) delete(%value : memref<10xf32>) attributes {async}
+acc.exit_data async(%cst: index) dataOperands(%0 : memref<f32>) attributes {async}
+acc.delete accPtr(%0 : memref<f32>)
 
 // -----
 
 %cst = arith.constant 1 : index
-%value = memref.alloc() : memref<10xf32>
+%value = memref.alloc() : memref<f32>
+%0 = acc.getdeviceptr varPtr(%value : memref<f32>) -> memref<f32>
 // expected-error@+1 {{wait_devnum cannot appear without waitOperands}}
-acc.exit_data wait_devnum(%cst: index) delete(%value : memref<10xf32>)
+acc.exit_data wait_devnum(%cst: index) dataOperands(%0 : memref<f32>)
+acc.delete accPtr(%0 : memref<f32>)
 
 // -----