Skip to content

Conversation

ivanradanov
Copy link
Contributor

@ivanradanov ivanradanov commented Aug 1, 2024

2/4

1/4 #101443
2/4 #101444
3/4 #101445
4/4 #101446

@ivanradanov
Copy link
Contributor Author

ivanradanov commented Aug 1, 2024

Should we have a -use-experimental-workshare or similar flag to facilitate some temporary in-tree development as this may require more moving pieces?

@ivanradanov ivanradanov force-pushed the flang-workshare-emit branch 4 times, most recently from 38322dc to 9046df2 Compare August 6, 2024 04:53
@ivanradanov ivanradanov force-pushed the users/ivanradanov/flang-workshare-omp branch from cc551ca to 7376fd4 Compare August 19, 2024 04:01
@ivanradanov ivanradanov force-pushed the flang-workshare-emit branch 2 times, most recently from 10b7a39 to d17c552 Compare August 19, 2024 06:22
@ivanradanov ivanradanov marked this pull request as ready for review August 19, 2024 08:25
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir flang:openmp labels Aug 19, 2024
@llvmbot
Copy link
Member

llvmbot commented Aug 19, 2024

@llvm/pr-subscribers-flang-fir-hlfir

@llvm/pr-subscribers-flang-openmp

Author: Ivan R. Ivanov (ivanradanov)

Changes

2/4

1/4 #101443
2/4 #101444
3/4 #101445
4/4 #101446


Full diff: https://github.com/llvm/llvm-project/pull/101444.diff

2 Files Affected:

  • (modified) flang/lib/Lower/OpenMP/OpenMP.cpp (+26-4)
  • (modified) flang/test/Lower/OpenMP/workshare.f90 (+3-3)
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 2b1839b5270d4f..f7bc565ea8cbc1 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -1270,6 +1270,15 @@ static void genTaskwaitClauses(lower::AbstractConverter &converter,
       loc, llvm::omp::Directive::OMPD_taskwait);
 }
 
+static void genWorkshareClauses(lower::AbstractConverter &converter,
+                                semantics::SemanticsContext &semaCtx,
+                                lower::StatementContext &stmtCtx,
+                                const List<Clause> &clauses, mlir::Location loc,
+                                mlir::omp::WorkshareOperands &clauseOps) {
+  ClauseProcessor cp(converter, semaCtx, clauses);
+  cp.processNowait(clauseOps);
+}
+
 static void genTeamsClauses(lower::AbstractConverter &converter,
                             semantics::SemanticsContext &semaCtx,
                             lower::StatementContext &stmtCtx,
@@ -1890,6 +1899,22 @@ genTaskyieldOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
   return converter.getFirOpBuilder().create<mlir::omp::TaskyieldOp>(loc);
 }
 
+static mlir::omp::WorkshareOp
+genWorkshareOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
+           semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
+           mlir::Location loc, const ConstructQueue &queue,
+           ConstructQueue::iterator item) {
+  lower::StatementContext stmtCtx;
+  mlir::omp::WorkshareOperands clauseOps;
+  genWorkshareClauses(converter, semaCtx, stmtCtx, item->clauses, loc, clauseOps);
+
+  return genOpWithBody<mlir::omp::WorkshareOp>(
+      OpWithBodyGenInfo(converter, symTable, semaCtx, loc, eval,
+                        llvm::omp::Directive::OMPD_workshare)
+          .setClauses(&item->clauses),
+      queue, item, clauseOps);
+}
+
 static mlir::omp::TeamsOp
 genTeamsOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
            semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
@@ -2249,10 +2274,7 @@ static void genOMPDispatch(lower::AbstractConverter &converter,
                   llvm::omp::getOpenMPDirectiveName(dir) + ")");
   // case llvm::omp::Directive::OMPD_workdistribute:
   case llvm::omp::Directive::OMPD_workshare:
-    // FIXME: Workshare is not a commonly used OpenMP construct, an
-    // implementation for this feature will come later. For the codes
-    // that use this construct, add a single construct for now.
-    genSingleOp(converter, symTable, semaCtx, eval, loc, queue, item);
+    genWorkshareOp(converter, symTable, semaCtx, eval, loc, queue, item);
     break;
 
   // Composite constructs
diff --git a/flang/test/Lower/OpenMP/workshare.f90 b/flang/test/Lower/OpenMP/workshare.f90
index 1e11677a15e1f0..8e771952f5b6da 100644
--- a/flang/test/Lower/OpenMP/workshare.f90
+++ b/flang/test/Lower/OpenMP/workshare.f90
@@ -6,7 +6,7 @@ subroutine sb1(arr)
   integer :: arr(:)
 !CHECK: omp.parallel  {
   !$omp parallel
-!CHECK: omp.single  {
+!CHECK: omp.workshare {
   !$omp workshare
     arr = 0
   !$omp end workshare
@@ -20,7 +20,7 @@ subroutine sb2(arr)
   integer :: arr(:)
 !CHECK: omp.parallel  {
   !$omp parallel
-!CHECK: omp.single nowait {
+!CHECK: omp.workshare nowait {
   !$omp workshare
     arr = 0
   !$omp end workshare nowait
@@ -33,7 +33,7 @@ subroutine sb2(arr)
 subroutine sb3(arr)
   integer :: arr(:)
 !CHECK: omp.parallel  {
-!CHECK: omp.single  {
+!CHECK: omp.workshare  {
   !$omp parallel workshare
     arr = 0
   !$omp end parallel workshare

@ivanradanov ivanradanov force-pushed the users/ivanradanov/flang-workshare-omp branch from d030f24 to 72e19fa Compare August 19, 2024 08:40
@ivanradanov ivanradanov force-pushed the flang-workshare-emit branch from d17c552 to 621b017 Compare August 19, 2024 08:40
@tblah
Copy link
Contributor

tblah commented Aug 21, 2024

Should we have a -use-experimental-workshare or similar flag to facilitate some temporary in-tree development as this may require more moving pieces?

A flag like that sounds appropriate yes. The current code changes look good.

@ivanradanov ivanradanov force-pushed the users/ivanradanov/flang-workshare-omp branch from 72e19fa to 45a8aa4 Compare August 21, 2024 14:07
@ivanradanov ivanradanov force-pushed the flang-workshare-emit branch from 621b017 to 5e01e41 Compare August 21, 2024 14:07
Copy link

github-actions bot commented Aug 21, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@ivanradanov ivanradanov force-pushed the users/ivanradanov/flang-workshare-omp branch from 45a8aa4 to 8503797 Compare August 22, 2024 03:48
@ivanradanov ivanradanov force-pushed the flang-workshare-emit branch from 5e01e41 to 70daa01 Compare August 22, 2024 03:48
@ivanradanov ivanradanov force-pushed the users/ivanradanov/flang-workshare-omp branch from 8503797 to d343f3a Compare August 22, 2024 08:59
@ivanradanov ivanradanov force-pushed the flang-workshare-emit branch 2 times, most recently from fb06794 to de32599 Compare August 22, 2024 09:09
Copy link
Contributor

@tblah tblah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks

Copy link
Member

@skatrak skatrak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, LGTM as well.

loc, llvm::omp::Directive::OMPD_taskwait);
}

static void genWorkshareClauses(lower::AbstractConverter &converter,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ultra-nit: Move below genTeamsClauses to keep alphabetical sorting.

@ivanradanov ivanradanov force-pushed the users/ivanradanov/flang-workshare-omp branch from 45a5069 to f8d7b47 Compare October 19, 2024 16:37
@ivanradanov ivanradanov force-pushed the flang-workshare-emit branch 2 times, most recently from 2ff1ac1 to e23cf32 Compare October 19, 2024 17:22
@ivanradanov ivanradanov force-pushed the users/ivanradanov/flang-workshare-omp branch from d001eec to fa6e0dd Compare November 19, 2024 04:02
@ivanradanov ivanradanov changed the base branch from users/ivanradanov/flang-workshare-omp to main November 19, 2024 07:43
Fix lower test for workshare

Fix function signature
@ivanradanov ivanradanov merged commit 7d6713d into llvm:main Nov 19, 2024
5 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

flang:fir-hlfir flang:openmp flang Flang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants