[flang][OpenMP] Resolve all components of OmpDirectiveSpecification #159946
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
Fully resolve all arguments and clauses in OmpDirectiveSpecification instead of just looking for special cases. Delegate resolution from nodes that inherit from ODS to use the ODS resolution.
llvmbot
added
flang
|
@llvm/pr-subscribers-flang-semantics Author: Krzysztof Parzyszek (kparzysz) ChangesFully resolve all arguments and clauses in OmpDirectiveSpecification instead of just looking for special cases. Delegate resolution from nodes that inherit from ODS to use the ODS resolution. Full diff: https://github.com/llvm/llvm-project/pull/159946.diff 1 Files Affected:
diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index 13edb118a3286..5dd0e1fd5072e 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -1486,34 +1486,16 @@ class OmpVisitor : public virtual DeclarationVisitor {
bool Pre(const parser::OmpBlockConstruct &);
void Post(const parser::OmpBlockConstruct &);
bool Pre(const parser::OmpBeginDirective &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 (x.DirName().v == llvm::omp::Directive::OMPD_critical) {
- for (const parser::OmpArgument &arg : x.Arguments().v) {
- ResolveCriticalName(arg);
- }
- }
- return true;
+ return Pre(static_cast<const parser::OmpDirectiveSpecification &>(x));
}
- void Post(const parser::OmpBeginDirective &) {
- messageHandler().set_currStmtSource(std::nullopt);
+ void Post(const parser::OmpBeginDirective &x) {
+ Post(static_cast<const parser::OmpDirectiveSpecification &>(x));
}
bool Pre(const parser::OmpEndDirective &x) {
- AddOmpSourceRange(x.source);
- // Manually resolve names in CRITICAL directives.
- if (x.DirName().v == llvm::omp::Directive::OMPD_critical) {
- for (const parser::OmpArgument &arg : x.Arguments().v) {
- ResolveCriticalName(arg);
- }
- }
- return true;
+ return Pre(static_cast<const parser::OmpDirectiveSpecification &>(x));
}
- void Post(const parser::OmpEndDirective &) {
- messageHandler().set_currStmtSource(std::nullopt);
+ void Post(const parser::OmpEndDirective &x) {
+ Post(static_cast<const parser::OmpDirectiveSpecification &>(x));
}
bool Pre(const parser::OpenMPLoopConstruct &) {
@@ -1522,8 +1504,16 @@ class OmpVisitor : public virtual DeclarationVisitor {
}
void Post(const parser::OpenMPLoopConstruct &) { PopScope(); }
bool Pre(const parser::OmpBeginLoopDirective &x) {
- AddOmpSourceRange(x.source);
- return true;
+ return Pre(static_cast<const parser::OmpDirectiveSpecification &>(x));
+ }
+ void Post(const parser::OmpBeginLoopDirective &x) {
+ Post(static_cast<const parser::OmpDirectiveSpecification &>(x));
+ }
+ bool Pre(const parser::OmpEndLoopDirective &x) {
+ return Pre(static_cast<const parser::OmpDirectiveSpecification &>(x));
+ }
+ void Post(const parser::OmpEndLoopDirective &x) {
+ Post(static_cast<const parser::OmpDirectiveSpecification &>(x));
}
bool Pre(const parser::OpenMPDeclareMapperConstruct &x) {
@@ -1580,35 +1570,22 @@ class OmpVisitor : public virtual DeclarationVisitor {
}
bool Pre(const parser::OmpMapClause &);
- void Post(const parser::OmpBeginLoopDirective &) {
- messageHandler().set_currStmtSource(std::nullopt);
- }
- bool Pre(const parser::OmpEndLoopDirective &x) {
- AddOmpSourceRange(x.source);
- return true;
- }
- void Post(const parser::OmpEndLoopDirective &) {
- messageHandler().set_currStmtSource(std::nullopt);
- }
-
bool Pre(const parser::OpenMPSectionsConstruct &) {
PushScope(Scope::Kind::OtherConstruct, nullptr);
return true;
}
void Post(const parser::OpenMPSectionsConstruct &) { PopScope(); }
bool Pre(const parser::OmpBeginSectionsDirective &x) {
- AddOmpSourceRange(x.source);
- return true;
+ return Pre(static_cast<const parser::OmpDirectiveSpecification &>(x));
}
- void Post(const parser::OmpBeginSectionsDirective &) {
- messageHandler().set_currStmtSource(std::nullopt);
+ void Post(const parser::OmpBeginSectionsDirective &x) {
+ Post(static_cast<const parser::OmpDirectiveSpecification &>(x));
}
bool Pre(const parser::OmpEndSectionsDirective &x) {
- AddOmpSourceRange(x.source);
- return true;
+ return Pre(static_cast<const parser::OmpDirectiveSpecification &>(x));
}
- void Post(const parser::OmpEndSectionsDirective &) {
- messageHandler().set_currStmtSource(std::nullopt);
+ void Post(const parser::OmpEndSectionsDirective &x) {
+ Post(static_cast<const parser::OmpDirectiveSpecification &>(x));
}
bool Pre(const parser::OpenMPThreadprivate &) {
SkipImplicitTyping(true);
@@ -1710,6 +1687,9 @@ class OmpVisitor : public virtual DeclarationVisitor {
}
}
bool Pre(const parser::OmpDirectiveSpecification &x);
+ void Post(const parser::OmpDirectiveSpecification &) {
+ messageHandler().set_currStmtSource(std::nullopt);
+ }
bool Pre(const parser::OmpTypeSpecifier &x) {
BeginDeclTypeSpec();
@@ -1719,6 +1699,16 @@ class OmpVisitor : public virtual DeclarationVisitor {
EndDeclTypeSpec();
}
+ bool Pre(const parser::OpenMPConstruct &x) {
+ // Indicate that the current directive is not a declarative one.
+ declaratives_.push_back(nullptr);
+ return true;
+ }
+ void Post(const parser::OpenMPConstruct &) {
+ // Pop the null pointer.
+ declaratives_.pop_back();
+ }
+
private:
void ProcessMapperSpecifier(const parser::OmpMapperSpecifier &spec,
const parser::OmpClauseList &clauses);
@@ -1987,29 +1977,34 @@ bool OmpVisitor::Pre(const parser::OmpDirectiveSpecification &x) {
const parser::OmpArgumentList &args{x.Arguments()};
const parser::OmpClauseList &clauses{x.Clauses()};
- switch (x.DirId()) {
- case llvm::omp::Directive::OMPD_declare_mapper:
- if (!args.v.empty()) {
- const parser::OmpArgument &first{args.v.front()};
- if (auto *spec{std::get_if<parser::OmpMapperSpecifier>(&first.u)}) {
- ProcessMapperSpecifier(*spec, clauses);
- }
- }
- break;
- case llvm::omp::Directive::OMPD_declare_reduction:
- if (!args.v.empty()) {
- const parser::OmpArgument &first{args.v.front()};
- if (auto *spec{std::get_if<parser::OmpReductionSpecifier>(&first.u)}) {
- ProcessReductionSpecifier(*spec, clauses, declaratives_.back());
- }
- }
- break;
- default:
- // Default processing.
- Walk(args);
- Walk(clauses);
- break;
+ for (const parser::OmpArgument &arg : args.v) {
+ common::visit( //
+ common::visitors{
+ [&](const parser::OmpMapperSpecifier &spec) {
+ ProcessMapperSpecifier(spec, clauses);
+ },
+ [&](const parser::OmpReductionSpecifier &spec) {
+ ProcessReductionSpecifier(spec, clauses, declaratives_.back());
+ },
+ [&](const parser::OmpLocator &locator) {
+ // 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 (x.DirId() == llvm::omp::Directive::OMPD_critical) {
+ ResolveCriticalName(arg);
+ } else {
+ Walk(locator);
+ }
+ },
+ },
+ arg.u);
}
+
+ Walk(clauses);
+
return false;
}
|
…03-resolve-arguments
This was referenced Sep 22, 2025
This was referenced Sep 22, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fully resolve all arguments and clauses in OmpDirectiveSpecification instead of just looking for special cases. Delegate resolution from nodes that inherit from ODS to use the ODS resolution.