Skip to content

[flang][OpenMP] Use GetOmpDirectiveName to find directive source loca… #150955

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

Merged
merged 5 commits into from
Jul 31, 2025

Conversation

kparzysz
Copy link
Contributor

…tion

kparzysz added 4 commits July 26, 2025 09:47
It was an alias for OmpDirectiveName, which could cause confusion in
parse-tree visitors: a visitor for OmpDirectiveNameModifier could be
executed for an OmpDirectiveName node, leading to unexpected results.
The OpenMPSectionConstruct corresponds to `!$omp section` directive, but
there is nothing in the AST node that stores the directive information.
Even though the only possibility (at the moment) is "section" without any
clauses, for improved generality it is helpful to have that information
anyway.
@kparzysz kparzysz requested review from tblah and luporl July 28, 2025 14:08
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir labels Jul 28, 2025
@llvmbot
Copy link
Member

llvmbot commented Jul 28, 2025

@llvm/pr-subscribers-flang-fir-hlfir

Author: Krzysztof Parzyszek (kparzysz)

Changes

…tion


Full diff: https://github.com/llvm/llvm-project/pull/150955.diff

1 Files Affected:

  • (modified) flang/lib/Lower/OpenMP/DataSharingProcessor.cpp (+13-39)
diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
index 2ac4d9548b65b..2c0cbb2b6168f 100644
--- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
@@ -389,42 +389,16 @@ void DataSharingProcessor::insertLastPrivateCompare(mlir::Operation *op) {
   }
 }
 
-static const parser::CharBlock *
-getSource(const semantics::SemanticsContext &semaCtx,
-          const lower::pft::Evaluation &eval) {
-  const parser::CharBlock *source = nullptr;
-
-  auto ompConsVisit = [&](const parser::OpenMPConstruct &x) {
-    std::visit(
-        common::visitors{
-            [&](const parser::OpenMPSectionsConstruct &x) {
-              source = &std::get<0>(x.t).source;
-            },
-            [&](const parser::OpenMPLoopConstruct &x) {
-              source = &std::get<0>(x.t).source;
-            },
-            [&](const parser::OpenMPBlockConstruct &x) {
-              source = &std::get<0>(x.t).source;
-            },
-            [&](const parser::OpenMPCriticalConstruct &x) {
-              source = &std::get<0>(x.t).source;
-            },
-            [&](const parser::OpenMPAtomicConstruct &x) {
-              source = &std::get<parser::OmpDirectiveSpecification>(x.t).source;
-            },
-            [&](const auto &x) { source = &x.source; },
-        },
-        x.u);
-  };
-
-  eval.visit(common::visitors{
-      [&](const parser::OpenMPConstruct &x) { ompConsVisit(x); },
-      [&](const parser::OpenMPDeclarativeConstruct &x) { source = &x.source; },
-      [&](const parser::OmpEndLoopDirective &x) { source = &x.source; },
-      [&](const auto &x) {},
+static parser::CharBlock getSource(const semantics::SemanticsContext &semaCtx,
+                                   const lower::pft::Evaluation &eval) {
+  return eval.visit(common::visitors{
+      [&](const parser::OpenMPConstruct &x) {
+        return parser::omp::GetOmpDirectiveName(x).source;
+      },
+      [&](const parser::OpenMPDeclarativeConstruct &x) { return x.source; },
+      [&](const parser::OmpEndLoopDirective &x) { return x.source; },
+      [&](const auto &x) { return parser::CharBlock{}; },
   });
-
-  return source;
 }
 
 static void collectPrivatizingConstructs(
@@ -518,11 +492,11 @@ void DataSharingProcessor::collectSymbols(
         for (const semantics::Scope &child : scope->children())
           collectScopes(&child);
       };
-  const parser::CharBlock *source =
-      clauses.empty() ? getSource(semaCtx, eval) : &clauses.front().source;
+  parser::CharBlock source =
+      clauses.empty() ? getSource(semaCtx, eval) : clauses.front().source;
   const semantics::Scope *curScope = nullptr;
-  if (source && !source->empty()) {
-    curScope = &semaCtx.FindScope(*source);
+  if (!source.empty()) {
+    curScope = &semaCtx.FindScope(source);
     collectScopes(curScope);
   }
   // Collect all symbols referenced in the evaluation being processed,

Copy link
Contributor

@luporl luporl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Base automatically changed from users/kparzysz/a02-section-construct-name to main July 31, 2025 12:51
@kparzysz kparzysz merged commit f4972a2 into main Jul 31, 2025
9 checks passed
@kparzysz kparzysz deleted the users/kparzysz/a03-getsource-dsa branch July 31, 2025 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:fir-hlfir flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants