[mlir][openacc] Add missing operands for acc.data operation

[clementval]

Summary

Add missing operands to represent copyin with readonly modifier, copyout with zero modifier
and create with zero modifier.

[ftynse]

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

439–443

Ultra-nit: that's a lot of space taken on stack (8 x sizeof(intptr_t) x number of operand groups). I'd take something like 2 for SmallVector stack-allocated size here unless some lists expect very long lists of values.

[clementval]

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

439–443

Sure! didn't think about it when adding new variables here.

[clementval]

Rebase + review comment

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

let description = [{
     }
     ```
   }];
 
 
   let arguments = (ins Variadic<AnyType>:$presentOperands,
                        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>:$deleteOperands,
                        Variadic<AnyType>:$attachOperands,
                        Variadic<AnyType>:$detachOperands);
 
   let regions = (region AnyRegion:$region);
 
   let extraClassDeclaration = [{
     static StringRef getAttachKeyword() { return "attach"; }
     static StringRef getDeleteKeyword() { return "delete"; }
     static StringRef getDetachKeyword() { return "detach"; }
     static StringRef getCopyinKeyword() { return "copyin"; }
+    static StringRef getCopyinReadonlyKeyword() { return "copyin_readonly"; }
     static StringRef getCopyKeyword() { return "copy"; }
     static StringRef getCopyoutKeyword() { return "copyout"; }
+    static StringRef getCopyoutZeroKeyword() { return "copyout_zero"; }
     static StringRef getCreateKeyword() { return "create"; }
+    static StringRef getCreateZeroKeyword() { return "create_zero"; }
     static StringRef getNoCreateKeyword() { return "no_create"; }
     static StringRef getPresentKeyword() { return "present"; }
   }];
 
   let verifier = ?;
 }
 
 def OpenACC_TerminatorOp : OpenACC_Op<"terminator", [Terminator]> {

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

printer.printOptionalAttrDictWithKeyword(
       op.getAttrs(), ParallelOp::getOperandSegmentSizeAttr());
 }
 
 //===----------------------------------------------------------------------===//
 // DataOp
 //===----------------------------------------------------------------------===//
 
 /// Parse acc.data operation
-/// operation := `acc.parallel` `present` `(` value-list `)`?
-///                             `copy` `(` value-list `)`?
-///                             `copyin` `(` value-list `)`?
-///                             `copyout` `(` value-list `)`?
-///                             `create` `(` value-list `)`?
-///                             `no_create` `(` value-list `)`?
-///                             `delete` `(` value-list `)`?
-///                             `attach` `(` value-list `)`?
-///                             `detach` `(` value-list `)`?
+/// operation := `acc.parallel` (`present` `(` value-list `)`)?
+///                             (`copy` `(` value-list `)`)?
+///                             (`copyin` `(` value-list `)`)?
+///                             (`copyin_readonly` `(` value-list `)`)?
+///                             (`copyout` `(` value-list `)`)?
+///                             (`copyout_zero` `(` value-list `)`)?
+///                             (`create` `(` value-list `)`)?
+///                             (`create_zero` `(` value-list `)`)?
+///                             (`no_create` `(` value-list `)`)?
+///                             (`delete` `(` value-list `)`)?
+///                             (`attach` `(` value-list `)`)?
+///                             (`detach` `(` value-list `)`)?
 ///                             region attr-dict?
 static ParseResult parseDataOp(OpAsmParser &parser, OperationState &result) {
   Builder &builder = parser.getBuilder();
-  SmallVector<OpAsmParser::OperandType, 8> presentOperands, copyOperands,
-      copyinOperands, copyoutOperands, createOperands, noCreateOperands,
+  SmallVector<OpAsmParser::OperandType, 2> presentOperands, copyOperands,
+      copyinOperands, copyinReadonlyOperands, copyoutOperands,
+      copyoutZeroOperands, createOperands, createZeroOperands, noCreateOperands,
       deleteOperands, attachOperands, detachOperands;
-  SmallVector<Type, 8> operandsTypes;
+  SmallVector<Type, 2> presentOperandTypes, copyOperandTypes,
+      copyinOperandTypes, copyinReadonlyOperandTypes, copyoutOperandTypes,
+      copyoutZeroOperandTypes, createOperandTypes, createZeroOperandTypes,
+      noCreateOperandTypes, deleteOperandTypes, attachOperandTypes,
+      detachOperandTypes;
 
   // present(value-list)?
   if (failed(parseOperandList(parser, DataOp::getPresentKeyword(),
-                              presentOperands, operandsTypes, result)))
+                              presentOperands, presentOperandTypes, result)))
     return failure();
 
   // copy(value-list)?
   if (failed(parseOperandList(parser, DataOp::getCopyKeyword(), copyOperands,
-                              operandsTypes, result)))
+                              copyOperandTypes, result)))
     return failure();
 
   // copyin(value-list)?
   if (failed(parseOperandList(parser, DataOp::getCopyinKeyword(),
-                              copyinOperands, operandsTypes, result)))
+                              copyinOperands, copyinOperandTypes, result)))
+    return failure();
+
+  // copyin_readonly(value-list)?
+  if (failed(parseOperandList(parser, DataOp::getCopyinReadonlyKeyword(),
+                              copyinReadonlyOperands, copyinOperandTypes,
+                              result)))
     return failure();
 
   // copyout(value-list)?
   if (failed(parseOperandList(parser, DataOp::getCopyoutKeyword(),
-                              copyoutOperands, operandsTypes, result)))
+                              copyoutOperands, copyoutOperandTypes, result)))
+    return failure();
+
+  // copyout_zero(value-list)?
+  if (failed(parseOperandList(parser, DataOp::getCopyoutZeroKeyword(),
+                              copyoutZeroOperands, copyoutZeroOperandTypes,
+                              result)))
     return failure();
 
   // create(value-list)?
   if (failed(parseOperandList(parser, DataOp::getCreateKeyword(),
-                              createOperands, operandsTypes, result)))
+                              createOperands, createOperandTypes, result)))
+    return failure();
+
+  // create_zero(value-list)?
+  if (failed(parseOperandList(parser, DataOp::getCreateZeroKeyword(),
+                              createZeroOperands, createZeroOperandTypes,
+                              result)))
     return failure();
 
   // no_create(value-list)?
-  if (failed(parseOperandList(parser, DataOp::getCreateKeyword(),
-                              noCreateOperands, operandsTypes, result)))
+  if (failed(parseOperandList(parser, DataOp::getNoCreateKeyword(),
+                              noCreateOperands, noCreateOperandTypes, result)))
     return failure();
 
   // delete(value-list)?
   if (failed(parseOperandList(parser, DataOp::getDeleteKeyword(),
-                              deleteOperands, operandsTypes, result)))
+                              deleteOperands, deleteOperandTypes, result)))
     return failure();
 
   // attach(value-list)?
   if (failed(parseOperandList(parser, DataOp::getAttachKeyword(),
-                              attachOperands, operandsTypes, result)))
+                              attachOperands, attachOperandTypes, result)))
     return failure();
 
   // detach(value-list)?
   if (failed(parseOperandList(parser, DataOp::getDetachKeyword(),
-                              detachOperands, operandsTypes, result)))
+                              detachOperands, detachOperandTypes, result)))
     return failure();
 
   // Data op region
   if (failed(parseRegions<ParallelOp>(parser, result)))
     return failure();
 
-  result.addAttribute(
-      ParallelOp::getOperandSegmentSizeAttr(),
-      builder.getI32VectorAttr({static_cast<int32_t>(presentOperands.size()),
-                                static_cast<int32_t>(copyOperands.size()),
-                                static_cast<int32_t>(copyinOperands.size()),
-                                static_cast<int32_t>(copyoutOperands.size()),
-                                static_cast<int32_t>(createOperands.size()),
-                                static_cast<int32_t>(noCreateOperands.size()),
-                                static_cast<int32_t>(deleteOperands.size()),
-                                static_cast<int32_t>(attachOperands.size()),
-                                static_cast<int32_t>(detachOperands.size())}));
+  result.addAttribute(ParallelOp::getOperandSegmentSizeAttr(),
+                      builder.getI32VectorAttr(
+                          {static_cast<int32_t>(presentOperands.size()),
+                           static_cast<int32_t>(copyinReadonlyOperands.size()),
+                           static_cast<int32_t>(copyoutZeroOperands.size()),
+                           static_cast<int32_t>(createZeroOperands.size()),
 
   // Additional attributes
   if (failed(parser.parseOptionalAttrDictWithKeyword(result.attributes)))
     return failure();
 
   return success();
 }
 
 static void print(OpAsmPrinter &printer, DataOp &op) {
   printer << DataOp::getOperationName();
 
   // present(value-list)?
   printOperandList(op.presentOperands(), DataOp::getPresentKeyword(), printer);
 
   // copy(value-list)?
   printOperandList(op.copyOperands(), DataOp::getCopyKeyword(), printer);
 
   // copyin(value-list)?
   printOperandList(op.copyinOperands(), DataOp::getCopyinKeyword(), printer);
 
+  // copyin_readonly(value-list)?
+  printOperandList(op.copyinReadonlyOperands(),
+                   DataOp::getCopyinReadonlyKeyword(), printer);
+
   // copyout(value-list)?
   printOperandList(op.copyoutOperands(), DataOp::getCopyoutKeyword(), printer);
 
+  // copyout(value-list)?
+  printOperandList(op.copyoutZeroOperands(), DataOp::getCopyoutZeroKeyword(),
+                   printer);
+
   // create(value-list)?
   printOperandList(op.createOperands(), DataOp::getCreateKeyword(), printer);
 
+  // create_zero(value-list)?
+  printOperandList(op.createZeroOperands(), DataOp::getCreateZeroKeyword(),
+                   printer);
+
   // no_create(value-list)?
   printOperandList(op.noCreateOperands(), DataOp::getNoCreateKeyword(),
                    printer);
 
   // delete(value-list)?
   printOperandList(op.deleteOperands(), DataOp::getDeleteKeyword(), printer);
 
   // attach(value-list)?

mlir/test/Dialect/OpenACC/ops.mlir

 // CHECK:      acc.parallel deviceptr([[ARGA]]: memref<10xf32>) attach([[ARGB]]: memref<10xf32>, [[ARGC]]: memref<10x10xf32>) {
 // CHECK-NEXT: }
 // CHECK:      acc.parallel private([[ARGA]]: memref<10xf32>, [[ARGC]]: memref<10x10xf32>) firstprivate([[ARGB]]: memref<10xf32>) {
 // CHECK-NEXT: }
 // CHECK:      acc.parallel {
 // CHECK-NEXT: } attributes {defaultAttr = "none"}
 // CHECK:      acc.parallel {
 // CHECK-NEXT: } attributes {defaultAttr = "present"}
+
+// -----
+
+func @testdataop(%a: memref<10xf32>, %b: memref<10xf32>, %c: memref<10x10xf32>) -> () {
+  acc.data present(%a: memref<10xf32>, %b: memref<10xf32>, %c: memref<10x10xf32>) {
+  }
+  acc.data copy(%a: memref<10xf32>, %b: memref<10xf32>, %c: memref<10x10xf32>) {
+  }
+  acc.data copyin(%a: memref<10xf32>, %b: memref<10xf32>, %c: memref<10x10xf32>) {
+  }
+  acc.data copyin_readonly(%a: memref<10xf32>, %b: memref<10xf32>, %c: memref<10x10xf32>) {
+  }
+  acc.data copyout(%a: memref<10xf32>, %b: memref<10xf32>, %c: memref<10x10xf32>) {
+  }
+  acc.data copyout_zero(%a: memref<10xf32>, %b: memref<10xf32>, %c: memref<10x10xf32>) {
+  }
+  acc.data create(%a: memref<10xf32>, %b: memref<10xf32>, %c: memref<10x10xf32>) {
+  }
+  acc.data create_zero(%a: memref<10xf32>, %b: memref<10xf32>, %c: memref<10x10xf32>) {
+  }
+  acc.data no_create(%a: memref<10xf32>, %b: memref<10xf32>, %c: memref<10x10xf32>) {
+  }
+  acc.data delete(%a: memref<10xf32>, %b: memref<10xf32>, %c: memref<10x10xf32>) {
+  }
+  acc.data attach(%a: memref<10xf32>, %b: memref<10xf32>, %c: memref<10x10xf32>) {
+  }
+  acc.data detach(%a: memref<10xf32>, %b: memref<10xf32>, %c: memref<10x10xf32>) {
+  }
+  acc.data present(%a: memref<10xf32>) copyin(%b: memref<10xf32>) copyout(%c: memref<10x10xf32>) {
+  }
+  return
+}
+
+// CHECK:      func @testdataop([[ARGA:%.*]]: memref<10xf32>, [[ARGB:%.*]]: memref<10xf32>, [[ARGC:%.*]]: memref<10x10xf32>) {
+// CHECK:      acc.data present([[ARGA]]: memref<10xf32>, [[ARGB]]: memref<10xf32>, [[ARGC]]: memref<10x10xf32>) {
+// CHECK-NEXT: }
+// CHECK:      acc.data copy([[ARGA]]: memref<10xf32>, [[ARGB]]: memref<10xf32>, [[ARGC]]: memref<10x10xf32>) {
+// CHECK-NEXT: }
+// CHECK:      acc.data copyin([[ARGA]]: memref<10xf32>, [[ARGB]]: memref<10xf32>, [[ARGC]]: memref<10x10xf32>) {
+// CHECK-NEXT: }
+// CHECK:      acc.data copyin_readonly([[ARGA]]: memref<10xf32>, [[ARGB]]: memref<10xf32>, [[ARGC]]: memref<10x10xf32>) {
+// CHECK-NEXT: }
+// CHECK:      acc.data copyout([[ARGA]]: memref<10xf32>, [[ARGB]]: memref<10xf32>, [[ARGC]]: memref<10x10xf32>) {
+// CHECK-NEXT: }
+// CHECK:      acc.data copyout_zero([[ARGA]]: memref<10xf32>, [[ARGB]]: memref<10xf32>, [[ARGC]]: memref<10x10xf32>) {
+// CHECK-NEXT: }
+// CHECK:      acc.data create([[ARGA]]: memref<10xf32>, [[ARGB]]: memref<10xf32>, [[ARGC]]: memref<10x10xf32>) {
+// CHECK-NEXT: }
+// CHECK:      acc.data create_zero([[ARGA]]: memref<10xf32>, [[ARGB]]: memref<10xf32>, [[ARGC]]: memref<10x10xf32>) {
+// CHECK-NEXT: }
+// CHECK:      acc.data no_create([[ARGA]]: memref<10xf32>, [[ARGB]]: memref<10xf32>, [[ARGC]]: memref<10x10xf32>) {
+// CHECK-NEXT: }
+// CHECK:      acc.data delete([[ARGA]]: memref<10xf32>, [[ARGB]]: memref<10xf32>, [[ARGC]]: memref<10x10xf32>) {
+// CHECK-NEXT: }
+// CHECK:      acc.data attach([[ARGA]]: memref<10xf32>, [[ARGB]]: memref<10xf32>, [[ARGC]]: memref<10x10xf32>) {
+// CHECK-NEXT: }
+// CHECK:      acc.data detach([[ARGA]]: memref<10xf32>, [[ARGB]]: memref<10xf32>, [[ARGC]]: memref<10x10xf32>) {
+// CHECK-NEXT: }
+// CHECK:      acc.data present([[ARGA]]: memref<10xf32>) copyin([[ARGB]]: memref<10xf32>) copyout([[ARGC]]: memref<10x10xf32>) {
+// CHECK-NEXT: }