[flang][OpenMP] Make static duration variables default to shared DSA

[mrkajetanp]

According to the OpenMP standard, variables with static storage duration are predetermined as shared.
Add a check when creating implicit symbols for OpenMP to fix them erroneously getting set to firstprivate.

Fixes #140732.

[llvmbot]

@llvm/pr-subscribers-flang-semantics

@llvm/pr-subscribers-flang-openmp

Author: Kajetan Puchalski (mrkajetanp)

Changes

According to the OpenMP standard, variables with static storage duration are predetermined as shared.
Add a check when creating implicit symbols for OpenMP to fix them erroneously getting set to firstprivate.

Fixes llvm#140732.

---

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

2 Files Affected:

• (modified) flang/lib/Semantics/resolve-directives.cpp (+4-1)
• (modified) flang/test/Semantics/OpenMP/implicit-dsa.f90 (+20)

diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index ec385b7baf8bd..d1495e7b3b7f2 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -2310,6 +2310,8 @@ void OmpAttributeVisitor::CreateImplicitSymbols(const Symbol *symbol) {
     bool targetDir = llvm::omp::allTargetSet.test(dirContext.directive);
     bool parallelDir = llvm::omp::allParallelSet.test(dirContext.directive);
     bool teamsDir = llvm::omp::allTeamsSet.test(dirContext.directive);
+    bool isStaticStorageDuration =
+        symbol->flags().test(Symbol::Flag::InDataStmt);
 
     if (dsa.any()) {
       if (parallelDir || taskGenDir || teamsDir) {
@@ -2367,7 +2369,8 @@ void OmpAttributeVisitor::CreateImplicitSymbols(const Symbol *symbol) {
       dsa = prevDSA;
     } else if (taskGenDir) {
       // TODO 5) dummy arg in orphaned taskgen construct -> firstprivate
-      if (prevDSA.test(Symbol::Flag::OmpShared)) {
+      // variables with static storage duration are predetermined as shared
+      if (prevDSA.test(Symbol::Flag::OmpShared) || isStaticStorageDuration) {
         // 6) shared in enclosing context -> shared
         dsa = {Symbol::Flag::OmpShared};
         makeSymbol(dsa);
diff --git a/flang/test/Semantics/OpenMP/implicit-dsa.f90 b/flang/test/Semantics/OpenMP/implicit-dsa.f90
index 7e38435274b7b..9e48dd2a83257 100644
--- a/flang/test/Semantics/OpenMP/implicit-dsa.f90
+++ b/flang/test/Semantics/OpenMP/implicit-dsa.f90
@@ -169,3 +169,23 @@ subroutine implicit_dsa_test8
     end do
   !$omp end task
 end subroutine
+
+! Test static storage duration variables default to shared DSA
+!DEF: /implicit_dsa_test9_mod Module
+module implicit_dsa_test9_mod
+ !DEF: /implicit_dsa_test9_mod/tm3a PUBLIC (InDataStmt) ObjectEntity COMPLEX(4)
+  complex tm3a/(0,0)/
+contains
+ !DEF: /implicit_dsa_test9_mod/implict_dsa_test9 PUBLIC (Subroutine) Subprogram
+  subroutine implict_dsa_test9
+    !$omp task
+      !$omp task
+        !DEF: /implicit_dsa_test9_mod/implict_dsa_test9/OtherConstruct1/OtherConstruct1/tm3a (OmpShared) HostAssoc COMPLEX(4)
+        tm3a = (1, 2)
+      !$omp end task
+    !$omp end task
+  !$omp taskwait
+  !REF: /implicit_dsa_test9_mod/tm3a
+  print *,tm3a
+  end subroutine
+end module

[kiranchandramohan]

The OpenMP standard says the following for static storage duration:

For Fortran, the lifetime of a variable with a SAVE attribute, implicit or explicit, a
common block object or a variable declared in a module.

Could you comment on whether all these cases (variable with SAVE, common block object, variables in module) are handled in this patch?

[mrkajetanp]

Could you comment on whether all these cases (variable with SAVE, common block object, variables in module) are handled in this patch?

The patch checks for the InDataStmt flag, which I think should be set in all of these cases. I'll add tests for those alongside the changes Tom asked for to make sure.

[mrkajetanp]

I've checked the other cases. Variables with SAVE and common block objects do not actually have any Symbol flags set on them as things stand, so with the patch as-is they will not be shared. Should the approach then be to always determine whether the symbol has static storage duration and if so make it shared? I think that'd require adding some more plumbing to get the right information.