[llvm-branch-commits] [flang] [flang][OpenMP] Insert CRITICAL construct names into global scope (PR #152004)
https://github.com/kparzysz updated
https://github.com/llvm/llvm-project/pull/152004
>From 4187a39e449369fa8e9b4917e1b2af91ebbb057d Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek
Date: Mon, 4 Aug 2025 11:58:35 -0500
Subject: [PATCH 1/2] [flang][OpenMP] Insert CRITICAL construct names into
global scope
OpenMP spec (all versions):
The names of critical constructs are global entities of the program.
If a name conflicts with any other entity, the behavior of the program
is unspecified.
---
flang/lib/Semantics/resolve-directives.cpp| 9
flang/lib/Semantics/resolve-names.cpp | 44 +++
.../OpenMP/critical-global-conflict.f90 | 15 +++
.../OpenMP/critical_within_default.f90| 7 ++-
4 files changed, 65 insertions(+), 10 deletions(-)
create mode 100644 flang/test/Semantics/OpenMP/critical-global-conflict.f90
diff --git a/flang/lib/Semantics/resolve-directives.cpp
b/flang/lib/Semantics/resolve-directives.cpp
index bb28cfb61764f..64bb27962faab 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -2125,17 +2125,8 @@ bool OmpAttributeVisitor::Pre(const
parser::OpenMPSectionConstruct &x) {
bool OmpAttributeVisitor::Pre(const parser::OpenMPCriticalConstruct &x) {
const auto &beginCriticalDir{std::get(x.t)};
- const auto &endCriticalDir{std::get(x.t)};
PushContext(beginCriticalDir.source, llvm::omp::Directive::OMPD_critical);
GetContext().withinConstruct = true;
- if (const auto &criticalName{
- std::get>(beginCriticalDir.t)}) {
-ResolveOmpName(*criticalName, Symbol::Flag::OmpCriticalLock);
- }
- if (const auto &endCriticalName{
- std::get>(endCriticalDir.t)}) {
-ResolveOmpName(*endCriticalName, Symbol::Flag::OmpCriticalLock);
- }
return true;
}
diff --git a/flang/lib/Semantics/resolve-names.cpp
b/flang/lib/Semantics/resolve-names.cpp
index 25b13700cd3ab..86201ebee8bdf 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -1593,6 +1593,14 @@ class OmpVisitor : public virtual DeclarationVisitor {
}
bool Pre(const parser::OmpCriticalDirective &x) {
AddOmpSourceRange(x.source);
+// Manually resolve names in CRITICAL directives. This is because these
+// names do not denote Fortran objects, and the CRITICAL directive causes
+// them to be "auto-declared", i.e. inserted into the global scope.
+// More specifically, they are not expected to have explicit declarations,
+// and if they do the behavior is unspeficied.
+if (auto &maybeName{std::get>(x.t)}) {
+ ResolveCriticalName(*maybeName);
+}
return true;
}
void Post(const parser::OmpCriticalDirective &) {
@@ -1600,6 +1608,10 @@ class OmpVisitor : public virtual DeclarationVisitor {
}
bool Pre(const parser::OmpEndCriticalDirective &x) {
AddOmpSourceRange(x.source);
+// Manually resolve names in CRITICAL directives.
+if (auto &maybeName{std::get>(x.t)}) {
+ ResolveCriticalName(*maybeName);
+}
return true;
}
void Post(const parser::OmpEndCriticalDirective &) {
@@ -1720,6 +1732,8 @@ class OmpVisitor : public virtual DeclarationVisitor {
const std::optional &clauses,
const T &wholeConstruct);
+ void ResolveCriticalName(const parser::Name &name);
+
int metaLevel_{0};
const parser::OmpMetadirectiveDirective *metaDirective_{nullptr};
};
@@ -1947,6 +1961,36 @@ void OmpVisitor::ProcessReductionSpecifier(
}
}
+void OmpVisitor::ResolveCriticalName(const parser::Name &name) {
+ auto &globalScope{[&]() -> Scope & {
+for (Scope *s{&currScope()};; s = &s->parent()) {
+ if (s->IsTopLevel()) {
+return *s;
+ }
+}
+llvm_unreachable("Cannot find global scope");
+ }()};
+
+ auto findSymbol{[&](const parser::Name &n) {
+if (auto *s{FindSymbol(n)}) {
+ return s;
+} else {
+ return FindInScope(globalScope, n);
+}
+ }};
+
+ if (auto *symbol{findSymbol(name)}) {
+if (!symbol->test(Symbol::Flag::OmpCriticalLock)) {
+ SayWithDecl(name, *symbol,
+ "CRITICAL construct name '%s' conflicts with a previous
declaration"_warn_en_US,
+ name.ToString());
+}
+ } else {
+name.symbol = &MakeSymbol(globalScope, name.source, Attrs{});
+name.symbol->set(Symbol::Flag::OmpCriticalLock);
+ }
+}
+
bool OmpVisitor::Pre(const parser::OmpDirectiveSpecification &x) {
AddOmpSourceRange(x.source);
if (metaLevel_ == 0) {
diff --git a/flang/test/Semantics/OpenMP/critical-global-conflict.f90
b/flang/test/Semantics/OpenMP/critical-global-conflict.f90
new file mode 100644
index 0..cee6f2f14b373
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/critical-global-conflict.f90
@@ -0,0 +1,15 @@
+! RUN: %python %S/../test_errors.py %s %flang_fc1 -fopenmp -Werror
+
+subroutine g
+end
+
+subroutine f(x)
+ implicit none
+ integer :: x
+
+!ERROR: CRITICAL construct name 'f' conflicts with a previ
[llvm-branch-commits] [flang] [flang][OpenMP] Insert CRITICAL construct names into global scope (PR #152004)
llvmbot wrote:
@llvm/pr-subscribers-flang-openmp
Author: Krzysztof Parzyszek (kparzysz)
Changes
They were inserted in the current scope.
OpenMP spec (all versions):
The names of critical constructs are global entities of the program. If a name
conflicts with any other entity, the behavior of the program is unspecified.
---
Full diff: https://github.com/llvm/llvm-project/pull/152004.diff
4 Files Affected:
- (modified) flang/lib/Semantics/resolve-directives.cpp (-9)
- (modified) flang/lib/Semantics/resolve-names.cpp (+44)
- (added) flang/test/Semantics/OpenMP/critical-global-conflict.f90 (+15)
- (modified) flang/test/Semantics/OpenMP/critical_within_default.f90 (+6-1)
``diff
diff --git a/flang/lib/Semantics/resolve-directives.cpp
b/flang/lib/Semantics/resolve-directives.cpp
index bb28cfb61764f..64bb27962faab 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -2125,17 +2125,8 @@ bool OmpAttributeVisitor::Pre(const
parser::OpenMPSectionConstruct &x) {
bool OmpAttributeVisitor::Pre(const parser::OpenMPCriticalConstruct &x) {
const auto &beginCriticalDir{std::get(x.t)};
- const auto &endCriticalDir{std::get(x.t)};
PushContext(beginCriticalDir.source, llvm::omp::Directive::OMPD_critical);
GetContext().withinConstruct = true;
- if (const auto &criticalName{
- std::get>(beginCriticalDir.t)}) {
-ResolveOmpName(*criticalName, Symbol::Flag::OmpCriticalLock);
- }
- if (const auto &endCriticalName{
- std::get>(endCriticalDir.t)}) {
-ResolveOmpName(*endCriticalName, Symbol::Flag::OmpCriticalLock);
- }
return true;
}
diff --git a/flang/lib/Semantics/resolve-names.cpp
b/flang/lib/Semantics/resolve-names.cpp
index 25b13700cd3ab..86201ebee8bdf 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -1593,6 +1593,14 @@ class OmpVisitor : public virtual DeclarationVisitor {
}
bool Pre(const parser::OmpCriticalDirective &x) {
AddOmpSourceRange(x.source);
+// Manually resolve names in CRITICAL directives. This is because these
+// names do not denote Fortran objects, and the CRITICAL directive causes
+// them to be "auto-declared", i.e. inserted into the global scope.
+// More specifically, they are not expected to have explicit declarations,
+// and if they do the behavior is unspeficied.
+if (auto &maybeName{std::get>(x.t)}) {
+ ResolveCriticalName(*maybeName);
+}
return true;
}
void Post(const parser::OmpCriticalDirective &) {
@@ -1600,6 +1608,10 @@ class OmpVisitor : public virtual DeclarationVisitor {
}
bool Pre(const parser::OmpEndCriticalDirective &x) {
AddOmpSourceRange(x.source);
+// Manually resolve names in CRITICAL directives.
+if (auto &maybeName{std::get>(x.t)}) {
+ ResolveCriticalName(*maybeName);
+}
return true;
}
void Post(const parser::OmpEndCriticalDirective &) {
@@ -1720,6 +1732,8 @@ class OmpVisitor : public virtual DeclarationVisitor {
const std::optional &clauses,
const T &wholeConstruct);
+ void ResolveCriticalName(const parser::Name &name);
+
int metaLevel_{0};
const parser::OmpMetadirectiveDirective *metaDirective_{nullptr};
};
@@ -1947,6 +1961,36 @@ void OmpVisitor::ProcessReductionSpecifier(
}
}
+void OmpVisitor::ResolveCriticalName(const parser::Name &name) {
+ auto &globalScope{[&]() -> Scope & {
+for (Scope *s{&currScope()};; s = &s->parent()) {
+ if (s->IsTopLevel()) {
+return *s;
+ }
+}
+llvm_unreachable("Cannot find global scope");
+ }()};
+
+ auto findSymbol{[&](const parser::Name &n) {
+if (auto *s{FindSymbol(n)}) {
+ return s;
+} else {
+ return FindInScope(globalScope, n);
+}
+ }};
+
+ if (auto *symbol{findSymbol(name)}) {
+if (!symbol->test(Symbol::Flag::OmpCriticalLock)) {
+ SayWithDecl(name, *symbol,
+ "CRITICAL construct name '%s' conflicts with a previous
declaration"_warn_en_US,
+ name.ToString());
+}
+ } else {
+name.symbol = &MakeSymbol(globalScope, name.source, Attrs{});
+name.symbol->set(Symbol::Flag::OmpCriticalLock);
+ }
+}
+
bool OmpVisitor::Pre(const parser::OmpDirectiveSpecification &x) {
AddOmpSourceRange(x.source);
if (metaLevel_ == 0) {
diff --git a/flang/test/Semantics/OpenMP/critical-global-conflict.f90
b/flang/test/Semantics/OpenMP/critical-global-conflict.f90
new file mode 100644
index 0..cee6f2f14b373
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/critical-global-conflict.f90
@@ -0,0 +1,15 @@
+! RUN: %python %S/../test_errors.py %s %flang_fc1 -fopenmp -Werror
+
+subroutine g
+end
+
+subroutine f(x)
+ implicit none
+ integer :: x
+
+!ERROR: CRITICAL construct name 'f' conflicts with a previous declaration
+ !$omp critical(f)
+ x = 0
+!ERROR: CRITICAL construct name 'f' conflicts with a previous declaration
+ !$omp end critical(f)
+end
diff --git a/f
[llvm-branch-commits] [flang] [flang][OpenMP] Insert CRITICAL construct names into global scope (PR #152004)
https://github.com/kparzysz created
https://github.com/llvm/llvm-project/pull/152004
They were inserted in the current scope.
OpenMP spec (all versions):
The names of critical constructs are global entities of the program. If a name
conflicts with any other entity, the behavior of the program is unspecified.
>From 4187a39e449369fa8e9b4917e1b2af91ebbb057d Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek
Date: Mon, 4 Aug 2025 11:58:35 -0500
Subject: [PATCH] [flang][OpenMP] Insert CRITICAL construct names into global
scope
OpenMP spec (all versions):
The names of critical constructs are global entities of the program.
If a name conflicts with any other entity, the behavior of the program
is unspecified.
---
flang/lib/Semantics/resolve-directives.cpp| 9
flang/lib/Semantics/resolve-names.cpp | 44 +++
.../OpenMP/critical-global-conflict.f90 | 15 +++
.../OpenMP/critical_within_default.f90| 7 ++-
4 files changed, 65 insertions(+), 10 deletions(-)
create mode 100644 flang/test/Semantics/OpenMP/critical-global-conflict.f90
diff --git a/flang/lib/Semantics/resolve-directives.cpp
b/flang/lib/Semantics/resolve-directives.cpp
index bb28cfb61764f..64bb27962faab 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -2125,17 +2125,8 @@ bool OmpAttributeVisitor::Pre(const
parser::OpenMPSectionConstruct &x) {
bool OmpAttributeVisitor::Pre(const parser::OpenMPCriticalConstruct &x) {
const auto &beginCriticalDir{std::get(x.t)};
- const auto &endCriticalDir{std::get(x.t)};
PushContext(beginCriticalDir.source, llvm::omp::Directive::OMPD_critical);
GetContext().withinConstruct = true;
- if (const auto &criticalName{
- std::get>(beginCriticalDir.t)}) {
-ResolveOmpName(*criticalName, Symbol::Flag::OmpCriticalLock);
- }
- if (const auto &endCriticalName{
- std::get>(endCriticalDir.t)}) {
-ResolveOmpName(*endCriticalName, Symbol::Flag::OmpCriticalLock);
- }
return true;
}
diff --git a/flang/lib/Semantics/resolve-names.cpp
b/flang/lib/Semantics/resolve-names.cpp
index 25b13700cd3ab..86201ebee8bdf 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -1593,6 +1593,14 @@ class OmpVisitor : public virtual DeclarationVisitor {
}
bool Pre(const parser::OmpCriticalDirective &x) {
AddOmpSourceRange(x.source);
+// Manually resolve names in CRITICAL directives. This is because these
+// names do not denote Fortran objects, and the CRITICAL directive causes
+// them to be "auto-declared", i.e. inserted into the global scope.
+// More specifically, they are not expected to have explicit declarations,
+// and if they do the behavior is unspeficied.
+if (auto &maybeName{std::get>(x.t)}) {
+ ResolveCriticalName(*maybeName);
+}
return true;
}
void Post(const parser::OmpCriticalDirective &) {
@@ -1600,6 +1608,10 @@ class OmpVisitor : public virtual DeclarationVisitor {
}
bool Pre(const parser::OmpEndCriticalDirective &x) {
AddOmpSourceRange(x.source);
+// Manually resolve names in CRITICAL directives.
+if (auto &maybeName{std::get>(x.t)}) {
+ ResolveCriticalName(*maybeName);
+}
return true;
}
void Post(const parser::OmpEndCriticalDirective &) {
@@ -1720,6 +1732,8 @@ class OmpVisitor : public virtual DeclarationVisitor {
const std::optional &clauses,
const T &wholeConstruct);
+ void ResolveCriticalName(const parser::Name &name);
+
int metaLevel_{0};
const parser::OmpMetadirectiveDirective *metaDirective_{nullptr};
};
@@ -1947,6 +1961,36 @@ void OmpVisitor::ProcessReductionSpecifier(
}
}
+void OmpVisitor::ResolveCriticalName(const parser::Name &name) {
+ auto &globalScope{[&]() -> Scope & {
+for (Scope *s{&currScope()};; s = &s->parent()) {
+ if (s->IsTopLevel()) {
+return *s;
+ }
+}
+llvm_unreachable("Cannot find global scope");
+ }()};
+
+ auto findSymbol{[&](const parser::Name &n) {
+if (auto *s{FindSymbol(n)}) {
+ return s;
+} else {
+ return FindInScope(globalScope, n);
+}
+ }};
+
+ if (auto *symbol{findSymbol(name)}) {
+if (!symbol->test(Symbol::Flag::OmpCriticalLock)) {
+ SayWithDecl(name, *symbol,
+ "CRITICAL construct name '%s' conflicts with a previous
declaration"_warn_en_US,
+ name.ToString());
+}
+ } else {
+name.symbol = &MakeSymbol(globalScope, name.source, Attrs{});
+name.symbol->set(Symbol::Flag::OmpCriticalLock);
+ }
+}
+
bool OmpVisitor::Pre(const parser::OmpDirectiveSpecification &x) {
AddOmpSourceRange(x.source);
if (metaLevel_ == 0) {
diff --git a/flang/test/Semantics/OpenMP/critical-global-conflict.f90
b/flang/test/Semantics/OpenMP/critical-global-conflict.f90
new file mode 100644
index 0..cee6f2f14b373
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/critical-global-conflict
