[mlir][openacc] Add loop op verifier

Add a verifier for the loop op in the OpenACC dialect. Check basic restriction
from 2.9 Loop construct from the OpenACC 3.0 specs.

Reviewed By: ftynse

Differential Revision: https://reviews.llvm.org/D87546

diff --git a/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td b/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
index c0178eb..0d37215 100644 (file)
--- a/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
+++ b/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
@@ -200,7 +200,8 @@ def OpenACC_TerminatorOp : OpenACC_Op<"terminator", [Terminator]> {
 //===----------------------------------------------------------------------===//
 
 def OpenACC_LoopOp : OpenACC_Op<"loop",
-                        [AttrSizedOperandSegments]> {
+      [AttrSizedOperandSegments,
+       SingleBlockImplicitTerminator<"acc::YieldOp">]> {
   let summary = "loop construct";
 
   let description = [{
@@ -228,13 +229,14 @@ def OpenACC_LoopOp : OpenACC_Op<"loop",
                        Optional<AnyInteger>:$gangStatic,
                        Optional<AnyInteger>:$workerNum,
                        Optional<AnyInteger>:$vectorLength,
-                       UnitAttr:$loopSeq,
-                       UnitAttr:$loopIndependent,
-                       UnitAttr:$loopAuto,
+                       UnitAttr:$seq,
+                       UnitAttr:$independent,
+                       UnitAttr:$auto_,
                        Variadic<AnyInteger>:$tileOperands,
                        Variadic<AnyType>:$privateOperands,
                        OptionalAttr<OpenACC_ReductionOpAttr>:$reductionOp,
-                       Variadic<AnyType>:$reductionOperands);
+                       Variadic<AnyType>:$reductionOperands,
+                       DefaultValuedAttr<I64Attr, "0">:$exec_mapping);
 
   let results = (outs Variadic<AnyType>:$results);
 
@@ -256,7 +258,7 @@ def OpenACC_LoopOp : OpenACC_Op<"loop",
     static StringRef getReductionKeyword() { return "reduction"; }
   }];
 
-  let verifier = ?;
+  let verifier = [{ return ::verifyLoopOp(*this); }];
 }
 
 // Yield operation for the acc.loop and acc.parallel operations.

diff --git a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
index 11a7748..3e4d1c3 100644 (file)
--- a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
+++ b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
@@ -487,7 +487,7 @@ static void print(OpAsmPrinter &printer, DataOp &op) {
 ///                         region attr-dict?
 static ParseResult parseLoopOp(OpAsmParser &parser, OperationState &result) {
   Builder &builder = parser.getBuilder();
-  unsigned executionMapping = 0;
+  unsigned executionMapping = OpenACCExecMapping::NONE;
   SmallVector<Type, 8> operandTypes;
   SmallVector<OpAsmParser::OperandType, 8> privateOperands, reductionOperands;
   SmallVector<OpAsmParser::OperandType, 8> tileOperands;
@@ -567,7 +567,7 @@ static ParseResult parseLoopOp(OpAsmParser &parser, OperationState &result) {
                               reductionOperands, operandTypes, result)))
     return failure();
 
-  if (executionMapping != 0)
+  if (executionMapping != acc::OpenACCExecMapping::NONE)
     result.addAttribute(LoopOp::getExecutionMappingAttrName(),
                         builder.getI64IntegerAttr(executionMapping));
 
@@ -597,13 +597,7 @@ static ParseResult parseLoopOp(OpAsmParser &parser, OperationState &result) {
 static void print(OpAsmPrinter &printer, LoopOp &op) {
   printer << LoopOp::getOperationName();
 
-  unsigned execMapping =
-      (op.getAttrOfType<IntegerAttr>(LoopOp::getExecutionMappingAttrName()) !=
-       nullptr)
-          ? op.getAttrOfType<IntegerAttr>(LoopOp::getExecutionMappingAttrName())
-                .getInt()
-          : 0;
-
+  unsigned execMapping = op.exec_mapping();
   if (execMapping & OpenACCExecMapping::GANG) {
     printer << " " << LoopOp::getGangKeyword();
     Value gangNum = op.gangNum();
@@ -661,5 +655,31 @@ static void print(OpAsmPrinter &printer, LoopOp &op) {
                       LoopOp::getOperandSegmentSizeAttr()});
 }
 
+static LogicalResult verifyLoopOp(acc::LoopOp loopOp) {
+  // auto, independent and seq attribute are mutually exclusive.
+  if ((loopOp.auto_() && (loopOp.independent() || loopOp.seq())) ||
+      (loopOp.independent() && loopOp.seq())) {
+    loopOp.emitError("only one of " + acc::LoopOp::getAutoAttrName() + ", " +
+                     acc::LoopOp::getIndependentAttrName() + ", " +
+                     acc::LoopOp::getSeqAttrName() +
+                     " can be present at the same time");
+    return failure();
+  }
+
+  // Gang, worker and vector are incompatible with seq.
+  if (loopOp.seq() && loopOp.exec_mapping() != OpenACCExecMapping::NONE) {
+    loopOp.emitError("gang, worker or vector cannot appear with the seq attr");
+    return failure();
+  }
+
+  // Check non-empty body().
+  if (loopOp.region().empty()) {
+    loopOp.emitError("expected non-empty body.");
+    return failure();
+  }
+
+  return success();
+}
+
 #define GET_OP_CLASSES
 #include "mlir/Dialect/OpenACC/OpenACCOps.cpp.inc"

diff --git a/mlir/test/Dialect/OpenACC/invalid.mlir b/mlir/test/Dialect/OpenACC/invalid.mlir
new file mode 100644 (file)
index 0000000..61a1321
--- /dev/null
+++ b/mlir/test/Dialect/OpenACC/invalid.mlir
@@ -0,0 +1,70 @@
+// RUN: mlir-opt -split-input-file -verify-diagnostics %s
+
+// expected-error@+1 {{gang, worker or vector cannot appear with the seq attr}}
+acc.loop gang {
+  "some.op"() : () -> ()
+  acc.yield
+} attributes {seq}
+
+// -----
+
+// expected-error@+1 {{gang, worker or vector cannot appear with the seq attr}}
+acc.loop worker {
+  "some.op"() : () -> ()
+  acc.yield
+} attributes {seq}
+
+// -----
+
+// expected-error@+1 {{gang, worker or vector cannot appear with the seq attr}}
+acc.loop vector {
+  "some.op"() : () -> ()
+  acc.yield
+} attributes {seq}
+
+// -----
+
+// expected-error@+1 {{gang, worker or vector cannot appear with the seq attr}}
+acc.loop gang worker {
+  "some.op"() : () -> ()
+  acc.yield
+} attributes {seq}
+
+// -----
+
+// expected-error@+1 {{gang, worker or vector cannot appear with the seq attr}}
+acc.loop gang vector {
+  "some.op"() : () -> ()
+  acc.yield
+} attributes {seq}
+
+// -----
+
+// expected-error@+1 {{gang, worker or vector cannot appear with the seq attr}}
+acc.loop worker vector {
+  "some.op"() : () -> ()
+  acc.yield
+} attributes {seq}
+
+// -----
+
+// expected-error@+1 {{gang, worker or vector cannot appear with the seq attr}}
+acc.loop gang worker vector {
+  "some.op"() : () -> ()
+  acc.yield
+} attributes {seq}
+
+// -----
+
+// expected-error@+1 {{expected non-empty body.}}
+acc.loop {
+}
+
+// -----
+
+// expected-error@+1 {{only one of auto, independent, seq can be present at the same time}}
+acc.loop {
+  acc.yield
+} attributes {auto_, seq}
+
+// -----

diff --git a/mlir/test/Dialect/OpenACC/ops.mlir b/mlir/test/Dialect/OpenACC/ops.mlir
index b534f70..b1a78c6 100644 (file)
--- a/mlir/test/Dialect/OpenACC/ops.mlir
+++ b/mlir/test/Dialect/OpenACC/ops.mlir
@@ -1,8 +1,8 @@
-// RUN: mlir-opt %s | FileCheck %s
+// RUN: mlir-opt -allow-unregistered-dialect %s | FileCheck %s
 // Verify the printed output can be parsed.
-// RUN: mlir-opt %s | mlir-opt | FileCheck %s
+// RUN: mlir-opt -allow-unregistered-dialect %s | mlir-opt -allow-unregistered-dialect  | FileCheck %s
 // Verify the generic form can be parsed.
-// RUN: mlir-opt -mlir-print-op-generic %s | mlir-opt | FileCheck %s
+// RUN: mlir-opt -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
@@ -186,27 +186,43 @@ func @compute3(%a: memref<10x10xf32>, %b: memref<10x10xf32>, %c: memref<10xf32>,
 // CHECK-NEXT:   return %{{.*}} : memref<10xf32>
 // CHECK-NEXT: }
 
-func @testop() -> () {
+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"() : () -> ()
+    acc.yield
   }
   acc.loop gang(num: %gangNum) {
+    "some.op"() : () -> ()
+    acc.yield
   }
   acc.loop gang(static: %gangStatic) {
+    "some.op"() : () -> ()
+    acc.yield
   }
   acc.loop worker(%workerNum) {
+    "some.op"() : () -> ()
+    acc.yield
   }
   acc.loop vector(%vectorLength) {
+    "some.op"() : () -> ()
+    acc.yield
   }
   acc.loop gang(num: %gangNum) worker vector {
+    "some.op"() : () -> ()
+    acc.yield
   }
   acc.loop gang(num: %gangNum, static: %gangStatic) worker(%workerNum) vector(%vectorLength) {
+    "some.op"() : () -> ()
+    acc.yield
   }
   acc.loop tile(%tileSize : i64, %tileSize : i64) {
+    "some.op"() : () -> ()
+    acc.yield
   }
   return
 }
@@ -217,20 +233,36 @@ func @testop() -> () {
 // CHECK-NEXT: [[GANGSTATIC:%.*]] = constant 2 : i64
 // CHECK-NEXT: [[TILESIZE:%.*]] = constant 2 : i64
 // CHECK-NEXT: acc.loop gang worker vector {
+// CHECK-NEXT:   "some.op"() : () -> ()
+// CHECK-NEXT:   acc.yield
 // CHECK-NEXT: }
 // CHECK-NEXT: acc.loop gang(num: [[GANGNUM]]) {
+// CHECK-NEXT:   "some.op"() : () -> ()
+// CHECK-NEXT:   acc.yield
 // CHECK-NEXT: }
 // CHECK-NEXT: acc.loop gang(static: [[GANGSTATIC]]) {
+// CHECK-NEXT:   "some.op"() : () -> ()
+// CHECK-NEXT:   acc.yield
 // CHECK-NEXT: }
 // CHECK-NEXT: acc.loop worker([[WORKERNUM]]) {
+// CHECK-NEXT:   "some.op"() : () -> ()
+// CHECK-NEXT:   acc.yield
 // CHECK-NEXT: }
 // CHECK-NEXT: acc.loop vector([[VECTORLENGTH]]) {
+// CHECK-NEXT:   "some.op"() : () -> ()
+// CHECK-NEXT:   acc.yield
 // CHECK-NEXT: }
 // CHECK-NEXT: acc.loop gang(num: [[GANGNUM]]) worker vector {
+// CHECK-NEXT:   "some.op"() : () -> ()
+// CHECK-NEXT:   acc.yield
 // CHECK-NEXT: }
 // CHECK-NEXT: acc.loop gang(num: [[GANGNUM]], static: [[GANGSTATIC]]) worker([[WORKERNUM]]) vector([[VECTORLENGTH]]) {
+// 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: }