Skip to content

Conversation

@mrkajetanp
Copy link
Contributor

Symbols that have a pre-existing DSA set in the enclosing context should not be made shared based on them being static duration variables.

Suggested-by: Leandro Lupori [email protected]

Symbols that have a pre-existing DSA set in the enclosing context
should not be made shared based on them being static duration
variables.

Suggested-by: Leandro Lupori <[email protected]>
Signed-off-by: Kajetan Puchalski <[email protected]>
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:openmp flang:semantics labels Jun 10, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 10, 2025

@llvm/pr-subscribers-flang-semantics

@llvm/pr-subscribers-flang-openmp

Author: Kajetan Puchalski (mrkajetanp)

Changes

Symbols that have a pre-existing DSA set in the enclosing context should not be made shared based on them being static duration variables.

Suggested-by: Leandro Lupori <[email protected]>


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

1 Files Affected:

  • (modified) flang/lib/Semantics/resolve-directives.cpp (+3-1)
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 65823adcef19d..93bf510fbc3c7 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -2382,7 +2382,9 @@ 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) || isStaticStorageDuration) {
+      if (prevDSA.test(Symbol::Flag::OmpShared) ||
+          (isStaticStorageDuration &&
+              (prevDSA & dataSharingAttributeFlags).none())) {
         // 6) shared in enclosing context -> shared
         dsa = {Symbol::Flag::OmpShared};
         makeSymbol(dsa);

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.

Please could you add a lit test for this case to guard against it regressing again in the future

@mrkajetanp mrkajetanp requested a review from tblah June 11, 2025 12:36
Copy link
Contributor

@luporl luporl left a comment

Choose a reason for hiding this comment

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

LGTM

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.

Thanks!

@mrkajetanp mrkajetanp merged commit cc9f674 into llvm:main Jun 11, 2025
7 checks passed
tomtor pushed a commit to tomtor/llvm-project that referenced this pull request Jun 14, 2025
…lvm#143601)

Symbols that have a pre-existing DSA set in the enclosing context should
not be made shared based on them being static duration variables.

Suggested-by: Leandro Lupori <[email protected]>

---------

Signed-off-by: Kajetan Puchalski <[email protected]>
akuhlens pushed a commit to akuhlens/llvm-project that referenced this pull request Jun 24, 2025
…lvm#143601)

Symbols that have a pre-existing DSA set in the enclosing context should
not be made shared based on them being static duration variables.

Suggested-by: Leandro Lupori <[email protected]>

---------

Signed-off-by: Kajetan Puchalski <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

flang:openmp flang:semantics flang Flang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants