[flang-commits] [flang] [flang][OpenMP] Limit scope creation to constructs with data environment (PR #198780)

Krzysztof Parzyszek via flang-commits flang-commits at lists.llvm.org
Thu May 21 07:15:28 PDT 2026


https://github.com/kparzysz updated https://github.com/llvm/llvm-project/pull/198780

>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/4] [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 570b6e58b8840db5cca95356a684ff1d058b76e4 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Wed, 20 May 2026 15:14:51 -0500
Subject: [PATCH 2/4] Update directive list

Add dispatch - it accepts privatizing clause: is_device_ptr.
Remove workshare, workdistribute - they execute in implicit tasks.
---
 flang/lib/Semantics/resolve-names.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index 25d3e5062eb41..539129740e724 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -1933,6 +1933,7 @@ class OmpVisitor : public virtual DeclarationVisitor {
 bool OmpVisitor::HasDataEnvironment(llvm::omp::Directive dir) {
   for (auto leaf : llvm::omp::getLeafConstructsOrSelf(dir)) {
     switch (leaf) {
+    case llvm::omp::Directive::OMPD_dispatch:
     case llvm::omp::Directive::OMPD_distribute: // work-distribution
     case llvm::omp::Directive::OMPD_do: // work-distribution
     case llvm::omp::Directive::OMPD_for: // work-distribution
@@ -1951,8 +1952,6 @@ bool OmpVisitor::HasDataEnvironment(llvm::omp::Directive dir) {
     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;

>From 157413dc4ddac8b5c35b723cef2dd4c136e90e2c Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Wed, 20 May 2026 15:33:18 -0500
Subject: [PATCH 3/4] Add taskgraph

"taskgraph" is explicitly mentioned in the data-sharing rules in the spec.
---
 flang/lib/Semantics/resolve-names.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index 539129740e724..3587e000994ae 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -1943,6 +1943,7 @@ bool OmpVisitor::HasDataEnvironment(llvm::omp::Directive dir) {
     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_taskgraph:
     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

>From 0b5f4b6384f7c17588be105d0809e69e84b7fad3 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Thu, 21 May 2026 09:14:22 -0500
Subject: [PATCH 4/4] Move the function to utils

It's going to be reused in resolve-directives.
---
 flang/include/flang/Semantics/openmp-utils.h |  2 ++
 flang/lib/Semantics/openmp-utils.cpp         | 31 ++++++++++++++++
 flang/lib/Semantics/resolve-names.cpp        | 37 ++------------------
 3 files changed, 35 insertions(+), 35 deletions(-)

diff --git a/flang/include/flang/Semantics/openmp-utils.h b/flang/include/flang/Semantics/openmp-utils.h
index aab63df2e6e14..ab7a09e4ca721 100644
--- a/flang/include/flang/Semantics/openmp-utils.h
+++ b/flang/include/flang/Semantics/openmp-utils.h
@@ -138,6 +138,8 @@ bool IsPointerAssignment(const evaluate::Assignment &x);
 MaybeExpr MakeEvaluateExpr(const parser::OmpStylizedInstance &inp);
 
 bool IsLoopTransforming(llvm::omp::Directive dir);
+bool HasDataEnvironment(llvm::omp::Directive dir);
+
 bool IsFullUnroll(const parser::OmpDirectiveSpecification &spec);
 
 inline bool IsDoConcurrentLegal(unsigned version) {
diff --git a/flang/lib/Semantics/openmp-utils.cpp b/flang/lib/Semantics/openmp-utils.cpp
index e837f8dca19f6..b615900672042 100644
--- a/flang/lib/Semantics/openmp-utils.cpp
+++ b/flang/lib/Semantics/openmp-utils.cpp
@@ -561,6 +561,37 @@ bool IsLoopTransforming(llvm::omp::Directive dir) {
   }
 }
 
+bool HasDataEnvironment(llvm::omp::Directive dir) {
+  for (auto leaf : llvm::omp::getLeafConstructsOrSelf(dir)) {
+    switch (leaf) {
+    case llvm::omp::Directive::OMPD_dispatch:
+    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_taskgraph:
+    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
+      return true;
+    default:
+      break;
+    }
+  }
+  return false;
+}
+
 bool IsFullUnroll(const parser::OmpDirectiveSpecification &spec) {
   if (spec.DirId() == llvm::omp::Directive::OMPD_unroll) {
     return !parser::omp::FindClause(spec, llvm::omp::Clause::OMPC_partial);
diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index 3587e000994ae..4ee6302f939e3 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -1723,8 +1723,6 @@ 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);
@@ -1901,7 +1899,7 @@ class OmpVisitor : public virtual DeclarationVisitor {
     declaratives_.push_back(nullptr);
 
     auto name{parser::omp::GetOmpDirectiveName(x)};
-    if (HasDataEnvironment(name.v)) {
+    if (omp::HasDataEnvironment(name.v)) {
       std::optional<parser::CharBlock> source{parser::GetSource(x)};
       assert(source.has_value() && "Expecting directive source");
       PushScopeWithSource(Scope::Kind::OtherConstruct, *source);
@@ -1913,7 +1911,7 @@ class OmpVisitor : public virtual DeclarationVisitor {
     declaratives_.pop_back();
 
     auto name{parser::omp::GetOmpDirectiveName(x)};
-    if (HasDataEnvironment(name.v)) {
+    if (omp::HasDataEnvironment(name.v)) {
       PopScope();
     }
   }
@@ -1930,37 +1928,6 @@ class OmpVisitor : public virtual DeclarationVisitor {
   std::vector<const parser::OpenMPDeclarativeConstruct *> declaratives_;
 };
 
-bool OmpVisitor::HasDataEnvironment(llvm::omp::Directive dir) {
-  for (auto leaf : llvm::omp::getLeafConstructsOrSelf(dir)) {
-    switch (leaf) {
-    case llvm::omp::Directive::OMPD_dispatch:
-    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_taskgraph:
-    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
-      return true;
-    default:
-      break;
-    }
-  }
-  return false;
-}
-
 bool OmpVisitor::NeedsScope(const parser::OmpClause &x) {
   // Iterators contain declarations, whose scope extends until the end
   // the clause.



More information about the flang-commits mailing list