[mlir][openacc] Switch host/device clauses to data operand operations

[clementval]

Summary

Introduce acc.update_host and acc.update_device as data
operands operation to be used by the acc.update op.

[jeanPerier]

Looks great

[razvanlupusoru]

Looks great to me - glad to see this representation normalized too :)
Just one suggestion - I find the current names ambiguous without the word "update"

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

87–89

Can we please rename these to:
OpenACC_UpdateHostClause : I64EnumAttrCase<"acc_update_host", 17>;
OpenACC_UpdateSelfClause : I64EnumAttrCase<"acc_update_self", 17>;
OpenACC_UpdateDeviceClause : I64EnumAttrCase<"acc_update_device", 17>;

[clementval]

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

87–89

I'll update that.

[clementval]

Rename enums

flang/lib/Lower/OpenACC.cpp

genACCUpdateOp(Fortran::lower::AbstractConverter &converter,
   llvm::SmallVector<int32_t> operandSegments;
   addOperand(operands, operandSegments, ifCond);
   addOperand(operands, operandSegments, async);
   addOperand(operands, operandSegments, waitDevnum);
   addOperands(operands, operandSegments, waitOperands);
   addOperands(operands, operandSegments, deviceTypeOperands);
   addOperands(operands, operandSegments, hostOperands);
   addOperands(operands, operandSegments, deviceOperands);
+  operandSegments.push_back(0); // temporary for dataClauseOperands.
 
   mlir::acc::UpdateOp updateOp = createSimpleOp<mlir::acc::UpdateOp>(
       firOpBuilder, currentLocation, operands, operandSegments);
 
   if (addAsyncAttr)
     updateOp.setAsyncAttr(firOpBuilder.getUnitAttr());
   if (addWaitAttr)
     updateOp.setWaitAttr(firOpBuilder.getUnitAttr());

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

 def OpenACC_DeleteClause          : I64EnumAttrCase<"acc_delete", 9>;
 def OpenACC_AttachClause          : I64EnumAttrCase<"acc_attach", 10>;
 def OpenACC_DetachClause          : I64EnumAttrCase<"acc_detach", 11>;
 def OpenACC_NoCreateClause        : I64EnumAttrCase<"acc_no_create", 12>;
 def OpenACC_PrivateClause         : I64EnumAttrCase<"acc_private", 13>;
 def OpenACC_FirstPrivateClause    : I64EnumAttrCase<"acc_firstprivate", 14>;
 def OpenACC_IsDevicePtrClause     : I64EnumAttrCase<"acc_deviceptr", 15>;
 def OpenACC_GetDevicePtrClause    : I64EnumAttrCase<"acc_getdeviceptr", 16>;
+def OpenACC_UpdateHost            : I64EnumAttrCase<"acc_update_host", 17>;
+def OpenACC_UpdateSelf            : I64EnumAttrCase<"acc_update_self", 18>;
+def OpenACC_UpdateDevice          : I64EnumAttrCase<"acc_update_device", 19>;
 
 def OpenACC_DataClauseEnum : I64EnumAttr<"DataClause",
     "data clauses supported by OpenACC",
     [OpenACC_CopyinClause, OpenACC_CopyinReadonlyClause, OpenACC_CopyClause,
      OpenACC_CopyoutClause, OpenACC_CopyoutZeroClause, OpenACC_PresentClause,
      OpenACC_CreateClause, OpenACC_CreateZeroClause, OpenACC_DeleteClause,
      OpenACC_AttachClause, OpenACC_DetachClause, OpenACC_NoCreateClause,
      OpenACC_PrivateClause, OpenACC_FirstPrivateClause,
-     OpenACC_IsDevicePtrClause, OpenACC_GetDevicePtrClause
+     OpenACC_IsDevicePtrClause, OpenACC_GetDevicePtrClause, OpenACC_UpdateHost,
+     OpenACC_UpdateSelf, OpenACC_UpdateDevice,
     ]> {
   let cppNamespace = "::mlir::acc";
 }
 
 // Used for data specification in data clauses (2.7.1).
 // Either (or both) extent and upperbound must be specified.
 def OpenACC_DataBoundsOp : OpenACC_Op<"bounds",
     [AttrSizedOperandSegments, NoMemoryEffect]> {

 // It is also useful for providing the device address for unstructured construct
 // exit_data since unlike structured constructs, there is no matching data entry
 // operation.
 def OpenACC_GetDevicePtrOp : OpenACC_DataEntryOp<"getdeviceptr",
     "mlir::acc::DataClause::acc_getdeviceptr"> {
   let summary = "Gets device address from host address if it exists on device.";
 }
 
+//===----------------------------------------------------------------------===//
+// 2.14.4 device clause
+//===----------------------------------------------------------------------===//
+def OpenACC_UpdateDeviceOp : OpenACC_DataEntryOp<"update_device",
+    "mlir::acc::DataClause::acc_update_device"> {
+  let summary = "Represents acc update device semantics.";
+}
+
 // Data exit operation does not refer to OpenACC spec terminology, but to
 // terminology used in this dialect. It refers to data operations that will appear
 // after data or compute region. It will be used as the base of acc dialect
 // operations for the following OpenACC data clauses: copyout, detach, delete.
 class OpenACC_DataExitOp<string mnemonic, string clause, list<Trait> traits = []> :
     OpenACC_Op<mnemonic, !listconcat(traits,
         [AttrSizedOperandSegments])> {
   let arguments = (ins OpenACC_PointerLikeTypeInterface:$accPtr,

 // 2.7.13 detach clause
 //===----------------------------------------------------------------------===//
 def OpenACC_DetachOp : OpenACC_DataExitOp<"detach",
     "mlir::acc::DataClause::acc_detach"> {
   let summary = "Represents acc detach semantics - reverse of attach.";
 }
 
 //===----------------------------------------------------------------------===//
+// 2.14.4 host clause
+//===----------------------------------------------------------------------===//
+def OpenACC_UpdateHostOp : OpenACC_DataExitOp<"update_host",
+    "mlir::acc::DataClause::acc_update_host"> {
+  let summary = "Represents acc update host semantics.";
+  let extraClassDeclaration = [{
+    /// Check if this is an acc update self.
+    bool isSelf() {
+      return getDataClause() == acc::DataClause::acc_update_self;
+    }
+  }];
+}
+
+//===----------------------------------------------------------------------===//
 // 2.5.1 parallel Construct
 //===----------------------------------------------------------------------===//
 
 def OpenACC_ParallelOp : OpenACC_Op<"parallel",
     [AttrSizedOperandSegments, RecursiveMemoryEffects]> {
   let summary = "parallel construct";
   let description = [{
     The "acc.parallel" operation represents a parallel construct block. It has
let arguments = (ins Optional<I1>:$ifCond,
                        Optional<IntOrIndex>:$asyncOperand,
                        Optional<IntOrIndex>:$waitDevnum,
                        Variadic<IntOrIndex>:$waitOperands,
                        UnitAttr:$async,
                        UnitAttr:$wait,
                        Variadic<IntOrIndex>:$deviceTypeOperands,
                        Variadic<AnyType>:$hostOperands,
                        Variadic<AnyType>:$deviceOperands,
+                       Variadic<OpenACC_PointerLikeTypeInterface>:$dataClauseOperands,
                        UnitAttr:$ifPresent);
 
   let extraClassDeclaration = [{
     /// The number of data operands.
     unsigned getNumDataOperands();
 
     /// The i-th data operand passed.
     Value getDataOperand(unsigned i);
   }];
 
   let assemblyFormat = [{
     oilist(
         `if` `(` $ifCond `)`
       | `async` `(` $asyncOperand `:` type($asyncOperand) `)`
       | `wait_devnum` `(` $waitDevnum `:` type($waitDevnum) `)`
       | `device_type` `(` $deviceTypeOperands `:`
           type($deviceTypeOperands) `)`
       | `wait` `(` $waitOperands `:` type($waitOperands) `)`
       | `host` `(` $hostOperands `:` type($hostOperands) `)`
       | `device` `(` $deviceOperands `:` type($deviceOperands) `)`
+      | `dataOperands` `(` $dataClauseOperands `:` type($dataClauseOperands) `)`
     )
     attr-dict-with-keyword
   }];
 
   let hasCanonicalizer = 1;
   let hasVerifier = 1;
 }
 

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

if (getDataClause() != acc::DataClause::acc_detach &&
     return emitError(
         "data clause associated with detach operation must match its intent"
         " or specify original clause this operation was decomposed from");
   if (!getVarPtr() && !getAccPtr())
     return emitError("must have either host or device pointer");
   return success();
 }
 
+//===----------------------------------------------------------------------===//
+// HostOp
+//===----------------------------------------------------------------------===//
+LogicalResult acc::UpdateHostOp::verify() {
+  // Test for all clauses this operation can be decomposed from:
+  if (getDataClause() != acc::DataClause::acc_update_host &&
+      getDataClause() != acc::DataClause::acc_update_self)
+    return emitError(
+        "data clause associated with host operation must match its intent"
+        " or specify original clause this operation was decomposed from");
+  if (!getVarPtr() || !getAccPtr())
+    return emitError("must have both host and device pointers");
+  return success();
+}
+
+//===----------------------------------------------------------------------===//
+// DeviceOp
+//===----------------------------------------------------------------------===//
+LogicalResult acc::UpdateDeviceOp::verify() {
+  // Test for all clauses this operation can be decomposed from:
+  if (getDataClause() != acc::DataClause::acc_update_device)
+    return emitError(
+        "data clause associated with device operation must match its intent"
+        " or specify original clause this operation was decomposed from");
+  return success();
+}
+
 template <typename StructureOp>
 static ParseResult parseRegions(OpAsmParser &parser, OperationState &state,
                                 unsigned nRegions = 1) {
 
   SmallVector<Region *, 2> regions;
   for (unsigned i = 0; i < nRegions; ++i)
     regions.push_back(state.addRegion());
 

 }
 
 //===----------------------------------------------------------------------===//
 // UpdateOp
 //===----------------------------------------------------------------------===//
 
 LogicalResult acc::UpdateOp::verify() {
   // At least one of host or device should have a value.
-  if (getHostOperands().empty() && getDeviceOperands().empty())
+  if (getHostOperands().empty() && getDeviceOperands().empty() &&
+      getDataClauseOperands().empty())
     return emitError(
         "at least one value must be present in hostOperands or deviceOperands");
 
   // The async attribute represent the async clause without value. Therefore the
   // attribute and operand cannot appear at the same time.
   if (getAsyncOperand() && getAsync())
     return emitError("async attribute cannot appear with asyncOperand");
 
   // The wait attribute represent the wait clause without values. Therefore the
   // attribute and operands cannot appear at the same time.
   if (!getWaitOperands().empty() && getWait())
     return emitError("wait attribute cannot appear with waitOperands");
 
   if (getWaitDevnum() && getWaitOperands().empty())
     return emitError("wait_devnum cannot appear without waitOperands");
 
   return success();
 }
 
 unsigned UpdateOp::getNumDataOperands() {
-  return getHostOperands().size() + getDeviceOperands().size();
+  return getHostOperands().size() + getDeviceOperands().size() +
+         getDataClauseOperands().size();
 }
 
 Value UpdateOp::getDataOperand(unsigned i) {
   unsigned numOptional = getAsyncOperand() ? 1 : 0;
   numOptional += getWaitDevnum() ? 1 : 0;
   numOptional += getIfCond() ? 1 : 0;
   return getOperand(getWaitOperands().size() + getDeviceTypeOperands().size() +
                     numOptional + i);

mlir/test/Dialect/OpenACC/ops.mlir

 }
 
 // CHECK: func.func @testunstructuredclauseops([[ARGA:%.*]]: memref<10xf32>) {
 // CHECK: [[COPYIN:%.*]] = acc.copyin varPtr([[ARGA]] : memref<10xf32>) -> memref<10xf32> {structured = false}
 // CHECK-NEXT: acc.enter_data dataOperands([[COPYIN]] : memref<10xf32>)
 // CHECK: [[DEVPTR:%.*]] = acc.getdeviceptr varPtr([[ARGA]] : memref<10xf32>) -> memref<10xf32> {dataClause = 4 : i64}
 // CHECK-NEXT: acc.exit_data dataOperands([[DEVPTR]] : memref<10xf32>)
 // CHECK-NEXT: acc.copyout accPtr([[DEVPTR]] : memref<10xf32>) to varPtr([[ARGA]] : memref<10xf32>) {structured = false}
+
+// -----
+
+func.func @host_device_ops(%a: memref<10xf32>) -> () {
+  %devptr = acc.getdeviceptr varPtr(%a : memref<10xf32>) -> memref<10xf32> {dataClause = 16}
+  acc.update_host accPtr(%devptr : memref<10xf32>) to varPtr(%a : memref<10xf32>) {structured = false}
+  acc.update dataOperands(%devptr : memref<10xf32>)
+
+  %accPtr = acc.update_device varPtr(%a : memref<10xf32>) -> memref<10xf32>
+  acc.update dataOperands(%accPtr : memref<10xf32>)
+  return
+}