[flang-commits] [flang] [flang][OpenMP] Remove unnecessary code from OmpVisitor, NFC (PR #198865)
Krzysztof Parzyszek via flang-commits
flang-commits at lists.llvm.org
Thu May 21 11:55:52 PDT 2026
https://github.com/kparzysz updated https://github.com/llvm/llvm-project/pull/198865
>From 2de7fe03633a02fa351b7f45ac373c639ab55fbb Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Wed, 20 May 2026 08:19:46 -0500
Subject: [PATCH 1/2] [flang][OpenMP] Limit scope creation to constructs with
data environment
Identify specific constructs that require data envorinments, and only
create scopes for them. This avoids scopes for loop-transformation
constructs, for example.
This isn't a correctness fix, but a clarification and a simplification
of the name-resolution code for OpenMP.
---
flang/lib/Semantics/resolve-names.cpp | 104 ++++++++----------
.../test/Semantics/OpenMP/affected-loops.f90 | 5 +-
2 files changed, 45 insertions(+), 64 deletions(-)
diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index aa0357392630d..25d3e5062eb41 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -1723,19 +1723,18 @@ void AccVisitor::Post(const parser::OpenACCCombinedConstruct &x) { PopScope(); }
// Create scopes for OpenMP constructs
class OmpVisitor : public virtual DeclarationVisitor {
public:
+ static bool HasDataEnvironment(llvm::omp::Directive dir);
+
void AddOmpSourceRange(const parser::CharBlock &);
void PushScopeWithSource(
Scope::Kind kind, parser::CharBlock source, Symbol *symbol = nullptr);
- static bool NeedsScope(const parser::OmpBlockConstruct &);
static bool NeedsScope(const parser::OmpClause &);
bool Pre(const parser::OmpRequiresDirective &x) {
AddOmpSourceRange(x.source);
return true;
}
- bool Pre(const parser::OmpBlockConstruct &);
- void Post(const parser::OmpBlockConstruct &);
bool Pre(const parser::OmpBeginDirective &x) {
return Pre(static_cast<const parser::OmpDirectiveSpecification &>(x));
}
@@ -1749,12 +1748,6 @@ class OmpVisitor : public virtual DeclarationVisitor {
Post(static_cast<const parser::OmpDirectiveSpecification &>(x));
}
- bool Pre(const parser::OpenMPLoopConstruct &x) {
- PushScopeWithSource(Scope::Kind::OtherConstruct, x.source);
- return true;
- }
- void Post(const parser::OpenMPLoopConstruct &) { PopScope(); }
-
void Post(const parser::OmpTypeName &);
bool Pre(const parser::OmpStylizedDeclaration &);
void Post(const parser::OmpStylizedDeclaration &);
@@ -1798,23 +1791,6 @@ class OmpVisitor : public virtual DeclarationVisitor {
bool Pre(const parser::OmpClause::To &);
bool Pre(const parser::OmpClause::From &);
- bool Pre(const parser::OpenMPSectionsConstruct &x) {
- PushScopeWithSource(Scope::Kind::OtherConstruct, x.source);
- return true;
- }
- void Post(const parser::OpenMPSectionsConstruct &) { PopScope(); }
- bool Pre(const parser::OmpBeginSectionsDirective &x) {
- return Pre(static_cast<const parser::OmpDirectiveSpecification &>(x));
- }
- void Post(const parser::OmpBeginSectionsDirective &x) {
- Post(static_cast<const parser::OmpDirectiveSpecification &>(x));
- }
- bool Pre(const parser::OmpEndSectionsDirective &x) {
- return Pre(static_cast<const parser::OmpDirectiveSpecification &>(x));
- }
- void Post(const parser::OmpEndSectionsDirective &x) {
- Post(static_cast<const parser::OmpDirectiveSpecification &>(x));
- }
bool Pre(const parser::OmpThreadprivateDirective &) {
SkipImplicitTyping(true);
return true;
@@ -1891,20 +1867,6 @@ class OmpVisitor : public virtual DeclarationVisitor {
SkipImplicitTyping(false);
messageHandler().set_currStmtSource(std::nullopt);
}
- bool Pre(const parser::OpenMPDepobjConstruct &x) {
- AddOmpSourceRange(x.source);
- return true;
- }
- void Post(const parser::OpenMPDepobjConstruct &x) {
- messageHandler().set_currStmtSource(std::nullopt);
- }
- bool Pre(const parser::OpenMPAtomicConstruct &x) {
- AddOmpSourceRange(x.source);
- return true;
- }
- void Post(const parser::OpenMPAtomicConstruct &) {
- messageHandler().set_currStmtSource(std::nullopt);
- }
bool Pre(const parser::OmpClause &x) {
if (NeedsScope(x)) {
PushScopeWithSource(Scope::Kind::OtherClause, x.source);
@@ -1937,11 +1899,23 @@ class OmpVisitor : public virtual DeclarationVisitor {
bool Pre(const parser::OpenMPConstruct &x) {
// Indicate that the current directive is not a declarative one.
declaratives_.push_back(nullptr);
+
+ auto name{parser::omp::GetOmpDirectiveName(x)};
+ if (HasDataEnvironment(name.v)) {
+ std::optional<parser::CharBlock> source{parser::GetSource(x)};
+ assert(source.has_value() && "Expecting directive source");
+ PushScopeWithSource(Scope::Kind::OtherConstruct, *source);
+ }
return true;
}
- void Post(const parser::OpenMPConstruct &) {
+ void Post(const parser::OpenMPConstruct &x) {
// Pop the null pointer.
declaratives_.pop_back();
+
+ auto name{parser::omp::GetOmpDirectiveName(x)};
+ if (HasDataEnvironment(name.v)) {
+ PopScope();
+ }
}
private:
@@ -1956,14 +1930,35 @@ class OmpVisitor : public virtual DeclarationVisitor {
std::vector<const parser::OpenMPDeclarativeConstruct *> declaratives_;
};
-bool OmpVisitor::NeedsScope(const parser::OmpBlockConstruct &x) {
- switch (x.BeginDir().DirId()) {
- case llvm::omp::Directive::OMPD_master:
- case llvm::omp::Directive::OMPD_ordered:
- return false;
- default:
- return true;
+bool OmpVisitor::HasDataEnvironment(llvm::omp::Directive dir) {
+ for (auto leaf : llvm::omp::getLeafConstructsOrSelf(dir)) {
+ switch (leaf) {
+ case llvm::omp::Directive::OMPD_distribute: // work-distribution
+ case llvm::omp::Directive::OMPD_do: // work-distribution
+ case llvm::omp::Directive::OMPD_for: // work-distribution
+ case llvm::omp::Directive::OMPD_loop: // work-distribution
+ case llvm::omp::Directive::OMPD_parallel: // team-generating
+ case llvm::omp::Directive::OMPD_scope: // work-distribution
+ case llvm::omp::Directive::OMPD_sections: // work-distribution
+ case llvm::omp::Directive::OMPD_simd:
+ case llvm::omp::Directive::OMPD_single: // work-distribution
+ case llvm::omp::Directive::OMPD_target: // task-generating
+ case llvm::omp::Directive::OMPD_target_data: // task-generating
+ case llvm::omp::Directive::OMPD_target_enter_data: // task-generating
+ case llvm::omp::Directive::OMPD_target_exit_data: // task-generating
+ case llvm::omp::Directive::OMPD_target_update: // task-generating
+ case llvm::omp::Directive::OMPD_task: // task-generating
+ case llvm::omp::Directive::OMPD_taskgroup:
+ case llvm::omp::Directive::OMPD_taskloop: // task-generating
+ case llvm::omp::Directive::OMPD_teams: // team-generating
+ case llvm::omp::Directive::OMPD_workdistribute: // work-distribution
+ case llvm::omp::Directive::OMPD_workshare: // work-distribution
+ return true;
+ default:
+ break;
+ }
}
+ return false;
}
bool OmpVisitor::NeedsScope(const parser::OmpClause &x) {
@@ -1983,19 +1978,6 @@ void OmpVisitor::PushScopeWithSource(
currScope().AddSourceRange(source);
}
-bool OmpVisitor::Pre(const parser::OmpBlockConstruct &x) {
- if (NeedsScope(x)) {
- PushScopeWithSource(Scope::Kind::OtherConstruct, x.source);
- }
- return true;
-}
-
-void OmpVisitor::Post(const parser::OmpBlockConstruct &x) {
- if (NeedsScope(x)) {
- PopScope();
- }
-}
-
void OmpVisitor::Post(const parser::OmpTypeName &x) {
x.declTypeSpec = GetDeclTypeSpec();
}
diff --git a/flang/test/Semantics/OpenMP/affected-loops.f90 b/flang/test/Semantics/OpenMP/affected-loops.f90
index 01273ec4725ec..5cedff1e2d8ed 100644
--- a/flang/test/Semantics/OpenMP/affected-loops.f90
+++ b/flang/test/Semantics/OpenMP/affected-loops.f90
@@ -21,7 +21,6 @@ subroutine f
!CHECK: j size=4 offset=4: ObjectEntity type: INTEGER(4)
!CHECK: k size=4 offset=8: ObjectEntity type: INTEGER(4)
!CHECK: OtherConstruct scope: size=0 alignment=1 sourceRange=98 bytes
+!CHECK: i (OmpPrivate, OmpPreDetermined): HostAssoc => i size=4 offset=0: ObjectEntity type: INTEGER(4)
+!CHECK: j (OmpPrivate, OmpPreDetermined): HostAssoc => j size=4 offset=4: ObjectEntity type: INTEGER(4)
!CHECK: k (OmpPrivate, OmpPreDetermined): HostAssoc => k size=4 offset=8: ObjectEntity type: INTEGER(4)
-!CHECK: OtherConstruct scope: size=0 alignment=1 sourceRange=77 bytes
-!CHECK: i (OmpPrivate, OmpPreDetermined): HostAssoc => i size=4 offset=0: ObjectEntity type: INTEGER(4)
-!CHECK: j (OmpPrivate, OmpPreDetermined): HostAssoc => j size=4 offset=4: ObjectEntity type: INTEGER(4)
>From 2b26f10a4bc9ee3ac62ce6b340780f68c89e7098 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Wed, 20 May 2026 09:46:14 -0500
Subject: [PATCH 2/2] [flang][OpenMP] Remove unnecessary code from OmpVisitor,
NFC
---
flang/lib/Semantics/resolve-names.cpp | 87 +++++++--------------------
1 file changed, 21 insertions(+), 66 deletions(-)
diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index 25d3e5062eb41..fc1537d6279e0 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -1731,22 +1731,12 @@ class OmpVisitor : public virtual DeclarationVisitor {
static bool NeedsScope(const parser::OmpClause &);
- bool Pre(const parser::OmpRequiresDirective &x) {
- AddOmpSourceRange(x.source);
- return true;
- }
bool Pre(const parser::OmpBeginDirective &x) {
return Pre(static_cast<const parser::OmpDirectiveSpecification &>(x));
}
- void Post(const parser::OmpBeginDirective &x) {
- Post(static_cast<const parser::OmpDirectiveSpecification &>(x));
- }
bool Pre(const parser::OmpEndDirective &x) {
return Pre(static_cast<const parser::OmpDirectiveSpecification &>(x));
}
- void Post(const parser::OmpEndDirective &x) {
- Post(static_cast<const parser::OmpDirectiveSpecification &>(x));
- }
void Post(const parser::OmpTypeName &);
bool Pre(const parser::OmpStylizedDeclaration &);
@@ -1768,36 +1758,10 @@ class OmpVisitor : public virtual DeclarationVisitor {
}
}
- bool Pre(const parser::OmpDeclareMapperDirective &x) {
- AddOmpSourceRange(x.source);
- return true;
- }
-
- bool Pre(const parser::OmpDeclareSimdDirective &x) {
- AddOmpSourceRange(x.source);
- return true;
- }
-
- bool Pre(const parser::OmpDeclareVariantDirective &x) {
- AddOmpSourceRange(x.source);
- return true;
- }
-
- bool Pre(const parser::OmpDeclareReductionDirective &x) {
- AddOmpSourceRange(x.source);
- return true;
- }
bool Pre(const parser::OmpMapClause &);
bool Pre(const parser::OmpClause::To &);
bool Pre(const parser::OmpClause::From &);
- bool Pre(const parser::OmpThreadprivateDirective &) {
- SkipImplicitTyping(true);
- return true;
- }
- void Post(const parser::OmpThreadprivateDirective &) {
- SkipImplicitTyping(false);
- }
bool Pre(const parser::OmpDeclareTargetDirective &x) {
auto addObjectName{[&](const parser::OmpObject &object) {
common::visit(
@@ -1841,32 +1805,6 @@ class OmpVisitor : public virtual DeclarationVisitor {
SkipImplicitTyping(true);
return true;
}
- void Post(const parser::OmpDeclareTargetDirective &) {
- SkipImplicitTyping(false);
- }
- bool Pre(const parser::OmpAllocateDirective &x) {
- AddOmpSourceRange(x.source);
- SkipImplicitTyping(true);
- return true;
- }
- void Post(const parser::OmpAllocateDirective &) {
- SkipImplicitTyping(false);
- messageHandler().set_currStmtSource(std::nullopt);
- }
- bool Pre(const parser::OpenMPDeclarativeConstruct &x) {
- AddOmpSourceRange(x.source);
- // Without skipping implicit typing, declarative constructs
- // can implicitly declare variables instead of only using the
- // ones already declared in the Fortran sources.
- SkipImplicitTyping(true);
- declaratives_.push_back(&x);
- return true;
- }
- void Post(const parser::OpenMPDeclarativeConstruct &) {
- declaratives_.pop_back();
- SkipImplicitTyping(false);
- messageHandler().set_currStmtSource(std::nullopt);
- }
bool Pre(const parser::OmpClause &x) {
if (NeedsScope(x)) {
PushScopeWithSource(Scope::Kind::OtherClause, x.source);
@@ -1892,7 +1830,20 @@ class OmpVisitor : public virtual DeclarationVisitor {
}
bool Pre(const parser::OmpDirectiveSpecification &x);
- void Post(const parser::OmpDirectiveSpecification &) {
+
+ bool Pre(const parser::OpenMPDeclarativeConstruct &x) {
+ AddOmpSourceRange(x.source);
+ // Without skipping implicit typing, declarative constructs
+ // can implicitly declare variables instead of only using the
+ // ones already declared in the Fortran sources.
+ SkipImplicitTyping(true);
+ declaratives_.push_back(&x);
+ return true;
+ }
+
+ void Post(const parser::OpenMPDeclarativeConstruct &) {
+ declaratives_.pop_back();
+ SkipImplicitTyping(false);
messageHandler().set_currStmtSource(std::nullopt);
}
@@ -1902,12 +1853,15 @@ class OmpVisitor : public virtual DeclarationVisitor {
auto name{parser::omp::GetOmpDirectiveName(x)};
if (HasDataEnvironment(name.v)) {
- std::optional<parser::CharBlock> source{parser::GetSource(x)};
- assert(source.has_value() && "Expecting directive source");
- PushScopeWithSource(Scope::Kind::OtherConstruct, *source);
+ PushScope(Scope::Kind::OtherConstruct, nullptr);
}
+
+ std::optional<parser::CharBlock> source{parser::GetSource(x)};
+ assert(source.has_value() && "Expecting directive source");
+ AddOmpSourceRange(*source);
return true;
}
+
void Post(const parser::OpenMPConstruct &x) {
// Pop the null pointer.
declaratives_.pop_back();
@@ -1916,6 +1870,7 @@ class OmpVisitor : public virtual DeclarationVisitor {
if (HasDataEnvironment(name.v)) {
PopScope();
}
+ messageHandler().set_currStmtSource(std::nullopt);
}
private:
More information about the flang-commits
mailing list