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

[clementval]

Summary

Remove old clause operands from acc.parallel operation since
the new dataOperands is now in place.
private, firstprivate and reductions will receive some redesign but are
not part of the new dataOperands.

[razvanlupusoru]

Thank you for this cleanup!

[clementval]

Rebase

[clementval]

Wrong update

[clementval]

Rebase

[clementval]

clang-format

flang/lib/Lower/OpenACC.cpp

if constexpr (!std::is_same_v<Op, mlir::acc::SerialOp>) {
     addOperand(operands, operandSegments, numGangs);
     addOperand(operands, operandSegments, numWorkers);
     addOperand(operands, operandSegments, vectorLength);
   }
   addOperand(operands, operandSegments, ifCond);
   addOperand(operands, operandSegments, selfCond);
   if constexpr (!std::is_same_v<Op, mlir::acc::KernelsOp>)
     addOperands(operands, operandSegments, reductionOperands);
+  if constexpr (!std::is_same_v<Op, mlir::acc::ParallelOp>)
     operandSegments.append({0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0});
   if constexpr (!std::is_same_v<Op, mlir::acc::KernelsOp>) {
     addOperands(operands, operandSegments, privateOperands);
     addOperands(operands, operandSegments, firstprivateOperands);
   }
   addOperands(operands, operandSegments, dataClauseOperands);
 
   Op computeOp;
   if constexpr (std::is_same_v<Op, mlir::acc::KernelsOp>)

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

for (Value operand : operands) {
         return false;
     }
     return true;
   };
 
   target.addDynamicallyLegalOp<acc::ParallelOp>(
       [allDataOperandsAreConverted](acc::ParallelOp op) {
         return allDataOperandsAreConverted(op.getReductionOperands()) &&
-               allDataOperandsAreConverted(op.getCopyOperands()) &&
-               allDataOperandsAreConverted(op.getCopyinOperands()) &&
-               allDataOperandsAreConverted(op.getCopyinReadonlyOperands()) &&
-               allDataOperandsAreConverted(op.getCopyoutOperands()) &&
-               allDataOperandsAreConverted(op.getCopyoutZeroOperands()) &&
-               allDataOperandsAreConverted(op.getCreateOperands()) &&
-               allDataOperandsAreConverted(op.getCreateZeroOperands()) &&
-               allDataOperandsAreConverted(op.getNoCreateOperands()) &&
-               allDataOperandsAreConverted(op.getPresentOperands()) &&
-               allDataOperandsAreConverted(op.getDevicePtrOperands()) &&
-               allDataOperandsAreConverted(op.getAttachOperands()) &&
                allDataOperandsAreConverted(op.getGangPrivateOperands()) &&
                allDataOperandsAreConverted(op.getGangFirstPrivateOperands());
       });
 
   if (failed(applyPartialConversion(op, target, std::move(patterns))))
     signalPassFailure();
 }

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

-// RUN: fir-opt -fir-openacc-data-operand-conversion='use-opaque-pointers=1' -split-input-file %s | FileCheck %s
-// RUN: fir-opt -fir-openacc-data-operand-conversion='use-opaque-pointers=1' -split-input-file %s | fir-opt -split-input-file --fir-to-llvm-ir | FileCheck %s --check-prefix=LLVMIR
-
-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"}
-  acc.parallel copyin(%0: !fir.ref<!fir.array<10xf32>>) {
-    acc.loop {
-      acc.yield
-    }
-    acc.yield
-  }
-  return
-}
-
-// CHECK-LABEL: func.func @_QQsub_parallel() attributes {fir.bindc_name = "test"} {
-// CHECK: %[[ADDR:.*]] = fir.address_of(@_QFEa) : !fir.ref<!fir.array<10xf32>>
-// CHECK: %[[CAST:.*]] = builtin.unrealized_conversion_cast %[[ADDR]] : !fir.ref<!fir.array<10xf32>> to !llvm.ptr<array<10 x f32>>
-// CHECK: acc.parallel copyin(%[[CAST]] : !llvm.ptr<array<10 x f32>>) {
-// CHECK:   acc.loop
-
-// LLVMIR-LABEL: llvm.func @_QQsub_parallel() attributes {fir.bindc_name = "test"} {
-// LLVMIR: %[[ADDR:.*]] = llvm.mlir.addressof @_QFEa : !llvm.ptr<array<10 x f32>>
-// LLVMIR: acc.parallel copyin(%[[ADDR]] : !llvm.ptr<array<10 x f32>>) {

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

let arguments = (ins Optional<IntOrIndex>:$async,
                        Optional<IntOrIndex>:$numGangs,
                        Optional<IntOrIndex>:$numWorkers,
                        Optional<IntOrIndex>:$vectorLength,
                        Optional<I1>:$ifCond,
                        Optional<I1>:$selfCond,
                        UnitAttr:$selfAttr,
                        OptionalAttr<OpenACC_ReductionOpAttr>:$reductionOp,
                        Variadic<AnyType>:$reductionOperands,
-                       Variadic<AnyType>:$copyOperands,
-                       Variadic<AnyType>:$copyinOperands,
-                       Variadic<AnyType>:$copyinReadonlyOperands,
-                       Variadic<AnyType>:$copyoutOperands,
-                       Variadic<AnyType>:$copyoutZeroOperands,
-                       Variadic<AnyType>:$createOperands,
-                       Variadic<AnyType>:$createZeroOperands,
-                       Variadic<AnyType>:$noCreateOperands,
-                       Variadic<AnyType>:$presentOperands,
-                       Variadic<AnyType>:$devicePtrOperands,
-                       Variadic<AnyType>:$attachOperands,
                        Variadic<AnyType>:$gangPrivateOperands,
                        Variadic<AnyType>:$gangFirstPrivateOperands,
                        Variadic<OpenACC_PointerLikeTypeInterface>:$dataClauseOperands,
                        OptionalAttr<DefaultValueAttr>:$defaultAttr);
 
   let regions = (region AnyRegion:$region);
 
   let extraClassDeclaration = [{
     /// The number of data operands.
     unsigned getNumDataOperands();
 
     /// The i-th data operand passed.
     Value getDataOperand(unsigned i);
   }];
 
   let assemblyFormat = [{
     oilist(
         `dataOperands` `(` $dataClauseOperands `:` type($dataClauseOperands) `)`
-      | `attach` `(` $attachOperands `:` type($attachOperands) `)`
       | `async` `(` $async `:` type($async) `)`
-      | `copy` `(` $copyOperands `:` type($copyOperands) `)`
-      | `copyin` `(` $copyinOperands `:` type($copyinOperands) `)`
-      | `copyin_readonly` `(` $copyinReadonlyOperands `:`
-          type($copyinReadonlyOperands) `)`
-      | `copyout` `(` $copyoutOperands `:` type($copyoutOperands) `)`
-      | `copyout_zero` `(` $copyoutZeroOperands `:`
-          type($copyoutZeroOperands) `)`
-      | `create` `(` $createOperands `:` type($createOperands) `)`
-      | `create_zero` `(` $createZeroOperands `:`
-          type($createZeroOperands) `)`
-      | `deviceptr` `(` $devicePtrOperands `:` type($devicePtrOperands) `)`
       | `firstprivate` `(` $gangFirstPrivateOperands `:`
             type($gangFirstPrivateOperands) `)`
-      | `no_create` `(` $noCreateOperands `:` type($noCreateOperands) `)`
       | `num_gangs` `(` $numGangs `:` type($numGangs) `)`
       | `num_workers` `(` $numWorkers `:` type($numWorkers) `)`
       | `private` `(` $gangPrivateOperands `:` type($gangPrivateOperands) `)`
-      | `present` `(` $presentOperands `:` type($presentOperands) `)`
       | `vector_length` `(` $vectorLength `:` type($vectorLength) `)`
       | `wait` `(` $waitOperands `:` type($waitOperands) `)`
       | `self` `(` $selfCond `)`
       | `if` `(` $ifCond `)`
       | `reduction` `(` $reductionOperands `:` type($reductionOperands) `)`
     )
     $region attr-dict-with-keyword
   }];

mlir/lib/Conversion/OpenACCToLLVM/OpenACCToLLVM.cpp

for (Value operand : operands) {
         return false;
     }
     return true;
   };
 
   target.addDynamicallyLegalOp<acc::ParallelOp>(
       [allDataOperandsAreConverted](acc::ParallelOp op) {
         return allDataOperandsAreConverted(op.getReductionOperands()) &&
-               allDataOperandsAreConverted(op.getCopyOperands()) &&
-               allDataOperandsAreConverted(op.getCopyinOperands()) &&
-               allDataOperandsAreConverted(op.getCopyinReadonlyOperands()) &&
-               allDataOperandsAreConverted(op.getCopyoutOperands()) &&
-               allDataOperandsAreConverted(op.getCopyoutZeroOperands()) &&
-               allDataOperandsAreConverted(op.getCreateOperands()) &&
-               allDataOperandsAreConverted(op.getCreateZeroOperands()) &&
-               allDataOperandsAreConverted(op.getNoCreateOperands()) &&
-               allDataOperandsAreConverted(op.getPresentOperands()) &&
-               allDataOperandsAreConverted(op.getDevicePtrOperands()) &&
-               allDataOperandsAreConverted(op.getAttachOperands()) &&
                allDataOperandsAreConverted(op.getGangPrivateOperands()) &&
                allDataOperandsAreConverted(op.getGangFirstPrivateOperands());
       });
 
   if (failed(applyPartialConversion(op, target, std::move(patterns))))
     signalPassFailure();
 }

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

if (!mlir::isa<acc::AttachOp, acc::CopyinOp, acc::CopyoutOp, acc::CreateOp,
             operand.getDefiningOp()))
       return op.emitError(
           "expect data entry/exit operation or acc.getdeviceptr "
           "as defining op");
   return success();
 }
 
 unsigned ParallelOp::getNumDataOperands() {
-  return getReductionOperands().size() + getCopyOperands().size() +
-         getCopyinOperands().size() + getCopyinReadonlyOperands().size() +
-         getCopyoutOperands().size() + getCopyoutZeroOperands().size() +
-         getCreateOperands().size() + getCreateZeroOperands().size() +
-         getNoCreateOperands().size() + getPresentOperands().size() +
-         getDevicePtrOperands().size() + getAttachOperands().size() +
-         getGangPrivateOperands().size() +
+  return getReductionOperands().size() + getGangPrivateOperands().size() +
          getGangFirstPrivateOperands().size() + getDataClauseOperands().size();
 }
 
 Value ParallelOp::getDataOperand(unsigned i) {
   unsigned numOptional = getAsync() ? 1 : 0;
   numOptional += getNumGangs() ? 1 : 0;
   numOptional += getNumWorkers() ? 1 : 0;
   numOptional += getVectorLength() ? 1 : 0;

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

-// RUN: mlir-opt -convert-openacc-to-llvm='use-opaque-pointers=1' -split-input-file %s | FileCheck %s
-
-func.func @testparallelop(%a: memref<10xf32>, %b: memref<10xf32>) -> () {
-  acc.parallel copy(%b : memref<10xf32>) copyout(%a : memref<10xf32>) {
-  }
-  return
-}
-
-// CHECK: acc.parallel copy(%{{.*}}: !llvm.struct<"openacc_data", (struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>, ptr, i64)>) copyout(%{{.*}}: !llvm.struct<"openacc_data.1", (struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>, ptr, i64)>)
-
-// -----
-
-func.func @testparallelop(%a: !llvm.ptr, %b: memref<10xf32>, %c: !llvm.ptr) -> () {
-  acc.parallel copyin(%b : memref<10xf32>) deviceptr(%c: !llvm.ptr) attach(%a : !llvm.ptr) {
-  }
-  return
-}
-
-// CHECK: acc.parallel attach(%{{.*}}: !llvm.ptr) copyin(%{{.*}}: !llvm.struct<"openacc_data", (struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>, ptr, i64)>) deviceptr(%{{.*}} : !llvm.ptr) 
-
-// -----
-
-func.func @testparallelop(%a: memref<10xf32>, %b: memref<10xf32>) -> () {
-  %ifCond = arith.constant true
-  acc.parallel if(%ifCond) copyin_readonly(%b : memref<10xf32>) copyout_zero(%a : memref<10xf32>) {
-  }
-  return
-}
-
-// CHECK: acc.parallel copyin_readonly(%{{.*}}: !llvm.struct<"openacc_data", (struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>, ptr, i64)>) copyout_zero(%{{.*}}: !llvm.struct<"openacc_data.1", (struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>, ptr, i64)>) if(%{{.*}})
-
-// -----
-
-func.func @testparallelop(%a: !llvm.ptr, %b: memref<10xf32>, %c: !llvm.ptr) -> () {
-  acc.parallel create(%b : memref<10xf32>) create_zero(%c: !llvm.ptr) no_create(%a : !llvm.ptr) {
-  }
-  return
-}
-
-// CHECK: acc.parallel create(%{{.*}}: !llvm.struct<"openacc_data", (struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>, ptr, i64)>) create_zero(%{{.*}}: !llvm.ptr) no_create(%{{.*}}: !llvm.ptr)
-
-// -----
-
-func.func @testparallelop(%a: memref<10xf32>, %b: memref<10xf32>) -> () {
-  acc.parallel present(%a, %b : memref<10xf32>, memref<10xf32>) {
-  }
-  return
-}
-
-// CHECK: acc.parallel present(%{{.*}}, %{{.*}} : !llvm.struct<"openacc_data", (struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>, ptr, i64)>, !llvm.struct<"openacc_data.1", (struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>, ptr, i64)>)
-
-// -----
-
-func.func @testparallelop(%i: i64, %a: memref<10xf32>, %b: memref<10xf32>) -> () {
-  acc.parallel num_gangs(%i: i64) present(%a, %b : memref<10xf32>, memref<10xf32>) {
-    %0 = arith.constant 0 : i32
-    acc.yield
-  } attributes {async}
-  return
-}
-
-// CHECK: acc.parallel num_gangs(%{{.*}}: i64) present(%{{.*}}, %{{.*}} : !llvm.struct<"openacc_data", (struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>, ptr, i64)>, !llvm.struct<"openacc_data.1", (struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>, ptr, i64)>)
-// CHECK:   %c0_i32 = arith.constant 0 : i32
-// CHECK:   acc.yield
-// CHECK: } attributes {async}

mlir/test/Dialect/OpenACC/ops.mlir

func.func @testparallelop(%a: memref<10xf32>, %b: memref<10xf32>, %c: memref<10x10xf32>) -> () {
   acc.parallel num_workers(%idxValue: index) {
   }
   acc.parallel vector_length(%i64value: i64) {
   }
   acc.parallel vector_length(%i32value: i32) {
   }
   acc.parallel vector_length(%idxValue: index) {
   }
-  acc.parallel copyin(%a, %b : memref<10xf32>, memref<10xf32>) {
-  }
-  acc.parallel copyin_readonly(%a, %b : memref<10xf32>, memref<10xf32>) {
-  }
-  acc.parallel copyin(%a: memref<10xf32>) copyout_zero(%b, %c : memref<10xf32>, memref<10x10xf32>) {
-  }
-  acc.parallel copyout(%b, %c : memref<10xf32>, memref<10x10xf32>) create(%a: memref<10xf32>) {
-  }
-  acc.parallel copyout_zero(%b, %c : memref<10xf32>, memref<10x10xf32>) create_zero(%a: memref<10xf32>) {
-  }
-  acc.parallel no_create(%a: memref<10xf32>) present(%b, %c : memref<10xf32>, memref<10x10xf32>) {
-  }
-  acc.parallel deviceptr(%a: memref<10xf32>) attach(%b, %c : memref<10xf32>, memref<10x10xf32>) {
-  }
   acc.parallel private(%a, %c : memref<10xf32>, memref<10x10xf32>) firstprivate(%b: memref<10xf32>) {
   }
   acc.parallel {
   } attributes {defaultAttr = #acc<defaultvalue none>}
   acc.parallel {
   } attributes {defaultAttr = #acc<defaultvalue present>}
   acc.parallel {
   } attributes {asyncAttr}

 // CHECK:      acc.parallel num_workers([[IDXVALUE]] : index) {
 // CHECK-NEXT: }
 // CHECK:      acc.parallel vector_length([[I64VALUE]] : i64) {
 // CHECK-NEXT: }
 // CHECK:      acc.parallel vector_length([[I32VALUE]] : i32) {
 // CHECK-NEXT: }
 // CHECK:      acc.parallel vector_length([[IDXVALUE]] : index) {
 // CHECK-NEXT: }
-// CHECK:      acc.parallel copyin([[ARGA]], [[ARGB]] : memref<10xf32>, memref<10xf32>) {
-// CHECK-NEXT: }
-// CHECK:      acc.parallel copyin_readonly([[ARGA]], [[ARGB]] : memref<10xf32>, memref<10xf32>) {
-// CHECK-NEXT: }
-// CHECK:      acc.parallel copyin([[ARGA]] : memref<10xf32>) copyout_zero([[ARGB]], [[ARGC]] : memref<10xf32>, memref<10x10xf32>) {
-// CHECK-NEXT: }
-// CHECK:      acc.parallel copyout([[ARGB]], [[ARGC]] : memref<10xf32>, memref<10x10xf32>) create([[ARGA]] : memref<10xf32>) {
-// CHECK-NEXT: }
-// CHECK:      acc.parallel copyout_zero([[ARGB]], [[ARGC]] : memref<10xf32>, memref<10x10xf32>) create_zero([[ARGA]] : memref<10xf32>) {
-// CHECK-NEXT: }
-// CHECK:      acc.parallel no_create([[ARGA]] : memref<10xf32>) present([[ARGB]], [[ARGC]] : memref<10xf32>, memref<10x10xf32>) {
-// CHECK-NEXT: }
-// CHECK:      acc.parallel attach([[ARGB]], [[ARGC]] : memref<10xf32>, memref<10x10xf32>) deviceptr([[ARGA]] : memref<10xf32>) {
-// CHECK-NEXT: }
 // CHECK:      acc.parallel firstprivate([[ARGB]] : memref<10xf32>) private([[ARGA]], [[ARGC]] : memref<10xf32>, memref<10x10xf32>) {
 // CHECK-NEXT: }
 // CHECK:      acc.parallel {
 // CHECK-NEXT: } attributes {defaultAttr = #acc<defaultvalue none>}
 // CHECK:      acc.parallel {
 // CHECK-NEXT: } attributes {defaultAttr = #acc<defaultvalue present>}
 // CHECK:      acc.parallel {
 // CHECK-NEXT: } attributes {asyncAttr}