[mlir][openacc] Add verifier for dataOperands on compute operations

[clementval]

Summary

Data operands associated with acc.parallel, acc.serial and
acc.kernels should comes from acc data entry/exit operations
or acc.getdeviceptr.

[razvanlupusoru]

Yes! Thank you for adding these verification checks!

[jeanPerier]

Looks good

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

oilist(
       | `vector_length` `(` $vectorLength `:` type($vectorLength) `)`
       | `wait` `(` $waitOperands `:` type($waitOperands) `)`
       | `self` `(` $selfCond `)`
       | `if` `(` $ifCond `)`
       | `reduction` `(` $reductionOperands `:` type($reductionOperands) `)`
     )
     $region attr-dict-with-keyword
   }];
+
+  let hasVerifier = 1;
 }
 
 //===----------------------------------------------------------------------===//
 // 2.5.2 serial Construct
 //===----------------------------------------------------------------------===//
 
 def OpenACC_SerialOp : OpenACC_Op<"serial",
     [AttrSizedOperandSegments, RecursiveMemoryEffects]> {
oilist(
       | `present` `(` $presentOperands `:` type($presentOperands) `)`
       | `wait` `(` $waitOperands `:` type($waitOperands) `)`
       | `self` `(` $selfCond `)`
       | `if` `(` $ifCond `)`
       | `reduction` `(` $reductionOperands `:` type($reductionOperands) `)`
     )
     $region attr-dict-with-keyword
   }];
+
+  let hasVerifier = 1;
 }
 
 //===----------------------------------------------------------------------===//
 // 2.5.1 kernels Construct
 //===----------------------------------------------------------------------===//
 
 def OpenACC_KernelsOp : OpenACC_Op<"kernels",
     [AttrSizedOperandSegments, RecursiveMemoryEffects]> {
oilist(
       | `present` `(` $presentOperands `:` type($presentOperands) `)`
       | `vector_length` `(` $vectorLength `:` type($vectorLength) `)`
       | `wait` `(` $waitOperands `:` type($waitOperands) `)`
       | `self` `(` $selfCond `)`
       | `if` `(` $ifCond `)`
     )
     $region attr-dict-with-keyword
   }];
+
+  let hasVerifier = 1;
 }
 
 //===----------------------------------------------------------------------===//
 // 2.6.5 data Construct
 //===----------------------------------------------------------------------===//
 
 def OpenACC_DataOp : OpenACC_Op<"data",
     [AttrSizedOperandSegments, RecursiveMemoryEffects]> {

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

struct RemoveConstantIfCondition : public OpRewritePattern<OpTy> {
   }
 };
 } // namespace
 
 //===----------------------------------------------------------------------===//
 // ParallelOp
 //===----------------------------------------------------------------------===//
 
+/// Check dataOperands for acc.parallel, acc.serial and acc.kernels.
+template <typename Op>
+static LogicalResult checkDataOperands(Op op,
+                                       const mlir::ValueRange &operands) {
+  for (mlir::Value operand : operands)
+    if (!mlir::isa<acc::AttachOp, acc::CopyinOp, acc::CopyoutOp, acc::CreateOp,
+                   acc::DeleteOp, acc::DetachOp, acc::DevicePtrOp,
+                   acc::GetDevicePtrOp, acc::NoCreateOp, acc::PresentOp>(
+            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() +
          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;
   numOptional += getIfCond() ? 1 : 0;
   numOptional += getSelfCond() ? 1 : 0;
   return getOperand(getWaitOperands().size() + numOptional + i);
 }
 
+LogicalResult acc::ParallelOp::verify() {
+  return checkDataOperands<acc::ParallelOp>(*this, getDataClauseOperands());
+}
+
 //===----------------------------------------------------------------------===//
 // SerialOp
 //===----------------------------------------------------------------------===//
 
 unsigned SerialOp::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() +
          getGangFirstPrivateOperands().size() + getDataClauseOperands().size();
 }
 
 Value SerialOp::getDataOperand(unsigned i) {
   unsigned numOptional = getAsync() ? 1 : 0;
   numOptional += getIfCond() ? 1 : 0;
   numOptional += getSelfCond() ? 1 : 0;
   return getOperand(getWaitOperands().size() + numOptional + i);
 }
 
+LogicalResult acc::SerialOp::verify() {
+  return checkDataOperands<acc::SerialOp>(*this, getDataClauseOperands());
+}
+
 //===----------------------------------------------------------------------===//
 // KernelsOp
 //===----------------------------------------------------------------------===//
 
 unsigned KernelsOp::getNumDataOperands() {
   return getCopyOperands().size() + getCopyinOperands().size() +
          getCopyinReadonlyOperands().size() + getCopyoutOperands().size() +
          getCopyoutZeroOperands().size() + getCreateOperands().size() +
          getCreateZeroOperands().size() + getNoCreateOperands().size() +
          getPresentOperands().size() + getDevicePtrOperands().size() +
          getAttachOperands().size() + getDataClauseOperands().size();
 }
 
 Value KernelsOp::getDataOperand(unsigned i) {
   unsigned numOptional = getAsync() ? 1 : 0;
   numOptional += getIfCond() ? 1 : 0;
   numOptional += getSelfCond() ? 1 : 0;
   return getOperand(getWaitOperands().size() + numOptional + i);
 }
 
+LogicalResult acc::KernelsOp::verify() {
+  return checkDataOperands<acc::KernelsOp>(*this, getDataClauseOperands());
+}
+
 //===----------------------------------------------------------------------===//
 // LoopOp
 //===----------------------------------------------------------------------===//
 
 static ParseResult
 parseGangClause(OpAsmParser &parser,
                 std::optional<OpAsmParser::UnresolvedOperand> &gangNum,
                 Type &gangNumType,

mlir/test/Dialect/OpenACC/invalid.mlir

 // expected-error@+1 {{operand #0 must be integer or index, but got 'f32'}}
 %1 = acc.bounds lowerbound(%0 : f32)
 
 // -----
 
 %value = memref.alloc() : memref<10xf32>
 // expected-error@+1 {{expect data entry/exit operation or acc.getdeviceptr as defining op}}
 acc.update dataOperands(%value : memref<10xf32>)
+
+// -----
+
+%value = memref.alloc() : memref<10xf32>
+// expected-error@+1 {{expect data entry/exit operation or acc.getdeviceptr as defining op}}
+acc.parallel dataOperands(%value : memref<10xf32>) {
+  acc.yield
+}
+
+// -----
+
+%value = memref.alloc() : memref<10xf32>
+// expected-error@+1 {{expect data entry/exit operation or acc.getdeviceptr as defining op}}
+acc.serial dataOperands(%value : memref<10xf32>) {
+  acc.yield
+}
+
+// -----
+
+%value = memref.alloc() : memref<10xf32>
+// expected-error@+1 {{expect data entry/exit operation or acc.getdeviceptr as defining op}}
+acc.kernels dataOperands(%value : memref<10xf32>) {
+  acc.yield
+}