[clang] [mlir][OpenMP] Added `omp.structured_region` operation (PR #68825)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Oct 11 11:27:30 PDT 2023
https://github.com/shraiysh updated https://github.com/llvm/llvm-project/pull/68825
>From ff635ce0ce910f0cde248a4babb3c27333ddc108 Mon Sep 17 00:00:00 2001
From: Shraiysh Vaishay <shraiysh.vaishay at amd.com>
Date: Sun, 3 Sep 2023 22:40:10 -0500
Subject: [PATCH 1/3] [mlir][OpenMP] Added omp.region operation
This patch adds omp.region operation. This is equivalent to a code block surrounded by an OpenMP construct in C/C++/Fortran. This is a part of the effort to implement the canonical loop operation in OpenMP dialect.
---
mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td | 34 ++++++++++++
mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp | 15 ++++++
mlir/test/Dialect/OpenMP/region.mlir | 53 +++++++++++++++++++
3 files changed, 102 insertions(+)
create mode 100644 mlir/test/Dialect/OpenMP/region.mlir
diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
index b1e1fe00b8594a7..cf5b246178f5d7a 100644
--- a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
+++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
@@ -1787,4 +1787,38 @@ def ClauseRequiresAttr :
EnumAttr<OpenMP_Dialect, ClauseRequires, "clause_requires"> {
}
+
+def RegionOp : OpenMP_Op<"region"> {
+ let summary = "Encapsulates a region in an OpenMP construct.";
+ let description = [{
+ Encapsulates a region surrounded by an OpenMP Construct. The intended use
+ of this operation is that within an OpenMP operation's region, there would
+ be a single `omp.region` operation and a terminator operation as shown
+ below.
+
+ ```
+ // Example with `omp.task`
+ omp.task {
+ omp.region {
+ call @foo : () -> ()
+ }
+ omp.terminator
+ }
+ ```
+
+ This operation is especially more useful in operations that use `omp.yield`
+ as a terminator. For example, in the proposed canonical loop operation,
+ this operation would help fix the arguments of the yield operation and it
+ is not affected by branches in the region assosciated with the canonical
+ loop. In cases where the yielded value has to be a compile time constant,
+ this provides a mechanism to avoid complex static analysis for the constant
+ value.
+ }];
+ let regions = (region AnyRegion:$block);
+ let assemblyFormat = [{
+ $block attr-dict
+ }];
+ let hasVerifier = 1;
+}
+
#endif // OPENMP_OPS
diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
index 2ba5f1aca9cf6b2..2a2fcdb788cb99b 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -1524,6 +1524,21 @@ LogicalResult CancellationPointOp::verify() {
return success();
}
+//===----------------------------------------------------------------------===//
+// RegionOp
+//===----------------------------------------------------------------------===//
+
+LogicalResult RegionOp::verify() {
+ Operation *parentOp = (*this)->getParentOp();
+ if (!parentOp)
+ return emitOpError() << "`omp.region` must have a parent";
+
+ if (!isa<OpenMPDialect>(parentOp->getDialect()))
+ return emitOpError()
+ << "`omp.region` must be used under an OpenMP Dialect operation.";
+ return success();
+}
+
#define GET_ATTRDEF_CLASSES
#include "mlir/Dialect/OpenMP/OpenMPOpsAttributes.cpp.inc"
diff --git a/mlir/test/Dialect/OpenMP/region.mlir b/mlir/test/Dialect/OpenMP/region.mlir
new file mode 100644
index 000000000000000..4e0ddbc07e9ec9d
--- /dev/null
+++ b/mlir/test/Dialect/OpenMP/region.mlir
@@ -0,0 +1,53 @@
+// RUN: mlir-opt %s | mlir-opt | FileCheck %s
+
+// CHECK-LABEL: @basic_omp_region
+// CHECK-NEXT: omp.parallel {
+// CHECK-NEXT: omp.region {
+// CHECK-NEXT: "test.foo"() : () -> ()
+// CHECK-NEXT: omp.terminator
+// CHECK-NEXT: }
+// CHECK-NEXT: omp.terminator
+// CHECK-NEXT: }
+// CHECK-NEXT: return
+func.func @basic_omp_region() {
+ omp.parallel {
+ omp.region {
+ "test.foo"() : () -> ()
+ omp.terminator
+ }
+ omp.terminator
+ }
+ return
+}
+
+// CHECK-LABEL: @omp_region_with_branch
+// CHECK-NEXT: omp.task {
+// CHECK-NEXT: omp.region {
+// CHECK-NEXT: %[[c:.*]] = "test.foo"() : () -> i1
+// CHECK-NEXT: cf.cond_br %[[c]], ^[[bb1:.*]](%[[c]] : i1), ^[[bb2:.*]](%[[c]] : i1)
+// CHECK-NEXT: ^[[bb1]](%[[arg:.*]]: i1):
+// CHECK-NEXT: "test.bar"() : () -> ()
+// CHECK-NEXT: omp.terminator
+// CHECK-NEXT: ^[[bb2]](%[[arg2:.*]]: i1):
+// CHECK-NEXT: "test.baz"() : () -> ()
+// CHECK-NEXT: omp.terminator
+// CHECK-NEXT: }
+// CHECK-NEXT: omp.terminator
+// CHECK-NEXT: }
+// CHECK-NEXT: return
+func.func @omp_region_with_branch(%a: i32) {
+ omp.task {
+ omp.region {
+ %c = "test.foo"() : () -> i1
+ cf.cond_br %c, ^bb1(%c: i1), ^bb2(%c: i1)
+ ^bb1(%arg: i1):
+ "test.bar"() : () -> ()
+ omp.terminator
+ ^bb2(%arg2: i1):
+ "test.baz"() : () -> ()
+ omp.terminator
+ }
+ omp.terminator
+ }
+ return
+}
>From f88bfd4cad856e915cb96b1238aac978f3aeb6d4 Mon Sep 17 00:00:00 2001
From: Shraiysh Vaishay <shraiysh.vaishay at amd.com>
Date: Mon, 4 Sep 2023 10:33:48 -0500
Subject: [PATCH 2/3] Change the name to structured region and add invalid
testcase.
---
mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td | 16 +++++++-------
mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp | 8 +++----
mlir/test/Dialect/OpenMP/invalid.mlir | 22 +++++++++++++++++++
.../{region.mlir => structured_region.mlir} | 8 +++----
4 files changed, 37 insertions(+), 17 deletions(-)
rename mlir/test/Dialect/OpenMP/{region.mlir => structured_region.mlir} (90%)
diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
index cf5b246178f5d7a..fa2814a763dcf99 100644
--- a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
+++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
@@ -1788,18 +1788,18 @@ def ClauseRequiresAttr :
}
-def RegionOp : OpenMP_Op<"region"> {
+def RegionOp : OpenMP_Op<"structured_region"> {
let summary = "Encapsulates a region in an OpenMP construct.";
let description = [{
Encapsulates a region surrounded by an OpenMP Construct. The intended use
of this operation is that within an OpenMP operation's region, there would
- be a single `omp.region` operation and a terminator operation as shown
- below.
+ be a single `omp.structured_region` operation and a terminator operation as
+ shown below.
```
- // Example with `omp.task`
- omp.task {
- omp.region {
+ // Example with `omp.sections`
+ omp.sections {
+ omp.structured_region {
call @foo : () -> ()
}
omp.terminator
@@ -1814,9 +1814,9 @@ def RegionOp : OpenMP_Op<"region"> {
this provides a mechanism to avoid complex static analysis for the constant
value.
}];
- let regions = (region AnyRegion:$block);
+ let regions = (region AnyRegion:$region);
let assemblyFormat = [{
- $block attr-dict
+ $region attr-dict
}];
let hasVerifier = 1;
}
diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
index 2a2fcdb788cb99b..47cad6e762ed16d 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -25,6 +25,7 @@
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/TypeSwitch.h"
#include "llvm/Frontend/OpenMP/OMPConstants.h"
+#include "llvm/Support/Debug.h"
#include <cstddef>
#include <optional>
@@ -1530,12 +1531,9 @@ LogicalResult CancellationPointOp::verify() {
LogicalResult RegionOp::verify() {
Operation *parentOp = (*this)->getParentOp();
- if (!parentOp)
- return emitOpError() << "`omp.region` must have a parent";
-
+ assert(parentOp && "'omp.region' op must have a parent");
if (!isa<OpenMPDialect>(parentOp->getDialect()))
- return emitOpError()
- << "`omp.region` must be used under an OpenMP Dialect operation.";
+ return emitOpError() << "must be used under an OpenMP Dialect operation";
return success();
}
diff --git a/mlir/test/Dialect/OpenMP/invalid.mlir b/mlir/test/Dialect/OpenMP/invalid.mlir
index 5a244ec1bebf23d..be41f4cf5890aa4 100644
--- a/mlir/test/Dialect/OpenMP/invalid.mlir
+++ b/mlir/test/Dialect/OpenMP/invalid.mlir
@@ -1653,3 +1653,25 @@ func.func @omp_target_exit_data(%map1: memref<?xi32>) {
}
llvm.mlir.global internal @_QFsubEx() : i32
+
+// -----
+
+func.func @omp_region_invalid() {
+ // expected-error @below {{'omp.structured_region' op must be used under an OpenMP Dialect operation}}
+ omp.structured_region {
+ omp.terminator
+ }
+ return
+}
+
+// -----
+
+func.func @omp_region_invalid(%c: i1) {
+ scf.if %c {
+ // expected-error @below {{'omp.structured_region' op must be used under an OpenMP Dialect operation}}
+ omp.structured_region {
+ omp.terminator
+ }
+ }
+ return
+}
diff --git a/mlir/test/Dialect/OpenMP/region.mlir b/mlir/test/Dialect/OpenMP/structured_region.mlir
similarity index 90%
rename from mlir/test/Dialect/OpenMP/region.mlir
rename to mlir/test/Dialect/OpenMP/structured_region.mlir
index 4e0ddbc07e9ec9d..fc1e0edb07388eb 100644
--- a/mlir/test/Dialect/OpenMP/region.mlir
+++ b/mlir/test/Dialect/OpenMP/structured_region.mlir
@@ -2,7 +2,7 @@
// CHECK-LABEL: @basic_omp_region
// CHECK-NEXT: omp.parallel {
-// CHECK-NEXT: omp.region {
+// CHECK-NEXT: omp.structured_region {
// CHECK-NEXT: "test.foo"() : () -> ()
// CHECK-NEXT: omp.terminator
// CHECK-NEXT: }
@@ -11,7 +11,7 @@
// CHECK-NEXT: return
func.func @basic_omp_region() {
omp.parallel {
- omp.region {
+ omp.structured_region {
"test.foo"() : () -> ()
omp.terminator
}
@@ -22,7 +22,7 @@ func.func @basic_omp_region() {
// CHECK-LABEL: @omp_region_with_branch
// CHECK-NEXT: omp.task {
-// CHECK-NEXT: omp.region {
+// CHECK-NEXT: omp.structured_region {
// CHECK-NEXT: %[[c:.*]] = "test.foo"() : () -> i1
// CHECK-NEXT: cf.cond_br %[[c]], ^[[bb1:.*]](%[[c]] : i1), ^[[bb2:.*]](%[[c]] : i1)
// CHECK-NEXT: ^[[bb1]](%[[arg:.*]]: i1):
@@ -37,7 +37,7 @@ func.func @basic_omp_region() {
// CHECK-NEXT: return
func.func @omp_region_with_branch(%a: i32) {
omp.task {
- omp.region {
+ omp.structured_region {
%c = "test.foo"() : () -> i1
cf.cond_br %c, ^bb1(%c: i1), ^bb2(%c: i1)
^bb1(%arg: i1):
>From 4591b52fe4e6714d2f8bf1d672951d5b08b09e01 Mon Sep 17 00:00:00 2001
From: Shraiysh Vaishay <shraiysh.vaishay at amd.com>
Date: Wed, 11 Oct 2023 13:27:01 -0500
Subject: [PATCH 3/3] Added yield operation as terminator
---
mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td | 10 +++---
mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp | 1 -
mlir/test/Dialect/OpenMP/invalid.mlir | 4 +--
.../Dialect/OpenMP/structured_region.mlir | 32 +++++++++----------
4 files changed, 24 insertions(+), 23 deletions(-)
diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
index b9e041a22b5bffb..8fbc680f26bab68 100644
--- a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
+++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
@@ -619,7 +619,7 @@ def SimdLoopOp : OpenMP_Op<"simdloop", [AttrSizedOperandSegments,
def YieldOp : OpenMP_Op<"yield",
[Pure, ReturnLike, Terminator,
ParentOneOf<["WsLoopOp", "ReductionDeclareOp",
- "AtomicUpdateOp", "SimdLoopOp"]>]> {
+ "AtomicUpdateOp", "SimdLoopOp", "StructuredRegionOp"]>]> {
let summary = "loop yield and termination operation";
let description = [{
"omp.yield" yields SSA values from the OpenMP dialect op region and
@@ -1999,8 +1999,9 @@ def StructuredRegionOp : OpenMP_Op<"structured_region"> {
```
// Example with `omp.sections`
omp.sections {
- omp.structured_region {
- call @foo : () -> ()
+ %x = omp.structured_region {
+ %t = call @foo : () -> (i32)
+ omp.yield(%t : i32)
}
omp.terminator
}
@@ -2015,8 +2016,9 @@ def StructuredRegionOp : OpenMP_Op<"structured_region"> {
value.
}];
let regions = (region AnyRegion:$region);
+ let results = (outs Variadic<AnyType>:$result);
let assemblyFormat = [{
- $region attr-dict
+ ( `(` type($result)^ `)` )? $region attr-dict
}];
let hasVerifier = 1;
}
diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
index 70811a230b36ccd..e00c4c44c60dc84 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -26,7 +26,6 @@
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/TypeSwitch.h"
#include "llvm/Frontend/OpenMP/OMPConstants.h"
-#include "llvm/Support/Debug.h"
#include <cstddef>
#include <optional>
diff --git a/mlir/test/Dialect/OpenMP/invalid.mlir b/mlir/test/Dialect/OpenMP/invalid.mlir
index 94814a4d398b8c7..e4c99304010b65f 100644
--- a/mlir/test/Dialect/OpenMP/invalid.mlir
+++ b/mlir/test/Dialect/OpenMP/invalid.mlir
@@ -1663,7 +1663,7 @@ llvm.mlir.global internal @_QFsubEx() : i32
func.func @omp_region_invalid() {
// expected-error @below {{'omp.structured_region' op must be used under an OpenMP Dialect operation}}
omp.structured_region {
- omp.terminator
+ omp.yield
}
return
}
@@ -1674,7 +1674,7 @@ func.func @omp_region_invalid(%c: i1) {
scf.if %c {
// expected-error @below {{'omp.structured_region' op must be used under an OpenMP Dialect operation}}
omp.structured_region {
- omp.terminator
+ omp.yield
}
}
return
diff --git a/mlir/test/Dialect/OpenMP/structured_region.mlir b/mlir/test/Dialect/OpenMP/structured_region.mlir
index fc1e0edb07388eb..13fc4a0d7284a8b 100644
--- a/mlir/test/Dialect/OpenMP/structured_region.mlir
+++ b/mlir/test/Dialect/OpenMP/structured_region.mlir
@@ -2,18 +2,18 @@
// CHECK-LABEL: @basic_omp_region
// CHECK-NEXT: omp.parallel {
-// CHECK-NEXT: omp.structured_region {
-// CHECK-NEXT: "test.foo"() : () -> ()
-// CHECK-NEXT: omp.terminator
+// CHECK-NEXT: {{.+}} = omp.structured_region(i32) {
+// CHECK-NEXT: {{.+}} = "test.foo"() : () -> i32
+// CHECK-NEXT: omp.yield({{.+}})
// CHECK-NEXT: }
// CHECK-NEXT: omp.terminator
// CHECK-NEXT: }
// CHECK-NEXT: return
func.func @basic_omp_region() {
omp.parallel {
- omp.structured_region {
- "test.foo"() : () -> ()
- omp.terminator
+ %x = omp.structured_region (i32) {
+ %y = "test.foo"() : () -> i32
+ omp.yield(%y : i32)
}
omp.terminator
}
@@ -22,30 +22,30 @@ func.func @basic_omp_region() {
// CHECK-LABEL: @omp_region_with_branch
// CHECK-NEXT: omp.task {
-// CHECK-NEXT: omp.structured_region {
+// CHECK-NEXT: {{.+}} = omp.structured_region(i32) {
// CHECK-NEXT: %[[c:.*]] = "test.foo"() : () -> i1
// CHECK-NEXT: cf.cond_br %[[c]], ^[[bb1:.*]](%[[c]] : i1), ^[[bb2:.*]](%[[c]] : i1)
// CHECK-NEXT: ^[[bb1]](%[[arg:.*]]: i1):
-// CHECK-NEXT: "test.bar"() : () -> ()
-// CHECK-NEXT: omp.terminator
+// CHECK-NEXT: {{.+}} = "test.bar"() : () -> i32
+// CHECK-NEXT: omp.yield({{.+}})
// CHECK-NEXT: ^[[bb2]](%[[arg2:.*]]: i1):
-// CHECK-NEXT: "test.baz"() : () -> ()
-// CHECK-NEXT: omp.terminator
+// CHECK-NEXT: {{.+}} = "test.baz"() : () -> i32
+// CHECK-NEXT: omp.yield({{.+}})
// CHECK-NEXT: }
// CHECK-NEXT: omp.terminator
// CHECK-NEXT: }
// CHECK-NEXT: return
func.func @omp_region_with_branch(%a: i32) {
omp.task {
- omp.structured_region {
+ %t = omp.structured_region (i32) {
%c = "test.foo"() : () -> i1
cf.cond_br %c, ^bb1(%c: i1), ^bb2(%c: i1)
^bb1(%arg: i1):
- "test.bar"() : () -> ()
- omp.terminator
+ %x = "test.bar"() : () -> i32
+ omp.yield(%x : i32)
^bb2(%arg2: i1):
- "test.baz"() : () -> ()
- omp.terminator
+ %y = "test.baz"() : () -> i32
+ omp.yield(%y: i32)
}
omp.terminator
}
More information about the cfe-commits
mailing list