[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

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

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

 //===----------------------------------------------------------------------===//
 // DataOp
 //===----------------------------------------------------------------------===//
 
 /// Parse acc.data operation
 /// 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,
+      copyinOperands, copyinReadonlyOperands, copyoutOperands,
+      copyoutZeroOperands, createOperands, createZeroOperands, noCreateOperands,
       deleteOperands, attachOperands, detachOperands;
-  SmallVector<Type, 8> operandsTypes;
+  SmallVector<Type, 8> 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

-// RUN: mlir-opt -allow-unregistered-dialect %s | FileCheck %s
+// RUN: mlir-opt -split-input-file -allow-unregistered-dialect %s | FileCheck %s
 // Verify the printed output can be parsed.
-// RUN: mlir-opt -allow-unregistered-dialect %s | mlir-opt -allow-unregistered-dialect  | FileCheck %s
+// RUN: mlir-opt -split-input-file -allow-unregistered-dialect %s | mlir-opt -allow-unregistered-dialect  | FileCheck %s
 // Verify the generic form can be parsed.
-// RUN: mlir-opt -allow-unregistered-dialect -mlir-print-op-generic %s | mlir-opt -allow-unregistered-dialect | FileCheck %s
+// RUN: mlir-opt -split-input-file -allow-unregistered-dialect -mlir-print-op-generic %s | mlir-opt -allow-unregistered-dialect | FileCheck %s
 
 func @compute1(%A: memref<10x10xf32>, %B: memref<10x10xf32>, %C: memref<10x10xf32>) -> memref<10x10xf32> {
   %c0 = constant 0 : index
   %c10 = constant 10 : index
   %c1 = constant 1 : index
   %async = constant 1 : i64
 
   acc.parallel async(%async: i64) {

 //  CHECK-NEXT:       }
 //  CHECK-NEXT:       acc.yield
 //  CHECK-NEXT:     } attributes {collapse = 3 : i64}
 //  CHECK-NEXT:     acc.yield
 //  CHECK-NEXT:   }
 //  CHECK-NEXT:   return %{{.*}} : memref<10x10xf32>
 //  CHECK-NEXT: }
 
+// -----
+
 func @compute2(%A: memref<10x10xf32>, %B: memref<10x10xf32>, %C: memref<10x10xf32>) -> memref<10x10xf32> {
   %c0 = constant 0 : index
   %c10 = constant 10 : index
   %c1 = constant 1 : index
 
   acc.parallel {
     acc.loop {
       scf.for %arg3 = %c0 to %c10 step %c1 {

 //  CHECK-NEXT:       }
 //  CHECK-NEXT:       acc.yield
 //  CHECK-NEXT:     } attributes {seq}
 //  CHECK-NEXT:     acc.yield
 //  CHECK-NEXT:   }
 //  CHECK-NEXT:   return %{{.*}} : memref<10x10xf32>
 //  CHECK-NEXT: }
 
+// -----
 
 func @compute3(%a: memref<10x10xf32>, %b: memref<10x10xf32>, %c: memref<10xf32>, %d: memref<10xf32>) -> memref<10xf32> {
   %lb = constant 0 : index
   %st = constant 1 : index
   %c10 = constant 10 : index
   %numGangs = constant 10 : i64
   %numWorkers = constant 10 : i64
 

 // CHECK-NEXT:       }
 // CHECK-NEXT:       acc.yield
 // CHECK-NEXT:     }
 // CHECK-NEXT:     acc.terminator
 // CHECK-NEXT:   }
 // CHECK-NEXT:   return %{{.*}} : memref<10xf32>
 // CHECK-NEXT: }
 
+// -----
+
 func @testop(%a: memref<10xf32>) -> () {
   %workerNum = constant 1 : i64
   %vectorLength = constant 128 : i64
   %gangNum = constant 8 : i64
   %gangStatic = constant 2 : i64
   %tileSize = constant 2 : i64
   acc.loop gang worker vector {
     "some.op"() : () -> ()

 // CHECK-NEXT:   "some.op"() : () -> ()
 // CHECK-NEXT:   acc.yield
 // CHECK-NEXT: }
 // CHECK-NEXT: acc.loop tile([[TILESIZE]]: i64, [[TILESIZE]]: i64) {
 // CHECK-NEXT:   "some.op"() : () -> ()
 // CHECK-NEXT:   acc.yield
 // CHECK-NEXT: }
 
+// -----
 
 func @testparallelop(%a: memref<10xf32>, %b: memref<10xf32>, %c: memref<10x10xf32>) -> () {
   %i64value = constant 1 : i64
   %i32value = constant 1 : i32
   %idxValue = constant 1 : index
   acc.parallel async(%i64value: i64) {
   }
   acc.parallel async(%i32value: i32) {

 // 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: }