diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp index 533319c5a8fbb..f60668dd0cf99 100644 --- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp +++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp @@ -1964,6 +1964,39 @@ LogicalResult CriticalOp::verifySymbolUses(SymbolTableCollection &symbolTable) { // Ordered construct //===----------------------------------------------------------------------===// +static LogicalResult verifyOrderedParent(Operation &op) { + bool hasRegion = op.getNumRegions() > 0; + auto loopOp = op.getParentOfType(); + if (!loopOp) { + if (hasRegion) + return success(); + + // TODO: Consider if this needs to be the case only for the standalone + // variant of the ordered construct. + return op.emitOpError() << "must be nested inside of a loop"; + } + + Operation *wrapper = loopOp->getParentOp(); + if (auto wsloopOp = dyn_cast(wrapper)) { + IntegerAttr orderedAttr = wsloopOp.getOrderedValAttr(); + if (!orderedAttr) + return op.emitOpError() << "the enclosing worksharing-loop region must " + "have an ordered clause"; + + if (hasRegion && orderedAttr.getInt() != 0) + return op.emitOpError() << "the enclosing loop's ordered clause must not " + "have a parameter present"; + + if (!hasRegion && orderedAttr.getInt() == 0) + return op.emitOpError() << "the enclosing loop's ordered clause must " + "have a parameter present"; + } else if (!isa(wrapper)) { + return op.emitOpError() << "must be nested inside of a worksharing, simd " + "or worksharing simd loop"; + } + return success(); +} + void OrderedOp::build(OpBuilder &builder, OperationState &state, const OrderedOpClauseOps &clauses) { OrderedOp::build(builder, state, clauses.doacrossDependTypeAttr, @@ -1971,14 +2004,11 @@ void OrderedOp::build(OpBuilder &builder, OperationState &state, } LogicalResult OrderedOp::verify() { - auto container = (*this)->getParentOfType(); - if (!container || !container.getOrderedValAttr() || - container.getOrderedValAttr().getInt() == 0) - return emitOpError() << "ordered depend directive must be closely " - << "nested inside a worksharing-loop with ordered " - << "clause with parameter present"; - - if (container.getOrderedValAttr().getInt() != (int64_t)*getNumLoopsVal()) + if (failed(verifyOrderedParent(**this))) + return failure(); + + auto wrapper = (*this)->getParentOfType(); + if (!wrapper || *wrapper.getOrderedVal() != *getNumLoopsVal()) return emitOpError() << "number of variables in depend clause does not " << "match number of iteration variables in the " << "doacross loop"; @@ -1996,15 +2026,7 @@ LogicalResult OrderedRegionOp::verify() { if (getSimd()) return failure(); - if (auto container = (*this)->getParentOfType()) { - if (!container.getOrderedValAttr() || - container.getOrderedValAttr().getInt() != 0) - return emitOpError() << "ordered region must be closely nested inside " - << "a worksharing-loop region with an ordered " - << "clause without parameter present"; - } - - return success(); + return verifyOrderedParent(**this); } //===----------------------------------------------------------------------===// @@ -2149,15 +2171,19 @@ LogicalResult CancelOp::verify() { << "inside a parallel region"; } if (cct == ClauseCancellationConstructType::Loop) { - if (!isa(parentOp)) { - return emitOpError() << "cancel loop must appear " - << "inside a worksharing-loop region"; + auto loopOp = dyn_cast(parentOp); + auto wsloopOp = llvm::dyn_cast_if_present( + loopOp ? loopOp->getParentOp() : nullptr); + + if (!wsloopOp) { + return emitOpError() + << "cancel loop must appear inside a worksharing-loop region"; } - if (cast(parentOp).getNowaitAttr()) { + if (wsloopOp.getNowaitAttr()) { return emitError() << "A worksharing construct that is canceled " << "must not have a nowait clause"; } - if (cast(parentOp).getOrderedValAttr()) { + if (wsloopOp.getOrderedValAttr()) { return emitError() << "A worksharing construct that is canceled " << "must not have an ordered clause"; } @@ -2195,7 +2221,7 @@ LogicalResult CancellationPointOp::verify() { << "inside a parallel region"; } if ((cct == ClauseCancellationConstructType::Loop) && - !isa(parentOp)) { + (!isa(parentOp) || !isa(parentOp->getParentOp()))) { return emitOpError() << "cancellation point loop must appear " << "inside a worksharing-loop region"; } diff --git a/mlir/test/Dialect/OpenMP/invalid.mlir b/mlir/test/Dialect/OpenMP/invalid.mlir index 920d72f066192..e329b3010017c 100644 --- a/mlir/test/Dialect/OpenMP/invalid.mlir +++ b/mlir/test/Dialect/OpenMP/invalid.mlir @@ -748,10 +748,10 @@ omp.critical.declare @mutex hint(invalid_hint) // ----- -func.func @omp_ordered1(%arg1 : i32, %arg2 : i32, %arg3 : i32) -> () { - omp.wsloop ordered(1) { - omp.loop_nest (%0) : i32 = (%arg1) to (%arg2) step (%arg3) { - // expected-error @below {{ordered region must be closely nested inside a worksharing-loop region with an ordered clause without parameter present}} +func.func @omp_ordered_region1(%x : i32) -> () { + omp.distribute { + omp.loop_nest (%i) : i32 = (%x) to (%x) step (%x) { + // expected-error @below {{op must be nested inside of a worksharing, simd or worksharing simd loop}} omp.ordered.region { omp.terminator } @@ -764,10 +764,26 @@ func.func @omp_ordered1(%arg1 : i32, %arg2 : i32, %arg3 : i32) -> () { // ----- -func.func @omp_ordered2(%arg1 : i32, %arg2 : i32, %arg3 : i32) -> () { +func.func @omp_ordered_region2(%x : i32) -> () { omp.wsloop { - omp.loop_nest (%0) : i32 = (%arg1) to (%arg2) step (%arg3) { - // expected-error @below {{ordered region must be closely nested inside a worksharing-loop region with an ordered clause without parameter present}} + omp.loop_nest (%i) : i32 = (%x) to (%x) step (%x) { + // expected-error @below {{the enclosing worksharing-loop region must have an ordered clause}} + omp.ordered.region { + omp.terminator + } + omp.yield + } + omp.terminator + } + return +} + +// ----- + +func.func @omp_ordered_region3(%x : i32) -> () { + omp.wsloop ordered(1) { + omp.loop_nest (%i) : i32 = (%x) to (%x) step (%x) { + // expected-error @below {{the enclosing loop's ordered clause must not have a parameter present}} omp.ordered.region { omp.terminator } @@ -780,26 +796,54 @@ func.func @omp_ordered2(%arg1 : i32, %arg2 : i32, %arg3 : i32) -> () { // ----- -func.func @omp_ordered3(%vec0 : i64) -> () { - // expected-error @below {{ordered depend directive must be closely nested inside a worksharing-loop with ordered clause with parameter present}} +func.func @omp_ordered1(%vec0 : i64) -> () { + // expected-error @below {{op must be nested inside of a loop}} omp.ordered depend_type(dependsink) depend_vec(%vec0 : i64) {num_loops_val = 1 : i64} return } // ----- +func.func @omp_ordered2(%arg1 : i32, %arg2 : i32, %arg3 : i32, %vec0 : i64) -> () { + omp.distribute { + omp.loop_nest (%0) : i32 = (%arg1) to (%arg2) step (%arg3) { + // expected-error @below {{op must be nested inside of a worksharing, simd or worksharing simd loop}} + omp.ordered depend_type(dependsink) depend_vec(%vec0 : i64) {num_loops_val = 1 : i64} + omp.yield + } + omp.terminator + } + return +} + +// ----- + +func.func @omp_ordered3(%arg1 : i32, %arg2 : i32, %arg3 : i32, %vec0 : i64) -> () { + omp.wsloop { + omp.loop_nest (%0) : i32 = (%arg1) to (%arg2) step (%arg3) { + // expected-error @below {{the enclosing worksharing-loop region must have an ordered clause}} + omp.ordered depend_type(dependsink) depend_vec(%vec0 : i64) {num_loops_val = 1 : i64} + omp.yield + } + omp.terminator + } + return +} + +// ----- + func.func @omp_ordered4(%arg1 : i32, %arg2 : i32, %arg3 : i32, %vec0 : i64) -> () { omp.wsloop ordered(0) { omp.loop_nest (%0) : i32 = (%arg1) to (%arg2) step (%arg3) { - // expected-error @below {{ordered depend directive must be closely nested inside a worksharing-loop with ordered clause with parameter present}} + // expected-error @below {{the enclosing loop's ordered clause must have a parameter present}} omp.ordered depend_type(dependsink) depend_vec(%vec0 : i64) {num_loops_val = 1 : i64} - omp.yield } omp.terminator } return } + // ----- func.func @omp_ordered5(%arg1 : i32, %arg2 : i32, %arg3 : i32, %vec0 : i64, %vec1 : i64) -> () { @@ -807,7 +851,6 @@ func.func @omp_ordered5(%arg1 : i32, %arg2 : i32, %arg3 : i32, %vec0 : i64, %vec omp.loop_nest (%0) : i32 = (%arg1) to (%arg2) step (%arg3) { // expected-error @below {{number of variables in depend clause does not match number of iteration variables in the doacross loop}} omp.ordered depend_type(dependsource) depend_vec(%vec0, %vec1 : i64, i64) {num_loops_val = 2 : i64} - omp.yield } omp.terminator