-
Notifications
You must be signed in to change notification settings - Fork 10.8k
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
[flang][OpenMP] Allow common blocks in nested directives #88430
Conversation
@llvm/pr-subscribers-flang-semantics @llvm/pr-subscribers-flang-openmp Author: Leandro Lupori (luporl) ChangesCOMMON block names must be declared in the same scoping unit in This patch implements this behavior by also looking for common Full diff: https://github.com/llvm/llvm-project/pull/88430.diff 2 Files Affected:
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 27af192f606be9..ac1c988e174275 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -2096,10 +2096,19 @@ Symbol *OmpAttributeVisitor::ResolveOmpCommonBlockName(
name->symbol = cur;
return cur;
}
- // Then check parent scope
- if (auto *prev{GetContext().scope.parent().FindCommonBlock(name->source)}) {
- name->symbol = prev;
- return prev;
+ // Then check parent scope, skipping OpenMP scopes and making sure
+ // that the top directive started its own scope.
+ DirContext &topDirContext = dirContext_.front();
+ Scope &topDirScope = topDirContext.scope;
+ assert(!topDirContext.directiveSource.empty());
+ assert(!topDirScope.sourceRange().empty());
+ if (!topDirScope.IsTopLevel() &&
+ topDirContext.directiveSource.begin() ==
+ topDirScope.sourceRange().begin()) {
+ if (auto *prev{topDirScope.parent().FindCommonBlock(name->source)}) {
+ name->symbol = prev;
+ return prev;
+ }
}
return nullptr;
}
diff --git a/flang/test/Semantics/OpenMP/resolve03.f90 b/flang/test/Semantics/OpenMP/resolve03.f90
index b9306c4fe9cb4d..1228f070e0595f 100644
--- a/flang/test/Semantics/OpenMP/resolve03.f90
+++ b/flang/test/Semantics/OpenMP/resolve03.f90
@@ -8,6 +8,9 @@
common /c/ a, b
integer a(3), b
+ common /tc/ xy
+ integer xz
+ !$omp threadprivate(/tc/)
A = 1
B = 2
@@ -19,4 +22,17 @@
!$omp end parallel
end block
print *, a, b
+
+ ! Common block names may be used inside nested OpenMP directives.
+ !$omp parallel
+ !$omp parallel copyin(/tc/)
+ xz = xz + 10
+ !$omp end parallel
+ !$omp end parallel
+
+ !$omp parallel
+ !$omp single
+ xz = 18
+ !$omp end single copyprivate(/tc/)
+ !$omp end parallel
end
|
This fixes the first issue reported here: #86907 |
I just realized now that looking for common blocks in the scope of the top directive is wrong in some cases, as this may skip other non-OpenMP scopes. |
COMMON block names must be declared in the same scoping unit in which the OpenMP directive or clause appears. However, given that common blocks and threadprivate directives can't be nested inside other directives, such as parallel, and based on the behavior of other compilers, it seems that OpenMP block constructs should not be considered as scoping units. This patch implements this behavior by also looking for common block names in the scoping unit of the outermost directive.
7581f96
to
54a0df1
Compare
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. Thanks for the fix.
COMMON block names must be declared in the same scoping unit in
which the OpenMP directive or clause appears, but OpenMP
constructs must not be considered as scoping units. Instead,
consider only program units and block constructs as such.