[Flang][OpenMP] Add semantic checks for Worshare construct

[Thirumalai-Shaktivel]

Add missing semantic checks for Workshare construct:
OpenMP 5.2: 11.4 workshare Construct

• The construct must not contain any user-defined function calls unless either the function is pure and elemental or the function call is contained inside a parallel construct that is nested inside the workshare construct.
  • Flang-new used to check only the elemental function, but now it needs to be impure elemental function
• At most one NoWait clause can appear in the Workshare construct.
• Add tests for the same.

[llvmbot]

@llvm/pr-subscribers-flang-semantics

@llvm/pr-subscribers-flang-openmp

Author: Thirumalai Shaktivel (Thirumalai-Shaktivel)

Changes

---

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

3 Files Affected:

• (modified) flang/lib/Semantics/check-omp-structure.cpp (+14-5)
• (modified) flang/test/Semantics/OpenMP/workshare02.f90 (+18)
• (modified) llvm/include/llvm/Frontend/OpenMP/OMP.td (+1-1)

diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 5ef504aa72326e..890e5f8b159888 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -68,11 +68,20 @@ class OmpWorkshareBlockChecker {
     if (const auto *e{GetExpr(context_, expr)}) {
       for (const Symbol &symbol : evaluate::CollectSymbols(*e)) {
         const Symbol &root{GetAssociationRoot(symbol)};
-        if (IsFunction(root) && !IsElementalProcedure(root)) {
-          context_.Say(expr.source,
-              "User defined non-ELEMENTAL function "
-              "'%s' is not allowed in a WORKSHARE construct"_err_en_US,
-              root.name());
+        if (IsFunction(root)) {
+          std::string attrs = "";
+          if (!IsElementalProcedure(root)) {
+            attrs = " non-ELEMENTAL";
+          }
+          if (symbol.attrs().test(Attr::IMPURE)) {
+            attrs = " IMPURE" + attrs;
+          }
+          if (attrs != "") {
+            context_.Say(expr.source,
+                "User defined%s function '%s' is not allowed in a "
+                "WORKSHARE construct"_err_en_US,
+                attrs, root.name());
+          }
         }
       }
     }
diff --git a/flang/test/Semantics/OpenMP/workshare02.f90 b/flang/test/Semantics/OpenMP/workshare02.f90
index 11f33d63a3eb80..da8fd76542a865 100644
--- a/flang/test/Semantics/OpenMP/workshare02.f90
+++ b/flang/test/Semantics/OpenMP/workshare02.f90
@@ -9,6 +9,14 @@ module my_mod
   integer function my_func()
     my_func = 10
   end function my_func
+
+  impure integer function impure_my_func()
+    impure_my_func_ = 20
+  end function impure_my_func
+
+  impure elemental integer function impure_ele_my_func()
+    impure_ele_my_func = 20
+  end function impure_ele_my_func
 end module my_mod
 
 subroutine workshare(aa, bb, cc, dd, ee, ff, n)
@@ -61,6 +69,16 @@ subroutine workshare(aa, bb, cc, dd, ee, ff, n)
   j = j - my_func()
   !$omp end atomic
 
+  !ERROR: User defined IMPURE non-ELEMENTAL function 'impure_my_func' is not allowed in a WORKSHARE construct
+  cc = impure_my_func()
+  !ERROR: User defined IMPURE function 'impure_ele_my_func' is not allowed in a WORKSHARE construct
+  aa(1) = impure_ele_my_func()
+
   !$omp end workshare
 
+  !$omp workshare
+    j = j + 1
+  !ERROR: At most one NOWAIT clause can appear on the END WORKSHARE directive
+  !$omp end workshare nowait nowait
+
 end subroutine workshare
diff --git a/llvm/include/llvm/Frontend/OpenMP/OMP.td b/llvm/include/llvm/Frontend/OpenMP/OMP.td
index fcf087d1f9c6e4..dbcbe75458677b 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMP.td
+++ b/llvm/include/llvm/Frontend/OpenMP/OMP.td
@@ -1162,7 +1162,7 @@ def OMP_Workshare : Directive<"workshare"> {
   let category = CA_Executable;
 }
 def OMP_EndWorkshare : Directive<"end workshare"> {
-  let allowedClauses = [
+  let allowedOnceClauses = [
     VersionedClause<OMPC_NoWait>,
   ];
   let leafConstructs = OMP_Workshare.leafConstructs;

[kiranchandramohan]

Can you update the summary to mention which checks you are adding? And a reference to the standard and section as well.

[kiranchandramohan]

Use braced initialization in frontend code.

[kiranchandramohan]

Should there be a comma?

[Thirumalai-Shaktivel]

Between the IMPURE and non-ElEMENTAL?

User defined IMPURE, non-ELEMENTAL function

[Thirumalai-Shaktivel]

I addressed the comment in the recent commit, @kiranchandramohan

[tblah]

nit: please could you add a test for this check as well. Something in flang/test/ is okay

Otherwise this LGTM

[Thirumalai-Shaktivel]

I have added a test here: https://github.com/llvm/llvm-project/pull/111358/files#diff-3b5218aa589f253f6fe3c0637625dfe6c38d9dc425d5290a626b2b6796f06db5R79-R82. Is it fine or should I move it to flang/test

[tblah]

Ahh my mistake

[Thirumalai-Shaktivel]

No issues, @tblah. Thanks for the review!

[tblah]

Thanks!

[kiranchandramohan]

LG.

[Thirumalai-Shaktivel]

Thanks for the reviews. As this PR is approved, I'm merging it.