Skip to content

[flang][OpenMP] Consider previous DSA for static duration variables #143601

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 11, 2025

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