[Mlir-commits] [mlir] 1465299 - [MLIR][OpenMP] Update op verifiers dependent on omp.wsloop (2/5) (#89211)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Wed Apr 24 06:28:43 PDT 2024
Author: Sergio Afonso
Date: 2024-04-24T14:28:39+01:00
New Revision: 1465299092c647fdb484235cc864826dca36f308
URL: https://github.com/llvm/llvm-project/commit/1465299092c647fdb484235cc864826dca36f308
DIFF: https://github.com/llvm/llvm-project/commit/1465299092c647fdb484235cc864826dca36f308.diff
LOG: [MLIR][OpenMP] Update op verifiers dependent on omp.wsloop (2/5) (#89211)
This patch updates verifiers for `omp.ordered`, `omp.ordered.region`,
`omp.cancel` and `omp.cancellation_point`, which check for a parent
`omp.wsloop`.
After transitioning to a loop wrapper-based approach, the expected
direct parent will become `omp.loop_nest` instead, so verifiers need to
take this into account.
This PR on its own will not pass premerge tests. All patches in the
stack are needed before it can be compiled and passes tests.
Added:
Modified:
mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
mlir/test/Dialect/OpenMP/invalid.mlir
Removed:
################################################################################
diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
index 533319c5a8fbb3..f60668dd0cf995 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<LoopNestOp>();
+ 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<WsloopOp>(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<SimdOp>(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<WsloopOp>();
- 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<WsloopOp>();
+ 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<WsloopOp>()) {
- 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<WsloopOp>(parentOp)) {
- return emitOpError() << "cancel loop must appear "
- << "inside a worksharing-loop region";
+ auto loopOp = dyn_cast<LoopNestOp>(parentOp);
+ auto wsloopOp = llvm::dyn_cast_if_present<WsloopOp>(
+ loopOp ? loopOp->getParentOp() : nullptr);
+
+ if (!wsloopOp) {
+ return emitOpError()
+ << "cancel loop must appear inside a worksharing-loop region";
}
- if (cast<WsloopOp>(parentOp).getNowaitAttr()) {
+ if (wsloopOp.getNowaitAttr()) {
return emitError() << "A worksharing construct that is canceled "
<< "must not have a nowait clause";
}
- if (cast<WsloopOp>(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<WsloopOp>(parentOp)) {
+ (!isa<LoopNestOp>(parentOp) || !isa<WsloopOp>(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 920d72f066192d..e329b3010017cf 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
More information about the Mlir-commits
mailing list