Skip to content

Conversation

kparzysz
Copy link
Contributor

This was checked in the visitor for OmpDirectiveSpecification, and is not necessary anymore: the early exit (in case of not being inside of a METADIRECTIVE) performs the same actions as the code that was skipped, except it does so through a different sequence of function calls. The net result ends up being the same in either case.

The processing of the mapper and reduction specifiers inside of OmpDirectiveSpecification is necesary for the declare directives on WHEN/OTHERWISE clauses, so it's the early exit that needs to be removed. In fact, when the DECLARE_MAPPER/REDUCTION use OmpDirectiveSpecification, this processing will automatically take over the handling of the contained specifiers.

Instead of having a variant with specific AST nodes that can contain
a reduction specifier, simply store the OpenMPDeclarativeConstruct.
It is used to emit the source code directive when generating a module
file, and unparsing the top-level AST node will work just fine.
This was checked in the visitor for OmpDirectiveSpecification, and is
not necessary anymore: the early exit (in case of not being inside of
a METADIRECTIVE) performs the same actions as the code that was skipped,
except it does so through a different sequence of function calls. The
net result ends up being the same in either case.

The processing of the mapper and reduction specifiers inside of
OmpDirectiveSpecification is necesary for the declare directives on
WHEN/OTHERWISE clauses, so it's the early exit that needs to be removed.
In fact, when the DECLARE_MAPPER/REDUCTION use OmpDirectiveSpecification,
this processing will automatically take over the handling of the contained
specifiers.
@kparzysz kparzysz requested review from tblah and Stylie777 September 20, 2025 21:07
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:semantics labels Sep 20, 2025
@llvmbot
Copy link
Member

llvmbot commented Sep 20, 2025

@llvm/pr-subscribers-flang-semantics

Author: Krzysztof Parzyszek (kparzysz)

Changes

This was checked in the visitor for OmpDirectiveSpecification, and is not necessary anymore: the early exit (in case of not being inside of a METADIRECTIVE) performs the same actions as the code that was skipped, except it does so through a different sequence of function calls. The net result ends up being the same in either case.

The processing of the mapper and reduction specifiers inside of OmpDirectiveSpecification is necesary for the declare directives on WHEN/OTHERWISE clauses, so it's the early exit that needs to be removed. In fact, when the DECLARE_MAPPER/REDUCTION use OmpDirectiveSpecification, this processing will automatically take over the handling of the contained specifiers.


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

1 Files Affected:

  • (modified) flang/lib/Semantics/resolve-names.cpp (+15-31)
diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index 69b8a45e6ceaa..13edb118a3286 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -1479,14 +1479,6 @@ class OmpVisitor : public virtual DeclarationVisitor {
   static bool NeedsScope(const parser::OmpBlockConstruct &);
   static bool NeedsScope(const parser::OmpClause &);
 
-  bool Pre(const parser::OmpMetadirectiveDirective &x) { //
-    ++metaLevel_;
-    return true;
-  }
-  void Post(const parser::OmpMetadirectiveDirective &) { //
-    --metaLevel_;
-  }
-
   bool Pre(const parser::OpenMPRequiresConstruct &x) {
     AddOmpSourceRange(x.source);
     return true;
@@ -1579,10 +1571,11 @@ class OmpVisitor : public virtual DeclarationVisitor {
 
   bool Pre(const parser::OpenMPDeclareReductionConstruct &x) {
     AddOmpSourceRange(x.source);
+    parser::OmpClauseList empty(std::list<parser::OmpClause>{});
+    auto &maybeClauses{std::get<std::optional<parser::OmpClauseList>>(x.t)};
     ProcessReductionSpecifier(
         std::get<Indirection<parser::OmpReductionSpecifier>>(x.t).value(),
-        std::get<std::optional<parser::OmpClauseList>>(x.t),
-        declaratives_.back());
+        maybeClauses ? *maybeClauses : empty, declaratives_.back());
     return false;
   }
   bool Pre(const parser::OmpMapClause &);
@@ -1730,12 +1723,11 @@ class OmpVisitor : public virtual DeclarationVisitor {
   void ProcessMapperSpecifier(const parser::OmpMapperSpecifier &spec,
       const parser::OmpClauseList &clauses);
   void ProcessReductionSpecifier(const parser::OmpReductionSpecifier &spec,
-      const std::optional<parser::OmpClauseList> &clauses,
+      const parser::OmpClauseList &clauses,
       const parser::OpenMPDeclarativeConstruct *wholeConstruct);
 
   void ResolveCriticalName(const parser::OmpArgument &arg);
 
-  int metaLevel_{0};
   std::vector<const parser::OpenMPDeclarativeConstruct *> declaratives_;
 };
 
@@ -1863,7 +1855,7 @@ std::string MangleDefinedOperator(const parser::CharBlock &name) {
 
 void OmpVisitor::ProcessReductionSpecifier(
     const parser::OmpReductionSpecifier &spec,
-    const std::optional<parser::OmpClauseList> &clauses,
+    const parser::OmpClauseList &clauses,
     const parser::OpenMPDeclarativeConstruct *construct) {
   const parser::Name *name{nullptr};
   parser::CharBlock mangledName;
@@ -1991,39 +1983,31 @@ void OmpVisitor::ResolveCriticalName(const parser::OmpArgument &arg) {
 
 bool OmpVisitor::Pre(const parser::OmpDirectiveSpecification &x) {
   AddOmpSourceRange(x.source);
-  if (metaLevel_ == 0) {
-    // Not in METADIRECTIVE.
-    return true;
-  }
 
-  // If OmpDirectiveSpecification (which contains clauses) is a part of
-  // METADIRECTIVE, some semantic checks may not be applicable.
-  // Disable the semantic analysis for it in such cases to allow the compiler
-  // to parse METADIRECTIVE without flagging errors.
-  auto &maybeArgs{std::get<std::optional<parser::OmpArgumentList>>(x.t)};
-  auto &maybeClauses{std::get<std::optional<parser::OmpClauseList>>(x.t)};
+  const parser::OmpArgumentList &args{x.Arguments()};
+  const parser::OmpClauseList &clauses{x.Clauses()};
 
   switch (x.DirId()) {
   case llvm::omp::Directive::OMPD_declare_mapper:
-    if (maybeArgs && maybeClauses) {
-      const parser::OmpArgument &first{maybeArgs->v.front()};
+    if (!args.v.empty()) {
+      const parser::OmpArgument &first{args.v.front()};
       if (auto *spec{std::get_if<parser::OmpMapperSpecifier>(&first.u)}) {
-        ProcessMapperSpecifier(*spec, *maybeClauses);
+        ProcessMapperSpecifier(*spec, clauses);
       }
     }
     break;
   case llvm::omp::Directive::OMPD_declare_reduction:
-    if (maybeArgs && maybeClauses) {
-      const parser::OmpArgument &first{maybeArgs->v.front()};
+    if (!args.v.empty()) {
+      const parser::OmpArgument &first{args.v.front()};
       if (auto *spec{std::get_if<parser::OmpReductionSpecifier>(&first.u)}) {
-        ProcessReductionSpecifier(*spec, maybeClauses, declaratives_.back());
+        ProcessReductionSpecifier(*spec, clauses, declaratives_.back());
       }
     }
     break;
   default:
     // Default processing.
-    Walk(maybeArgs);
-    Walk(maybeClauses);
+    Walk(args);
+    Walk(clauses);
     break;
   }
   return false;

@kparzysz
Copy link
Contributor Author

kparzysz commented Sep 22, 2025

Copy link
Contributor

@tblah tblah left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

Base automatically changed from users/kparzysz/r01-declare-red to main September 22, 2025 18:18
@kparzysz kparzysz merged commit fc12467 into main Sep 22, 2025
9 checks passed
@kparzysz kparzysz deleted the users/kparzysz/r02-metadirective branch September 22, 2025 18:52
SeongjaeP pushed a commit to SeongjaeP/llvm-project that referenced this pull request Sep 23, 2025
…lvm#159945)

This was checked in the visitor for OmpDirectiveSpecification, and is
not necessary anymore: the early exit (in case of not being inside of a
METADIRECTIVE) performs the same actions as the code that was skipped,
except it does so through a different sequence of function calls. The
net result ends up being the same in either case.

The processing of the mapper and reduction specifiers inside of
OmpDirectiveSpecification is necesary for the declare directives on
WHEN/OTHERWISE clauses, so it's the early exit that needs to be removed.
In fact, when the DECLARE_MAPPER/REDUCTION use
OmpDirectiveSpecification, this processing will automatically take over
the handling of the contained specifiers.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:semantics flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants