-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[flang][OpenMP] Don't try to privatize FORALL/DO CONCURRENT indices #123341
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
Conversation
FORALL indices have predetermined private DSA (OpenMP 5.2 5.1.1). As lowering already makes them private, don't modify FORALL index symbols. Fixes llvm#120023
@llvm/pr-subscribers-flang-semantics @llvm/pr-subscribers-flang-openmp Author: Leandro Lupori (luporl) ChangesFORALL indices have predetermined private DSA (OpenMP 5.2 5.1.1). As lowering already makes them private, don't modify FORALL index Fixes #120023 Full diff: https://github.com/llvm/llvm-project/pull/123341.diff 2 Files Affected:
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 39478b58a9070d..878021e0f71609 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -2119,6 +2119,7 @@ static bool IsPrivatizable(const Symbol *sym) {
*sym) && /* OpenMP 5.2, 5.1.1: Assumed-size arrays are shared*/
!sym->owner().IsDerivedType() &&
sym->owner().kind() != Scope::Kind::ImpliedDos &&
+ sym->owner().kind() != Scope::Kind::Forall &&
!sym->detailsIf<semantics::AssocEntityDetails>() &&
!sym->detailsIf<semantics::NamelistDetails>() &&
(!misc ||
diff --git a/flang/test/Semantics/OpenMP/forall.f90 b/flang/test/Semantics/OpenMP/forall.f90
new file mode 100644
index 00000000000000..c7fb8c64edb24e
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/forall.f90
@@ -0,0 +1,30 @@
+! RUN: %python %S/../test_symbols.py %s %flang_fc1 -fopenmp
+
+! OpenMP 5.2 5.1.1 Variables Referenced in a Construct
+! FORALL indices have predetermined private DSA.
+! As lowering already makes them private, check that their symbols are not
+! modified.
+
+ !DEF: /MainProgram1/a ObjectEntity INTEGER(4)
+ !DEF: /MainProgram1/b ObjectEntity INTEGER(4)
+ integer a(5), b(5)
+
+ !REF: /MainProgram1/a
+ a = 0
+ !REF: /MainProgram1/b
+ b = 0
+
+ !$omp parallel
+ !DEF: /MainProgram1/OtherConstruct1/Forall1/i (Implicit) ObjectEntity INTEGER(4)
+ !DEF: /MainProgram1/OtherConstruct1/a HostAssoc INTEGER(4)
+ !DEF: /MainProgram1/OtherConstruct1/b HostAssoc INTEGER(4)
+ forall(i = 1:5) a(i) = b(i) * 2
+ !$omp end parallel
+
+ !$omp parallel default(private)
+ !DEF: /MainProgram1/OtherConstruct2/Forall1/i (Implicit) ObjectEntity INTEGER(4)
+ !DEF: /MainProgram1/OtherConstruct2/a (OmpPrivate) HostAssoc INTEGER(4)
+ !DEF: /MainProgram1/OtherConstruct2/b (OmpPrivate) HostAssoc INTEGER(4)
+ forall(i = 1:5) a(i) = b(i) * 2
+ !$omp end parallel
+end program
|
@@ -1777,28 +1777,13 @@ void OmpAttributeVisitor::ResolveSeqLoopIndexInParallelOrTaskConstruct( | |||
// Use of DO CONCURRENT inside OpenMP construct is unspecified behavior | |||
// till OpenMP-5.0 standard. | |||
// In above both cases we skip the privatization of iteration variables. | |||
// [OpenMP 5.1] DO CONCURRENT indices are private |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess it would be better to include a comment like the one below to explain why nothing is done for DO CONCURRENT in this function:
// DO CONCURRENT indices have predetermined private DSA, but as they are
// defined in the construct itself, and OpenMP directives may not appear in it,
// they are already private.
I was hoping that it would be the same change required for both FORALL and DOConcurrent. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LG.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Thanks for the reviews. I've run the gfortran tests and there were no failures. |
FORALL/DO CONCURRENT indices have predetermined private DSA (OpenMP 5.2 5.1.1).
As FORALL/DO CONCURRENT indices are defined in the construct itself, and OpenMP
directives may not appear in it, they are already private and don't need to be modified.
Fixes #100919
Fixes #120023
Fixes #123537