[llvm-branch-commits] [flang] [mlir] [OpenMP][MLIR] Set omp.composite attr for composite loop wrappers and add verifier checks (PR #102341)
Akash Banerjee via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Mon Aug 12 07:34:06 PDT 2024
https://github.com/TIFitis updated https://github.com/llvm/llvm-project/pull/102341
>From ba8cf358a98cfcce6b1239ac88391bc25bcc7197 Mon Sep 17 00:00:00 2001
From: Akash Banerjee <Akash.Banerjee at amd.com>
Date: Wed, 7 Aug 2024 18:31:08 +0100
Subject: [PATCH 1/3] [OpenMP][MLIR] Set omp.composite attr for composite loop
wrappers and add verifier checks
This patch sets the omp.composite unit attr for composite wrapper ops and also add appropriate checks to the verifiers of supported ops for the presence/absence of the attribute.
---
flang/lib/Lower/OpenMP/OpenMP.cpp | 8 +++++
mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp | 32 ++++++++++++++++++++
2 files changed, 40 insertions(+)
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index bbde77c14f36a1..3b18e7b3ecf80e 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -2063,10 +2063,14 @@ static void genCompositeDistributeSimd(
// TODO: Populate entry block arguments with private variables.
auto distributeOp = genWrapperOp<mlir::omp::DistributeOp>(
converter, loc, distributeClauseOps, /*blockArgTypes=*/{});
+ llvm::cast<mlir::omp::ComposableOpInterface>(distributeOp.getOperation())
+ .setComposite(/*val=*/true);
// TODO: Populate entry block arguments with reduction and private variables.
auto simdOp = genWrapperOp<mlir::omp::SimdOp>(converter, loc, simdClauseOps,
/*blockArgTypes=*/{});
+ llvm::cast<mlir::omp::ComposableOpInterface>(simdOp.getOperation())
+ .setComposite(/*val=*/true);
// Construct wrapper entry block list and associated symbols. It is important
// that the symbol order and the block argument order match, so that the
@@ -2111,10 +2115,14 @@ static void genCompositeDoSimd(lower::AbstractConverter &converter,
// TODO: Add private variables to entry block arguments.
auto wsloopOp = genWrapperOp<mlir::omp::WsloopOp>(
converter, loc, wsloopClauseOps, wsloopReductionTypes);
+ llvm::cast<mlir::omp::ComposableOpInterface>(wsloopOp.getOperation())
+ .setComposite(/*val=*/true);
// TODO: Populate entry block arguments with reduction and private variables.
auto simdOp = genWrapperOp<mlir::omp::SimdOp>(converter, loc, simdClauseOps,
/*blockArgTypes=*/{});
+ llvm::cast<mlir::omp::ComposableOpInterface>(simdOp.getOperation())
+ .setComposite(/*val=*/true);
// Construct wrapper entry block list and associated symbols. It is important
// that the symbol and block argument order match, so that the symbol-value
diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
index 11780f84697b15..641fbb5a1418f6 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -1546,6 +1546,9 @@ LogicalResult ParallelOp::verify() {
if (!isWrapper())
return emitOpError() << "must take a loop wrapper role if nested inside "
"of 'omp.distribute'";
+ if (!llvm::cast<ComposableOpInterface>(getOperation()).isComposite())
+ return emitError()
+ << "'omp.composite' attribute missing from composite wrapper";
if (LoopWrapperInterface nested = getNestedWrapper()) {
// Check for the allowed leaf constructs that may appear in a composite
@@ -1555,6 +1558,9 @@ LogicalResult ParallelOp::verify() {
} else {
return emitOpError() << "must not wrap an 'omp.loop_nest' directly";
}
+ } else if (llvm::cast<ComposableOpInterface>(getOperation()).isComposite()) {
+ return emitError()
+ << "'omp.composite' attribute present in non-composite wrapper";
}
if (getAllocateVars().size() != getAllocatorVars().size())
@@ -1749,10 +1755,18 @@ LogicalResult WsloopOp::verify() {
return emitOpError() << "must be a loop wrapper";
if (LoopWrapperInterface nested = getNestedWrapper()) {
+ if (!llvm::cast<ComposableOpInterface>(getOperation()).isComposite())
+ return emitError()
+ << "'omp.composite' attribute missing from composite wrapper";
+
// Check for the allowed leaf constructs that may appear in a composite
// construct directly after DO/FOR.
if (!isa<SimdOp>(nested))
return emitError() << "only supported nested wrapper is 'omp.simd'";
+
+ } else if (llvm::cast<ComposableOpInterface>(getOperation()).isComposite()) {
+ return emitError()
+ << "'omp.composite' attribute present in non-composite wrapper";
}
return verifyReductionVarList(*this, getReductionSyms(), getReductionVars(),
@@ -1796,6 +1810,10 @@ LogicalResult SimdOp::verify() {
if (getNestedWrapper())
return emitOpError() << "must wrap an 'omp.loop_nest' directly";
+ if (llvm::cast<ComposableOpInterface>(getOperation()).isComposite())
+ return emitError()
+ << "'omp.composite' attribute present in non-composite wrapper";
+
return success();
}
@@ -1825,11 +1843,17 @@ LogicalResult DistributeOp::verify() {
return emitOpError() << "must be a loop wrapper";
if (LoopWrapperInterface nested = getNestedWrapper()) {
+ if (!llvm::cast<ComposableOpInterface>(getOperation()).isComposite())
+ return emitError()
+ << "'omp.composite' attribute missing from composite wrapper";
// Check for the allowed leaf constructs that may appear in a composite
// construct directly after DISTRIBUTE.
if (!isa<ParallelOp, SimdOp>(nested))
return emitError() << "only supported nested wrappers are 'omp.parallel' "
"and 'omp.simd'";
+ } else if (llvm::cast<ComposableOpInterface>(getOperation()).isComposite()) {
+ return emitError()
+ << "'omp.composite' attribute present in non-composite wrapper";
}
return success();
@@ -2031,11 +2055,19 @@ LogicalResult TaskloopOp::verify() {
return emitOpError() << "must be a loop wrapper";
if (LoopWrapperInterface nested = getNestedWrapper()) {
+ if (!llvm::cast<ComposableOpInterface>(getOperation()).isComposite())
+ return emitError()
+ << "'omp.composite' attribute missing from composite wrapper";
+
// Check for the allowed leaf constructs that may appear in a composite
// construct directly after TASKLOOP.
if (!isa<SimdOp>(nested))
return emitError() << "only supported nested wrapper is 'omp.simd'";
+ } else if (llvm::cast<ComposableOpInterface>(getOperation()).isComposite()) {
+ return emitError()
+ << "'omp.composite' attribute present in non-composite wrapper";
}
+
return success();
}
>From 4bd3ea1689bf73edb840672e0d5b5b3b1aa174cc Mon Sep 17 00:00:00 2001
From: Akash Banerjee <Akash.Banerjee at amd.com>
Date: Thu, 8 Aug 2024 18:10:58 +0100
Subject: [PATCH 2/3] Address reviewer comments. Update test cases with
omp.composite checks.
---
flang/lib/Lower/OpenMP/OpenMP.cpp | 12 +-
mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp | 36 +++-
mlir/test/Dialect/OpenMP/invalid.mlir | 176 ++++++++++++++++++-
mlir/test/Dialect/OpenMP/ops.mlir | 48 ++---
4 files changed, 224 insertions(+), 48 deletions(-)
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 3b18e7b3ecf80e..3a9ca5be82a5f9 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -2063,14 +2063,12 @@ static void genCompositeDistributeSimd(
// TODO: Populate entry block arguments with private variables.
auto distributeOp = genWrapperOp<mlir::omp::DistributeOp>(
converter, loc, distributeClauseOps, /*blockArgTypes=*/{});
- llvm::cast<mlir::omp::ComposableOpInterface>(distributeOp.getOperation())
- .setComposite(/*val=*/true);
+ distributeOp.setComposite(/*val=*/true);
// TODO: Populate entry block arguments with reduction and private variables.
auto simdOp = genWrapperOp<mlir::omp::SimdOp>(converter, loc, simdClauseOps,
/*blockArgTypes=*/{});
- llvm::cast<mlir::omp::ComposableOpInterface>(simdOp.getOperation())
- .setComposite(/*val=*/true);
+ simdOp.setComposite(/*val=*/true);
// Construct wrapper entry block list and associated symbols. It is important
// that the symbol order and the block argument order match, so that the
@@ -2115,14 +2113,12 @@ static void genCompositeDoSimd(lower::AbstractConverter &converter,
// TODO: Add private variables to entry block arguments.
auto wsloopOp = genWrapperOp<mlir::omp::WsloopOp>(
converter, loc, wsloopClauseOps, wsloopReductionTypes);
- llvm::cast<mlir::omp::ComposableOpInterface>(wsloopOp.getOperation())
- .setComposite(/*val=*/true);
+ wsloopOp.setComposite(/*val=*/true);
// TODO: Populate entry block arguments with reduction and private variables.
auto simdOp = genWrapperOp<mlir::omp::SimdOp>(converter, loc, simdClauseOps,
/*blockArgTypes=*/{});
- llvm::cast<mlir::omp::ComposableOpInterface>(simdOp.getOperation())
- .setComposite(/*val=*/true);
+ simdOp.setComposite(/*val=*/true);
// Construct wrapper entry block list and associated symbols. It is important
// that the symbol and block argument order match, so that the symbol-value
diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
index 641fbb5a1418f6..6ddfe812d04fea 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -1546,7 +1546,7 @@ LogicalResult ParallelOp::verify() {
if (!isWrapper())
return emitOpError() << "must take a loop wrapper role if nested inside "
"of 'omp.distribute'";
- if (!llvm::cast<ComposableOpInterface>(getOperation()).isComposite())
+ if (!isComposite())
return emitError()
<< "'omp.composite' attribute missing from composite wrapper";
@@ -1558,7 +1558,7 @@ LogicalResult ParallelOp::verify() {
} else {
return emitOpError() << "must not wrap an 'omp.loop_nest' directly";
}
- } else if (llvm::cast<ComposableOpInterface>(getOperation()).isComposite()) {
+ } else if (isComposite()) {
return emitError()
<< "'omp.composite' attribute present in non-composite wrapper";
}
@@ -1754,8 +1754,13 @@ LogicalResult WsloopOp::verify() {
if (!isWrapper())
return emitOpError() << "must be a loop wrapper";
+ auto wrapper =
+ llvm::dyn_cast_if_present<LoopWrapperInterface>((*this)->getParentOp());
+ bool isCompositeWrapper = wrapper && wrapper.isWrapper() &&
+ (!llvm::isa<ParallelOp>(wrapper) ||
+ llvm::isa<DistributeOp>(wrapper->getParentOp()));
if (LoopWrapperInterface nested = getNestedWrapper()) {
- if (!llvm::cast<ComposableOpInterface>(getOperation()).isComposite())
+ if (!isComposite())
return emitError()
<< "'omp.composite' attribute missing from composite wrapper";
@@ -1764,9 +1769,12 @@ LogicalResult WsloopOp::verify() {
if (!isa<SimdOp>(nested))
return emitError() << "only supported nested wrapper is 'omp.simd'";
- } else if (llvm::cast<ComposableOpInterface>(getOperation()).isComposite()) {
+ } else if (isComposite() && !isCompositeWrapper) {
return emitError()
<< "'omp.composite' attribute present in non-composite wrapper";
+ } else if (!isComposite() && isCompositeWrapper) {
+ return emitError()
+ << "'omp.composite' attribute missing from composite wrapper";
}
return verifyReductionVarList(*this, getReductionSyms(), getReductionVars(),
@@ -1810,7 +1818,17 @@ LogicalResult SimdOp::verify() {
if (getNestedWrapper())
return emitOpError() << "must wrap an 'omp.loop_nest' directly";
- if (llvm::cast<ComposableOpInterface>(getOperation()).isComposite())
+ auto wrapper =
+ llvm::dyn_cast_if_present<LoopWrapperInterface>((*this)->getParentOp());
+ bool isCompositeWrapper = wrapper && wrapper.isWrapper() &&
+ (!llvm::isa<ParallelOp>(wrapper) ||
+ llvm::isa<DistributeOp>(wrapper->getParentOp()));
+
+ if (!isComposite() && isCompositeWrapper)
+ return emitError()
+ << "'omp.composite' attribute missing from composite wrapper";
+
+ if (isComposite() && !isCompositeWrapper)
return emitError()
<< "'omp.composite' attribute present in non-composite wrapper";
@@ -1843,7 +1861,7 @@ LogicalResult DistributeOp::verify() {
return emitOpError() << "must be a loop wrapper";
if (LoopWrapperInterface nested = getNestedWrapper()) {
- if (!llvm::cast<ComposableOpInterface>(getOperation()).isComposite())
+ if (!isComposite())
return emitError()
<< "'omp.composite' attribute missing from composite wrapper";
// Check for the allowed leaf constructs that may appear in a composite
@@ -1851,7 +1869,7 @@ LogicalResult DistributeOp::verify() {
if (!isa<ParallelOp, SimdOp>(nested))
return emitError() << "only supported nested wrappers are 'omp.parallel' "
"and 'omp.simd'";
- } else if (llvm::cast<ComposableOpInterface>(getOperation()).isComposite()) {
+ } else if (isComposite()) {
return emitError()
<< "'omp.composite' attribute present in non-composite wrapper";
}
@@ -2055,7 +2073,7 @@ LogicalResult TaskloopOp::verify() {
return emitOpError() << "must be a loop wrapper";
if (LoopWrapperInterface nested = getNestedWrapper()) {
- if (!llvm::cast<ComposableOpInterface>(getOperation()).isComposite())
+ if (!isComposite())
return emitError()
<< "'omp.composite' attribute missing from composite wrapper";
@@ -2063,7 +2081,7 @@ LogicalResult TaskloopOp::verify() {
// construct directly after TASKLOOP.
if (!isa<SimdOp>(nested))
return emitError() << "only supported nested wrapper is 'omp.simd'";
- } else if (llvm::cast<ComposableOpInterface>(getOperation()).isComposite()) {
+ } else if (isComposite()) {
return emitError()
<< "'omp.composite' attribute present in non-composite wrapper";
}
diff --git a/mlir/test/Dialect/OpenMP/invalid.mlir b/mlir/test/Dialect/OpenMP/invalid.mlir
index 1d1d93f0977588..2844051d3a9a0b 100644
--- a/mlir/test/Dialect/OpenMP/invalid.mlir
+++ b/mlir/test/Dialect/OpenMP/invalid.mlir
@@ -36,9 +36,9 @@ func.func @invalid_nested_wrapper(%lb : index, %ub : index, %step : index) {
omp.terminator
}
omp.terminator
- }
+ } {omp.composite}
omp.terminator
- }
+ } {omp.composite}
return
}
@@ -53,9 +53,9 @@ func.func @no_nested_wrapper(%lb : index, %ub : index, %step : index) {
omp.yield
}
omp.terminator
- }
+ } {omp.composite}
omp.terminator
- }
+ } {omp.composite}
return
}
@@ -210,7 +210,7 @@ func.func @invalid_nested_wrapper(%lb : index, %ub : index, %step : index) {
omp.terminator
}
omp.terminator
- }
+ } {omp.composite}
}
// -----
@@ -1965,7 +1965,7 @@ func.func @taskloop(%lb: i32, %ub: i32, %step: i32) {
omp.terminator
}
omp.terminator
- }
+ } {omp.composite}
return
}
@@ -2173,7 +2173,7 @@ func.func @omp_distribute_nested_wrapper(%lb: index, %ub: index, %step: index) -
"omp.terminator"() : () -> ()
}) : () -> ()
"omp.terminator"() : () -> ()
- }
+ } {omp.composite}
}
// -----
@@ -2383,3 +2383,165 @@ func.func @masked_arg_count_mismatch(%arg0: i32, %arg1: i32) {
}) : (i32, i32) -> ()
return
}
+
+// -----
+func.func @omp_parallel_missing_composite(%lb: index, %ub: index, %step: index) -> () {
+ omp.distribute {
+ // expected-error at +1 {{'omp.composite' attribute missing from composite wrapper}}
+ omp.parallel {
+ omp.wsloop {
+ omp.loop_nest (%iv) : index = (%lb) to (%ub) step (%step) {
+ omp.yield
+ }
+ omp.terminator
+ } {omp.composite}
+ omp.terminator
+ }
+ omp.terminator
+ } {omp.composite}
+ return
+}
+
+// -----
+func.func @omp_parallel_invalid_composite(%lb: index, %ub: index, %step: index) -> () {
+ // expected-error @below {{'omp.composite' attribute present in non-composite wrapper}}
+ omp.parallel {
+ omp.wsloop {
+ omp.loop_nest (%iv) : index = (%lb) to (%ub) step (%step) {
+ omp.yield
+ }
+ omp.terminator
+ }
+ omp.terminator
+ } {omp.composite}
+ return
+}
+
+// -----
+func.func @omp_wsloop_missing_composite(%lb: index, %ub: index, %step: index) -> () {
+ // expected-error @below {{'omp.composite' attribute missing from composite wrapper}}
+ omp.wsloop {
+ omp.simd {
+ omp.loop_nest (%iv) : index = (%lb) to (%ub) step (%step) {
+ omp.yield
+ }
+ omp.terminator
+ } {omp.composite}
+ omp.terminator
+ }
+ return
+}
+
+// -----
+func.func @omp_wsloop_invalid_composite(%lb: index, %ub: index, %step: index) -> () {
+ // expected-error @below {{'omp.composite' attribute present in non-composite wrapper}}
+ omp.wsloop {
+ omp.loop_nest (%iv) : index = (%lb) to (%ub) step (%step) {
+ omp.yield
+ }
+ omp.terminator
+ } {omp.composite}
+ return
+}
+
+// -----
+func.func @omp_wsloop_missing_composite_2(%lb: index, %ub: index, %step: index) -> () {
+ omp.distribute {
+ omp.parallel {
+ // expected-error @below {{'omp.composite' attribute missing from composite wrapper}}
+ omp.wsloop {
+ omp.loop_nest (%iv) : index = (%lb) to (%ub) step (%step) {
+ omp.yield
+ }
+ omp.terminator
+ }
+ omp.terminator
+ } {omp.composite}
+ omp.terminator
+ } {omp.composite}
+ return
+}
+
+// -----
+func.func @omp_simd_missing_composite(%lb: index, %ub: index, %step: index) -> () {
+ omp.wsloop {
+ // expected-error @below {{'omp.composite' attribute missing from composite wrapper}}
+ omp.simd {
+ omp.loop_nest (%iv) : index = (%lb) to (%ub) step (%step) {
+ omp.yield
+ }
+ omp.terminator
+ }
+ omp.terminator
+ } {omp.composite}
+ return
+}
+
+// -----
+func.func @omp_simd_invalid_composite(%lb: index, %ub: index, %step: index) -> () {
+ // expected-error @below {{'omp.composite' attribute present in non-composite wrapper}}
+ omp.simd {
+ omp.loop_nest (%iv) : index = (%lb) to (%ub) step (%step) {
+ omp.yield
+ }
+ omp.terminator
+ } {omp.composite}
+ return
+}
+
+// -----
+func.func @omp_distribute_missing_composite(%lb: index, %ub: index, %step: index) -> () {
+ // expected-error @below {{'omp.composite' attribute missing from composite wrapper}}
+ omp.distribute {
+ omp.parallel {
+ omp.wsloop {
+ omp.loop_nest (%iv) : index = (%lb) to (%ub) step (%step) {
+ omp.yield
+ }
+ omp.terminator
+ } {omp.composite}
+ omp.terminator
+ } {omp.composite}
+ omp.terminator
+ }
+ return
+}
+
+// -----
+func.func @omp_distribute_invalid_composite(%lb: index, %ub: index, %step: index) -> () {
+ // expected-error @below {{'omp.composite' attribute present in non-composite wrapper}}
+ omp.distribute {
+ omp.loop_nest (%0) : index = (%lb) to (%ub) step (%step) {
+ omp.yield
+ }
+ omp.terminator
+ } {omp.composite}
+ return
+}
+
+// -----
+func.func @omp_taskloop_missing_composite(%lb: index, %ub: index, %step: index) -> () {
+ // expected-error @below {{'omp.composite' attribute missing from composite wrapper}}
+ omp.taskloop {
+ omp.simd {
+ omp.loop_nest (%i) : index = (%lb) to (%ub) step (%step) {
+ omp.yield
+ }
+ omp.terminator
+ } {omp.composite}
+ omp.terminator
+ }
+ return
+}
+
+// -----
+func.func @omp_taskloop_invalid_composite(%lb: index, %ub: index, %step: index) -> () {
+ // expected-error @below {{'omp.composite' attribute present in non-composite wrapper}}
+ omp.taskloop {
+ omp.loop_nest (%i) : index = (%lb) to (%ub) step (%step) {
+ omp.yield
+ }
+ omp.terminator
+ } {omp.composite}
+ return
+}
diff --git a/mlir/test/Dialect/OpenMP/ops.mlir b/mlir/test/Dialect/OpenMP/ops.mlir
index d2924998f41b87..988e0096850bd7 100644
--- a/mlir/test/Dialect/OpenMP/ops.mlir
+++ b/mlir/test/Dialect/OpenMP/ops.mlir
@@ -110,11 +110,11 @@ func.func @omp_parallel(%data_var : memref<i32>, %if_cond : i1, %num_threads : i
omp.yield
}
omp.terminator
- }
+ } {omp.composite}
omp.terminator
- }
+ } {omp.composite}
omp.terminator
- }
+ } {omp.composite}
return
}
@@ -434,9 +434,9 @@ func.func @omp_wsloop(%lb : index, %ub : index, %step : index, %data_var : memre
omp.yield
}
omp.terminator
- }
+ } {omp.composite}
omp.terminator
- }) : () -> ()
+ }) {omp.composite} : () -> ()
return
}
@@ -541,9 +541,9 @@ func.func @omp_wsloop_pretty(%lb : index, %ub : index, %step : index, %data_var
omp.yield
}
omp.terminator
- }
+ } {omp.composite}
omp.terminator
- }
+ } {omp.composite}
return
}
@@ -794,9 +794,9 @@ func.func @omp_distribute(%chunk_size : i32, %data_var : memref<i32>, %arg0 : i3
omp.yield
}
omp.terminator
- }
+ } {omp.composite}
omp.terminator
- }
+ } {omp.composite}
return
}
@@ -2101,7 +2101,7 @@ func.func @omp_task(%bool_var: i1, %i64_var: i64, %i32_var: i32, %data_var: memr
// CHECK: omp.task allocate(%[[data_var]] : memref<i32> -> %[[data_var]] : memref<i32>)
omp.task allocate(%data_var : memref<i32> -> %data_var : memref<i32>)
// CHECK-SAME: final(%[[bool_var]]) if(%[[bool_var]])
- final(%bool_var) if(%bool_var)
+ final(%bool_var) if(%bool_var)
// CHECK-SAME: in_reduction(@add_f32 -> %[[redn_var1]] : !llvm.ptr, byref @add_f32 -> %[[redn_var2]] : !llvm.ptr)
in_reduction(@add_f32 -> %0 : !llvm.ptr, byref @add_f32 -> %1 : !llvm.ptr)
// CHECK-SAME: priority(%[[i32_var]] : i32) untied
@@ -2456,9 +2456,9 @@ func.func @omp_taskloop(%lb: i32, %ub: i32, %step: i32) -> () {
omp.yield
}
omp.terminator
- }
+ } {omp.composite}
omp.terminator
- }
+ } {omp.composite}
// CHECK: return
return
@@ -2585,10 +2585,10 @@ func.func @omp_target_update_data (%if_cond : i1, %device : si32, %map1: memref<
// CHECK-LABEL: omp_targets_is_allocatable
// CHECK-SAME: (%[[ARG0:.*]]: !llvm.ptr, %[[ARG1:.*]]: !llvm.ptr)
func.func @omp_targets_is_allocatable(%arg0: !llvm.ptr, %arg1: !llvm.ptr) -> () {
- // CHECK: %[[MAP0:.*]] = omp.map.info var_ptr(%[[ARG0]] : !llvm.ptr, i32) map_clauses(tofrom) capture(ByRef) -> !llvm.ptr {name = ""}
+ // CHECK: %[[MAP0:.*]] = omp.map.info var_ptr(%[[ARG0]] : !llvm.ptr, i32) map_clauses(tofrom) capture(ByRef) -> !llvm.ptr {name = ""}
%mapv1 = omp.map.info var_ptr(%arg0 : !llvm.ptr, i32) map_clauses(tofrom) capture(ByRef) -> !llvm.ptr {name = ""}
// CHECK: %[[MAP1:.*]] = omp.map.info var_ptr(%[[ARG1]] : !llvm.ptr, !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8)>) map_clauses(tofrom) capture(ByRef) members(%[[MAP0]] : [0] : !llvm.ptr) -> !llvm.ptr {name = ""}
- %mapv2 = omp.map.info var_ptr(%arg1 : !llvm.ptr, !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8)>) map_clauses(tofrom) capture(ByRef) members(%mapv1 : [0] : !llvm.ptr) -> !llvm.ptr {name = ""}
+ %mapv2 = omp.map.info var_ptr(%arg1 : !llvm.ptr, !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8)>) map_clauses(tofrom) capture(ByRef) members(%mapv1 : [0] : !llvm.ptr) -> !llvm.ptr {name = ""}
// CHECK: omp.target map_entries(%[[MAP0]] -> {{.*}}, %[[MAP1]] -> {{.*}} : !llvm.ptr, !llvm.ptr)
omp.target map_entries(%mapv1 -> %arg2, %mapv2 -> %arg3 : !llvm.ptr, !llvm.ptr) {
^bb0(%arg2: !llvm.ptr, %arg3 : !llvm.ptr):
@@ -2650,27 +2650,27 @@ func.func @omp_target_enter_update_exit_data_depend(%a: memref<?xi32>, %b: memre
// CHECK-LABEL: omp_map_with_members
// CHECK-SAME: (%[[ARG0:.*]]: !llvm.ptr, %[[ARG1:.*]]: !llvm.ptr, %[[ARG2:.*]]: !llvm.ptr, %[[ARG3:.*]]: !llvm.ptr, %[[ARG4:.*]]: !llvm.ptr, %[[ARG5:.*]]: !llvm.ptr)
func.func @omp_map_with_members(%arg0: !llvm.ptr, %arg1: !llvm.ptr, %arg2: !llvm.ptr, %arg3: !llvm.ptr, %arg4: !llvm.ptr, %arg5: !llvm.ptr) -> () {
- // CHECK: %[[MAP0:.*]] = omp.map.info var_ptr(%[[ARG0]] : !llvm.ptr, i32) map_clauses(to) capture(ByRef) -> !llvm.ptr {name = ""}
+ // CHECK: %[[MAP0:.*]] = omp.map.info var_ptr(%[[ARG0]] : !llvm.ptr, i32) map_clauses(to) capture(ByRef) -> !llvm.ptr {name = ""}
%mapv1 = omp.map.info var_ptr(%arg0 : !llvm.ptr, i32) map_clauses(to) capture(ByRef) -> !llvm.ptr {name = ""}
- // CHECK: %[[MAP1:.*]] = omp.map.info var_ptr(%[[ARG1]] : !llvm.ptr, f32) map_clauses(to) capture(ByRef) -> !llvm.ptr {name = ""}
+ // CHECK: %[[MAP1:.*]] = omp.map.info var_ptr(%[[ARG1]] : !llvm.ptr, f32) map_clauses(to) capture(ByRef) -> !llvm.ptr {name = ""}
%mapv2 = omp.map.info var_ptr(%arg1 : !llvm.ptr, f32) map_clauses(to) capture(ByRef) -> !llvm.ptr {name = ""}
-
+
// CHECK: %[[MAP2:.*]] = omp.map.info var_ptr(%[[ARG2]] : !llvm.ptr, !llvm.struct<(i32, f32)>) map_clauses(to) capture(ByRef) members(%[[MAP0]], %[[MAP1]] : [0], [1] : !llvm.ptr, !llvm.ptr) -> !llvm.ptr {name = "", partial_map = true}
- %mapv3 = omp.map.info var_ptr(%arg2 : !llvm.ptr, !llvm.struct<(i32, f32)>) map_clauses(to) capture(ByRef) members(%mapv1, %mapv2 : [0], [1] : !llvm.ptr, !llvm.ptr) -> !llvm.ptr {name = "", partial_map = true}
-
+ %mapv3 = omp.map.info var_ptr(%arg2 : !llvm.ptr, !llvm.struct<(i32, f32)>) map_clauses(to) capture(ByRef) members(%mapv1, %mapv2 : [0], [1] : !llvm.ptr, !llvm.ptr) -> !llvm.ptr {name = "", partial_map = true}
+
// CHECK: omp.target_enter_data map_entries(%[[MAP0]], %[[MAP1]], %[[MAP2]] : !llvm.ptr, !llvm.ptr, !llvm.ptr)
omp.target_enter_data map_entries(%mapv1, %mapv2, %mapv3 : !llvm.ptr, !llvm.ptr, !llvm.ptr){}
- // CHECK: %[[MAP3:.*]] = omp.map.info var_ptr(%[[ARG3]] : !llvm.ptr, i32) map_clauses(from) capture(ByRef) -> !llvm.ptr {name = ""}
+ // CHECK: %[[MAP3:.*]] = omp.map.info var_ptr(%[[ARG3]] : !llvm.ptr, i32) map_clauses(from) capture(ByRef) -> !llvm.ptr {name = ""}
%mapv4 = omp.map.info var_ptr(%arg3 : !llvm.ptr, i32) map_clauses(from) capture(ByRef) -> !llvm.ptr {name = ""}
- // CHECK: %[[MAP4:.*]] = omp.map.info var_ptr(%[[ARG4]] : !llvm.ptr, f32) map_clauses(from) capture(ByRef) -> !llvm.ptr {name = ""}
+ // CHECK: %[[MAP4:.*]] = omp.map.info var_ptr(%[[ARG4]] : !llvm.ptr, f32) map_clauses(from) capture(ByRef) -> !llvm.ptr {name = ""}
%mapv5 = omp.map.info var_ptr(%arg4 : !llvm.ptr, f32) map_clauses(from) capture(ByRef) -> !llvm.ptr {name = ""}
-
+
// CHECK: %[[MAP5:.*]] = omp.map.info var_ptr(%[[ARG5]] : !llvm.ptr, !llvm.struct<(i32, struct<(i32, f32)>)>) map_clauses(from) capture(ByRef) members(%[[MAP3]], %[[MAP4]] : [1,0], [1,1] : !llvm.ptr, !llvm.ptr) -> !llvm.ptr {name = "", partial_map = true}
- %mapv6 = omp.map.info var_ptr(%arg5 : !llvm.ptr, !llvm.struct<(i32, struct<(i32, f32)>)>) map_clauses(from) capture(ByRef) members(%mapv4, %mapv5 : [1,0], [1,1] : !llvm.ptr, !llvm.ptr) -> !llvm.ptr {name = "", partial_map = true}
-
+ %mapv6 = omp.map.info var_ptr(%arg5 : !llvm.ptr, !llvm.struct<(i32, struct<(i32, f32)>)>) map_clauses(from) capture(ByRef) members(%mapv4, %mapv5 : [1,0], [1,1] : !llvm.ptr, !llvm.ptr) -> !llvm.ptr {name = "", partial_map = true}
+
// CHECK: omp.target_exit_data map_entries(%[[MAP3]], %[[MAP4]], %[[MAP5]] : !llvm.ptr, !llvm.ptr, !llvm.ptr)
omp.target_exit_data map_entries(%mapv4, %mapv5, %mapv6 : !llvm.ptr, !llvm.ptr, !llvm.ptr){}
>From 8a51455b12693004041e6902ec49e53e2c7d38ae Mon Sep 17 00:00:00 2001
From: Akash Banerjee <Akash.Banerjee at amd.com>
Date: Mon, 12 Aug 2024 15:33:40 +0100
Subject: [PATCH 3/3] Addressed reviewer comments.
---
mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp | 22 +++++++++++---------
mlir/test/Dialect/OpenMP/invalid.mlir | 10 ++++-----
2 files changed, 17 insertions(+), 15 deletions(-)
diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
index 6ddfe812d04fea..4c943ebbe3144f 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -1756,9 +1756,10 @@ LogicalResult WsloopOp::verify() {
auto wrapper =
llvm::dyn_cast_if_present<LoopWrapperInterface>((*this)->getParentOp());
- bool isCompositeWrapper = wrapper && wrapper.isWrapper() &&
- (!llvm::isa<ParallelOp>(wrapper) ||
- llvm::isa<DistributeOp>(wrapper->getParentOp()));
+ bool isCompositeChildLeaf =
+ wrapper && wrapper.isWrapper() &&
+ (!llvm::isa<ParallelOp>(wrapper) ||
+ llvm::isa_and_present<DistributeOp>(wrapper->getParentOp()));
if (LoopWrapperInterface nested = getNestedWrapper()) {
if (!isComposite())
return emitError()
@@ -1769,10 +1770,10 @@ LogicalResult WsloopOp::verify() {
if (!isa<SimdOp>(nested))
return emitError() << "only supported nested wrapper is 'omp.simd'";
- } else if (isComposite() && !isCompositeWrapper) {
+ } else if (isComposite() && !isCompositeChildLeaf) {
return emitError()
<< "'omp.composite' attribute present in non-composite wrapper";
- } else if (!isComposite() && isCompositeWrapper) {
+ } else if (!isComposite() && isCompositeChildLeaf) {
return emitError()
<< "'omp.composite' attribute missing from composite wrapper";
}
@@ -1820,15 +1821,16 @@ LogicalResult SimdOp::verify() {
auto wrapper =
llvm::dyn_cast_if_present<LoopWrapperInterface>((*this)->getParentOp());
- bool isCompositeWrapper = wrapper && wrapper.isWrapper() &&
- (!llvm::isa<ParallelOp>(wrapper) ||
- llvm::isa<DistributeOp>(wrapper->getParentOp()));
+ bool isCompositeChildLeaf =
+ wrapper && wrapper.isWrapper() &&
+ (!llvm::isa<ParallelOp>(wrapper) ||
+ llvm::isa_and_present<DistributeOp>(wrapper->getParentOp()));
- if (!isComposite() && isCompositeWrapper)
+ if (!isComposite() && isCompositeChildLeaf)
return emitError()
<< "'omp.composite' attribute missing from composite wrapper";
- if (isComposite() && !isCompositeWrapper)
+ if (isComposite() && !isCompositeChildLeaf)
return emitError()
<< "'omp.composite' attribute present in non-composite wrapper";
diff --git a/mlir/test/Dialect/OpenMP/invalid.mlir b/mlir/test/Dialect/OpenMP/invalid.mlir
index 2844051d3a9a0b..c76b07ec94a597 100644
--- a/mlir/test/Dialect/OpenMP/invalid.mlir
+++ b/mlir/test/Dialect/OpenMP/invalid.mlir
@@ -34,7 +34,7 @@ func.func @invalid_nested_wrapper(%lb : index, %ub : index, %step : index) {
omp.yield
}
omp.terminator
- }
+ } {omp.composite}
omp.terminator
} {omp.composite}
omp.terminator
@@ -208,7 +208,7 @@ func.func @invalid_nested_wrapper(%lb : index, %ub : index, %step : index) {
omp.yield
}
omp.terminator
- }
+ } {omp.composite}
omp.terminator
} {omp.composite}
}
@@ -1963,7 +1963,7 @@ func.func @taskloop(%lb: i32, %ub: i32, %step: i32) {
omp.yield
}
omp.terminator
- }
+ } {omp.composite}
omp.terminator
} {omp.composite}
return
@@ -2171,7 +2171,7 @@ func.func @omp_distribute_nested_wrapper(%lb: index, %ub: index, %step: index) -
"omp.yield"() : () -> ()
}
"omp.terminator"() : () -> ()
- }) : () -> ()
+ }) {omp.composite} : () -> ()
"omp.terminator"() : () -> ()
} {omp.composite}
}
@@ -2387,7 +2387,7 @@ func.func @masked_arg_count_mismatch(%arg0: i32, %arg1: i32) {
// -----
func.func @omp_parallel_missing_composite(%lb: index, %ub: index, %step: index) -> () {
omp.distribute {
- // expected-error at +1 {{'omp.composite' attribute missing from composite wrapper}}
+ // expected-error at +1 {{'omp.composite' attribute missing from composite wrapper}}
omp.parallel {
omp.wsloop {
omp.loop_nest (%iv) : index = (%lb) to (%ub) step (%step) {
More information about the llvm-branch-commits
mailing list