[libc-commits] [libc] [mlir][OpenMP] Added `omp.structured_region` operation (PR #68825)

via libc-commits libc-commits at lists.llvm.org
Wed Oct 11 11:27:34 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 libc-commits mailing list