[flang-commits] [flang] [flang][OpenMP] Insert CRITICAL construct names into global scope (PR #152004)
Krzysztof Parzyszek via flang-commits
flang-commits at lists.llvm.org
Wed Aug 6 07:01:31 PDT 2025
https://github.com/kparzysz updated https://github.com/llvm/llvm-project/pull/152004
>From 73171dd21e3e14bcf0f25e4b5e6e4ab3679ad7d5 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Mon, 4 Aug 2025 07:59:47 -0500
Subject: [PATCH 1/5] [flang][OpenMP] Fix crash in unparse-with-symbols for
CRITICAL
---
flang/lib/Semantics/unparse-with-symbols.cpp | 33 +++++++++++++++++++
.../OpenMP/critical-unparse-with-symbols.f90 | 21 ++++++++++++
2 files changed, 54 insertions(+)
create mode 100644 flang/test/Parser/OpenMP/critical-unparse-with-symbols.f90
diff --git a/flang/lib/Semantics/unparse-with-symbols.cpp b/flang/lib/Semantics/unparse-with-symbols.cpp
index f1e2e4ea7f119..4548fbeba5df6 100644
--- a/flang/lib/Semantics/unparse-with-symbols.cpp
+++ b/flang/lib/Semantics/unparse-with-symbols.cpp
@@ -70,6 +70,39 @@ class SymbolDumpVisitor {
currStmt_ = std::nullopt;
}
+ bool Pre(const parser::OmpCriticalDirective &x) {
+ currStmt_ = x.source;
+ return true;
+ }
+ void Post(const parser::OmpCriticalDirective &) {
+ currStmt_ = std::nullopt;
+ }
+
+ bool Pre(const parser::OmpEndCriticalDirective &x) {
+ currStmt_ = x.source;
+ return true;
+ }
+ void Post(const parser::OmpEndCriticalDirective &) {
+ currStmt_ = std::nullopt;
+ }
+
+ // Directive arguments can be objects with symbols.
+ bool Pre(const parser::OmpBeginDirective &x) {
+ currStmt_ = x.source;
+ return true;
+ }
+ void Post(const parser::OmpBeginDirective &) {
+ currStmt_ = std::nullopt;
+ }
+
+ bool Pre(const parser::OmpEndDirective &x) {
+ currStmt_ = x.source;
+ return true;
+ }
+ void Post(const parser::OmpEndDirective &) {
+ currStmt_ = std::nullopt;
+ }
+
private:
std::optional<SourceName> currStmt_; // current statement we are processing
std::multimap<const char *, const Symbol *> symbols_; // location to symbol
diff --git a/flang/test/Parser/OpenMP/critical-unparse-with-symbols.f90 b/flang/test/Parser/OpenMP/critical-unparse-with-symbols.f90
new file mode 100644
index 0000000000000..4d0d93ac48740
--- /dev/null
+++ b/flang/test/Parser/OpenMP/critical-unparse-with-symbols.f90
@@ -0,0 +1,21 @@
+!RUN: %flang_fc1 -fdebug-unparse-with-symbols -fopenmp -fopenmp-version=50 %s | FileCheck --ignore-case --check-prefix="UNPARSE" %s
+
+subroutine f
+ implicit none
+ integer :: x
+ !$omp critical(c)
+ x = 0
+ !$omp end critical(c)
+end
+
+!UNPARSE: !DEF: /f (Subroutine) Subprogram
+!UNPARSE: subroutine f
+!UNPARSE: implicit none
+!UNPARSE: !DEF: /f/x ObjectEntity INTEGER(4)
+!UNPARSE: integer x
+!UNPARSE: !$omp critical (c)
+!UNPARSE: !REF: /f/x
+!UNPARSE: x = 0
+!UNPARSE: !$omp end critical (c)
+!UNPARSE: end subroutine
+
>From 1b33e0ff393735c84a58d721c929a7fc74909c94 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Mon, 4 Aug 2025 08:37:04 -0500
Subject: [PATCH 2/5] format
---
flang/lib/Semantics/unparse-with-symbols.cpp | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/flang/lib/Semantics/unparse-with-symbols.cpp b/flang/lib/Semantics/unparse-with-symbols.cpp
index 4548fbeba5df6..3093e39ba2411 100644
--- a/flang/lib/Semantics/unparse-with-symbols.cpp
+++ b/flang/lib/Semantics/unparse-with-symbols.cpp
@@ -74,9 +74,7 @@ class SymbolDumpVisitor {
currStmt_ = x.source;
return true;
}
- void Post(const parser::OmpCriticalDirective &) {
- currStmt_ = std::nullopt;
- }
+ void Post(const parser::OmpCriticalDirective &) { currStmt_ = std::nullopt; }
bool Pre(const parser::OmpEndCriticalDirective &x) {
currStmt_ = x.source;
@@ -91,17 +89,13 @@ class SymbolDumpVisitor {
currStmt_ = x.source;
return true;
}
- void Post(const parser::OmpBeginDirective &) {
- currStmt_ = std::nullopt;
- }
+ void Post(const parser::OmpBeginDirective &) { currStmt_ = std::nullopt; }
bool Pre(const parser::OmpEndDirective &x) {
currStmt_ = x.source;
return true;
}
- void Post(const parser::OmpEndDirective &) {
- currStmt_ = std::nullopt;
- }
+ void Post(const parser::OmpEndDirective &) { currStmt_ = std::nullopt; }
private:
std::optional<SourceName> currStmt_; // current statement we are processing
>From 4187a39e449369fa8e9b4917e1b2af91ebbb057d Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Mon, 4 Aug 2025 11:58:35 -0500
Subject: [PATCH 3/5] [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<parser::OmpCriticalDirective>(x.t)};
- const auto &endCriticalDir{std::get<parser::OmpEndCriticalDirective>(x.t)};
PushContext(beginCriticalDir.source, llvm::omp::Directive::OMPD_critical);
GetContext().withinConstruct = true;
- if (const auto &criticalName{
- std::get<std::optional<parser::Name>>(beginCriticalDir.t)}) {
- ResolveOmpName(*criticalName, Symbol::Flag::OmpCriticalLock);
- }
- if (const auto &endCriticalName{
- std::get<std::optional<parser::Name>>(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<std::optional<parser::Name>>(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<std::optional<parser::Name>>(x.t)}) {
+ ResolveCriticalName(*maybeName);
+ }
return true;
}
void Post(const parser::OmpEndCriticalDirective &) {
@@ -1720,6 +1732,8 @@ class OmpVisitor : public virtual DeclarationVisitor {
const std::optional<parser::OmpClauseList> &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 0000000000000..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/flang/test/Semantics/OpenMP/critical_within_default.f90 b/flang/test/Semantics/OpenMP/critical_within_default.f90
index a5fe30eeb7de0..70353e8e4b585 100644
--- a/flang/test/Semantics/OpenMP/critical_within_default.f90
+++ b/flang/test/Semantics/OpenMP/critical_within_default.f90
@@ -1,11 +1,16 @@
! RUN: %flang_fc1 -fopenmp -fdebug-dump-symbols %s | FileCheck %s
! Test that we do not make a private copy of the critical name
+!CHECK: Global scope:
+!CHECK-NEXT: MN: MainProgram
+!CHECK-NEXT: k2 (OmpCriticalLock): Unknown
+
!CHECK: MainProgram scope: MN
!CHECK-NEXT: j size=4 offset=0: ObjectEntity type: INTEGER(4)
!CHECK-NEXT: OtherConstruct scope:
!CHECK-NEXT: j (OmpPrivate): HostAssoc
-!CHECK-NEXT: k2 (OmpCriticalLock): Unknown
+!CHECK-NOT: k2
+
program mn
integer :: j
j=2
>From 6d348e1814f39437630a3a1af977277241f775de Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Mon, 4 Aug 2025 12:05:41 -0500
Subject: [PATCH 4/5] Rename name in test
---
flang/test/Semantics/OpenMP/critical-global-conflict.f90 | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/flang/test/Semantics/OpenMP/critical-global-conflict.f90 b/flang/test/Semantics/OpenMP/critical-global-conflict.f90
index cee6f2f14b373..2546b68748d93 100644
--- a/flang/test/Semantics/OpenMP/critical-global-conflict.f90
+++ b/flang/test/Semantics/OpenMP/critical-global-conflict.f90
@@ -7,9 +7,9 @@ subroutine f(x)
implicit none
integer :: x
-!ERROR: CRITICAL construct name 'f' conflicts with a previous declaration
- !$omp critical(f)
+!ERROR: CRITICAL construct name 'g' conflicts with a previous declaration
+ !$omp critical(g)
x = 0
-!ERROR: CRITICAL construct name 'f' conflicts with a previous declaration
- !$omp end critical(f)
+!ERROR: CRITICAL construct name 'g' conflicts with a previous declaration
+ !$omp end critical(g)
end
>From ba4bf7595335b00defe44172da0bd617f34f7641 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Wed, 6 Aug 2025 09:00:52 -0500
Subject: [PATCH 5/5] Don't search current scope
---
flang/lib/Semantics/resolve-names.cpp | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index 86201ebee8bdf..66a45dd3d07fa 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -1971,15 +1971,7 @@ void OmpVisitor::ResolveCriticalName(const parser::Name &name) {
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 (auto *symbol{FindInScope(globalScope, name)}) {
if (!symbol->test(Symbol::Flag::OmpCriticalLock)) {
SayWithDecl(name, *symbol,
"CRITICAL construct name '%s' conflicts with a previous declaration"_warn_en_US,
More information about the flang-commits
mailing list