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

Since the new data operand operations have been added in D148389 and
adopted on acc.enter_data in D148721, 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/D150132

flang/lib/Lower/OpenACC.cpp

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

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::EnterDataOp>>(converter);
   patterns.add<LegalizeDataOpForLLVMTranslation<acc::ExitDataOp>>(converter);
   patterns.add<LegalizeDataOpForLLVMTranslation<acc::ParallelOp>>(converter);
 
@@ -148,14 +147,6 @@ void OpenACCDataOperandConversion::runOnOperation() {
                allDataOperandsAreConverted(op.getAttachOperands());
       });
 
-  target.addDynamicallyLegalOp<acc::EnterDataOp>(
-      [allDataOperandsAreConverted](acc::EnterDataOp op) {
-        return allDataOperandsAreConverted(op.getCopyinOperands()) &&
-               allDataOperandsAreConverted(op.getCreateOperands()) &&
-               allDataOperandsAreConverted(op.getCreateZeroOperands()) &&
-               allDataOperandsAreConverted(op.getAttachOperands());
-      });
-
   target.addDynamicallyLegalOp<acc::ExitDataOp>(
       [allDataOperandsAreConverted](acc::ExitDataOp op) {
         return allDataOperandsAreConverted(op.getCopyoutOperands()) &&

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

@@ -32,21 +32,17 @@ fir.global internal @_QFEa : !fir.array<10xf32> {
 
 func.func @_QQsub_enter_exit() attributes {fir.bindc_name = "a"} {
   %0 = fir.address_of(@_QFEa) : !fir.ref<!fir.array<10xf32>>
-  acc.enter_data copyin(%0 : !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: %[[CAST0:.*]] = builtin.unrealized_conversion_cast %[[ADDR]] : !fir.ref<!fir.array<10xf32>> to !llvm.ptr<array<10 x f32>>
-// CHECK: acc.enter_data copyin(%[[CAST0]] : !llvm.ptr<array<10 x f32>>)
 // 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.enter_data copyin(%[[ADDR]] : !llvm.ptr<array<10 x f32>>)
 // LLVMIR: acc.exit_data copyout(%[[ADDR]] : !llvm.ptr<array<10 x f32>>)
 
 // -----

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

@@ -765,10 +765,6 @@ def OpenACC_EnterDataOp : OpenACC_Op<"enter_data", [AttrSizedOperandSegments]> {
                        Optional<IntOrIndex>:$waitDevnum,
                        Variadic<IntOrIndex>:$waitOperands,
                        UnitAttr:$wait,
-                       Variadic<AnyType>:$copyinOperands,
-                       Variadic<AnyType>:$createOperands,
-                       Variadic<AnyType>:$createZeroOperands,
-                       Variadic<AnyType>:$attachOperands,
                        Variadic<OpenACC_PointerLikeTypeInterface>:$dataClauseOperands);
 
   let extraClassDeclaration = [{
@@ -785,11 +781,6 @@ def OpenACC_EnterDataOp : OpenACC_Op<"enter_data", [AttrSizedOperandSegments]> {
       | `async` `(` $asyncOperand `:` type($asyncOperand) `)`
       | `wait_devnum` `(` $waitDevnum `:` type($waitDevnum) `)`
       | `wait` `(` $waitOperands `:` type($waitOperands) `)`
-      | `copyin` `(` $copyinOperands `:` type($copyinOperands) `)`
-      | `create` `(` $createOperands `:` type($createOperands) `)`
-      | `create_zero` `(` $createZeroOperands `:`
-          type($createZeroOperands) `)`
-      | `attach` `(` $attachOperands `:` type($attachOperands) `)`
       | `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::EnterDataOp>>(converter);
   patterns.add<LegalizeDataOpForLLVMTranslation<acc::ExitDataOp>>(converter);
   patterns.add<LegalizeDataOpForLLVMTranslation<acc::ParallelOp>>(converter);
 }
@@ -209,14 +208,6 @@ void ConvertOpenACCToLLVMPass::runOnOperation() {
                allDataOperandsAreConverted(op.getAttachOperands());
       });
 
-  target.addDynamicallyLegalOp<acc::EnterDataOp>(
-      [allDataOperandsAreConverted](acc::EnterDataOp op) {
-        return allDataOperandsAreConverted(op.getCopyinOperands()) &&
-               allDataOperandsAreConverted(op.getCreateOperands()) &&
-               allDataOperandsAreConverted(op.getCreateZeroOperands()) &&
-               allDataOperandsAreConverted(op.getAttachOperands());
-      });
-
   target.addDynamicallyLegalOp<acc::ExitDataOp>(
       [allDataOperandsAreConverted](acc::ExitDataOp op) {
         return allDataOperandsAreConverted(op.getCopyoutOperands()) &&

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

@@ -588,12 +588,9 @@ LogicalResult acc::EnterDataOp::verify() {
   // 2.6.6. Data Enter Directive restriction
   // At least one copyin, create, or attach clause must appear on an enter data
   // directive.
-  if (getCopyinOperands().empty() && getCreateOperands().empty() &&
-      getCreateZeroOperands().empty() && getAttachOperands().empty() &&
-      getDataClauseOperands().empty())
-    return emitError(
-        "at least one operand in copyin, create, "
-        "create_zero or attach must appear on the enter data operation");
+  if (getDataClauseOperands().empty())
+    return emitError("at least one operand must be present in dataOperands on "
+                     "the enter data operation");
 
   // The async attribute represent the async clause without value. Therefore the
   // attribute and operand cannot appear at the same time.
@@ -617,9 +614,7 @@ LogicalResult acc::EnterDataOp::verify() {
 }
 
 unsigned EnterDataOp::getNumDataOperands() {
-  return getCopyinOperands().size() + getCreateOperands().size() +
-         getCreateZeroOperands().size() + getAttachOperands().size() +
-         getDataClauseOperands().size();
+  return getDataClauseOperands().size();
 }
 
 Value EnterDataOp::getDataOperand(unsigned i) {

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

@@ -162,16 +162,30 @@ processDataOperands(llvm::IRBuilderBase &builder,
   unsigned index = 0;
 
   // Create operands are handled as `alloc` call.
-  if (failed(processOperands(builder, moduleTranslation, op,
-                             op.getCreateOperands(), op.getNumDataOperands(),
-                             kCreateFlag, flags, names, index, mapperAllocas)))
+  // Copyin operands are handled as `to` call.
+  llvm::SmallVector<mlir::Value> create, copyin;
+  for (mlir::Value dataOp : op.getDataClauseOperands()) {
+    if (auto createOp =
+            mlir::dyn_cast_or_null<acc::CreateOp>(dataOp.getDefiningOp())) {
+      create.push_back(createOp.getVarPtr());
+    } else if (auto copyinOp = mlir::dyn_cast_or_null<acc::CopyinOp>(
+                   dataOp.getDefiningOp())) {
+      copyin.push_back(copyinOp.getVarPtr());
+    }
+  }
+
+  auto nbTotalOperands = create.size() + copyin.size();
+
+  // Create operands are handled as `alloc` call.
+  if (failed(processOperands(builder, moduleTranslation, op, create,
+                             nbTotalOperands, kCreateFlag, flags, names, index,
+                             mapperAllocas)))
     return failure();
 
   // Copyin operands are handled as `to` call.
-  if (failed(processOperands(builder, moduleTranslation, op,
-                             op.getCopyinOperands(), op.getNumDataOperands(),
-                             kDeviceCopyinFlag, flags, names, index,
-                             mapperAllocas)))
+  if (failed(processOperands(builder, moduleTranslation, op, copyin,
+                             nbTotalOperands, kDeviceCopyinFlag, flags, names,
+                             index, mapperAllocas)))
     return failure();
 
   return success();
@@ -495,7 +509,7 @@ LogicalResult OpenACCDialectLLVMIRTranslationInterface::convertOperation(
                "unexpected OpenACC terminator with operands");
         return success();
       })
-      .Case([&](acc::UpdateDeviceOp) {
+      .Case<acc::CreateOp, acc::CopyinOp, acc::UpdateDeviceOp>([](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 @testenterdataop(%a: memref<10xf32>, %b: memref<10xf32>) -> () {
-  acc.enter_data copyin(%b : memref<10xf32>) create(%a : memref<10xf32>)
-  return
-}
-
-// CHECK: acc.enter_data copyin(%{{.*}} : !llvm.struct<"openacc_data", (struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>, ptr, i64)>) create(%{{.*}} : !llvm.struct<"openacc_data.1", (struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>, ptr, i64)>)
-
-// -----
-
-func.func @testenterdataop(%a: !llvm.ptr, %b: memref<10xf32>) -> () {
-  acc.enter_data copyin(%b : memref<10xf32>) create(%a : !llvm.ptr)
-  return
-}
-
-// CHECK: acc.enter_data copyin(%{{.*}} : !llvm.struct<"openacc_data", (struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>, ptr, i64)>) create(%{{.*}} : !llvm.ptr)
-
-// -----
-
-func.func @testenterdataop(%a: memref<10xi64>, %b: memref<10xf32>) -> () {
-  acc.enter_data copyin(%b : memref<10xf32>) create_zero(%a : memref<10xi64>) attributes {async}
-  return
-}
-
-// CHECK: acc.enter_data copyin(%{{.*}} : !llvm.struct<"openacc_data", (struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>, ptr, i64)>) create_zero(%{{.*}} : !llvm.struct<"openacc_data.1", (struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>, ptr, i64)>) attributes {async}
-
-// -----
-
-func.func @testenterdataop(%a: memref<10xf32>, %b: memref<10xf32>) -> () {
-  %ifCond = arith.constant true
-  acc.enter_data if(%ifCond) copyin(%b : memref<10xf32>) create(%a : memref<10xf32>)
-  return
-}
-
-// CHECK: acc.enter_data if(%{{.*}}) copyin(%{{.*}} : !llvm.struct<"openacc_data", (struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>, ptr, i64)>) create(%{{.*}} : !llvm.struct<"openacc_data.1", (struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>, ptr, i64)>)
-
-// -----
-
 func.func @testexitdataop(%a: memref<10xf32>, %b: memref<10xf32>) -> () {
   acc.exit_data copyout(%b : memref<10xf32>) delete(%a : memref<10xf32>)
   return

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

@@ -1,13 +1,14 @@
 // RUN: mlir-opt %s -convert-openacc-to-scf -split-input-file | FileCheck %s
 
-func.func @testenterdataop(%a: memref<10xf32>, %ifCond: i1) -> () {
-  acc.enter_data if(%ifCond) create(%a: memref<10xf32>)
+func.func @testenterdataop(%a: memref<f32>, %ifCond: i1) -> () {
+  %0 = acc.create varPtr(%a : memref<f32>) -> memref<f32>
+  acc.enter_data if(%ifCond) dataOperands(%0 : memref<f32>)
   return
 }
 
-// CHECK:      func @testenterdataop(%{{.*}}: memref<10xf32>, [[IFCOND:%.*]]: i1)
+// CHECK:      func @testenterdataop(%{{.*}}: memref<f32>, [[IFCOND:%.*]]: i1)
 // CHECK:        scf.if [[IFCOND]] {
-// CHECK-NEXT:     acc.enter_data create(%{{.*}} : memref<10xf32>)
+// CHECK-NEXT:     acc.enter_data dataOperands(%{{.*}} : memref<f32>)
 // CHECK-NEXT:   }
 
 // -----
@@ -62,26 +63,28 @@ func.func @update_false(%arg0: memref<f32>) {
 
 // -----
 
-func.func @enter_data_true(%d1 : memref<10xf32>) {
+func.func @enter_data_true(%d1 : memref<f32>) {
   %true = arith.constant true
-  acc.enter_data if(%true) create(%d1 : memref<10xf32>) attributes {async}
+  %0 = acc.create varPtr(%d1 : memref<f32>) -> memref<f32>
+  acc.enter_data if(%true) dataOperands(%0 : memref<f32>) attributes {async}
   return
 }
 
 // CHECK-LABEL: func.func @enter_data_true
 // CHECK-NOT:     if
-// CHECK:           acc.enter_data create
+// CHECK:           acc.enter_data dataOperands
 
 // -----
 
-func.func @enter_data_false(%d1 : memref<10xf32>) {
+func.func @enter_data_false(%d1 : memref<f32>) {
   %false = arith.constant false
-  acc.enter_data if(%false) create(%d1 : memref<10xf32>) attributes {async}
+  %0 = acc.create varPtr(%d1 : memref<f32>) -> memref<f32>
+  acc.enter_data if(%false) dataOperands(%0 : memref<f32>) attributes {async}
   return
 }
 
 // CHECK-LABEL: func.func @enter_data_false
-// CHECK-NOT:acc.enter_data
+// CHECK-NOT:     acc.enter_data
 
 // -----
 

mlir/test/Dialect/OpenACC/canonicalize.mlir

@@ -1,18 +1,20 @@
 // RUN: mlir-opt %s -canonicalize="test-convergence" -split-input-file | FileCheck %s
 
-func.func @testenterdataop(%a: memref<10xf32>) -> () {
+func.func @testenterdataop(%a: memref<f32>) -> () {
   %ifCond = arith.constant true
-  acc.enter_data if(%ifCond) create(%a: memref<10xf32>)
+  %0 = acc.create varPtr(%a : memref<f32>) -> memref<f32>
+  acc.enter_data if(%ifCond) dataOperands(%0 : memref<f32>)
   return
 }
 
-// CHECK: acc.enter_data create(%{{.*}} : memref<10xf32>)
+// CHECK: acc.enter_data dataOperands(%{{.*}} : memref<f32>)
 
 // -----
 
-func.func @testenterdataop(%a: memref<10xf32>) -> () {
+func.func @testenterdataop(%a: memref<f32>) -> () {
   %ifCond = arith.constant false
-  acc.enter_data if(%ifCond) create(%a: memref<10xf32>)
+  %0 = acc.create varPtr(%a : memref<f32>) -> memref<f32>
+  acc.enter_data if(%ifCond) dataOperands(%0 : memref<f32>)
   return
 }
 
@@ -67,13 +69,14 @@ func.func @testupdateop(%a: memref<f32>) -> () {
 
 // -----
 
-func.func @testenterdataop(%a: memref<10xf32>, %ifCond: i1) -> () {
-  acc.enter_data if(%ifCond) create(%a: memref<10xf32>)
+func.func @testenterdataop(%a: memref<f32>, %ifCond: i1) -> () {
+  %0 = acc.create varPtr(%a : memref<f32>) -> memref<f32>
+  acc.enter_data if(%ifCond) dataOperands(%0 : memref<f32>)
   return
 }
 
-// CHECK:  func @testenterdataop(%{{.*}}: memref<10xf32>, [[IFCOND:%.*]]: i1)
-// CHECK:    acc.enter_data if(%{{.*}}) create(%{{.*}} : memref<10xf32>)
+// CHECK:  func @testenterdataop(%{{.*}}: memref<f32>, [[IFCOND:%.*]]: i1)
+// CHECK:    acc.enter_data if(%{{.*}}) dataOperands(%{{.*}} : memref<f32>)
 
 // -----
 

mlir/test/Dialect/OpenACC/invalid.mlir

@@ -186,29 +186,32 @@ acc.exit_data wait_devnum(%cst: index) delete(%value : memref<10xf32>)
 
 // -----
 
-// expected-error@+1 {{at least one operand in copyin, create, create_zero or attach must appear on the enter data operation}}
+// expected-error@+1 {{at least one operand must be present in dataOperands on the enter data operation}}
 acc.enter_data attributes {async}
 
 // -----
 
 %cst = arith.constant 1 : index
-%value = memref.alloc() : memref<10xf32>
+%value = memref.alloc() : memref<f32>
+%0 = acc.create varPtr(%value : memref<f32>) -> memref<f32>
 // expected-error@+1 {{async attribute cannot appear with asyncOperand}}
-acc.enter_data async(%cst: index) create(%value : memref<10xf32>) attributes {async}
+acc.enter_data async(%cst: index) dataOperands(%0 : memref<f32>) attributes {async}
 
 // -----
 
 %cst = arith.constant 1 : index
-%value = memref.alloc() : memref<10xf32>
+%value = memref.alloc() : memref<f32>
+%0 = acc.create varPtr(%value : memref<f32>) -> memref<f32>
 // expected-error@+1 {{wait attribute cannot appear with waitOperands}}
-acc.enter_data wait(%cst: index) create(%value : memref<10xf32>) attributes {wait}
+acc.enter_data wait(%cst: index) dataOperands(%0 : memref<f32>) attributes {wait}
 
 // -----
 
 %cst = arith.constant 1 : index
-%value = memref.alloc() : memref<10xf32>
+%value = memref.alloc() : memref<f32>
+%0 = acc.create varPtr(%value : memref<f32>) -> memref<f32>
 // expected-error@+1 {{wait_devnum cannot appear without waitOperands}}
-acc.enter_data wait_devnum(%cst: index) create(%value : memref<10xf32>)
+acc.enter_data wait_devnum(%cst: index) dataOperands(%0 : memref<f32>)
 
 // -----