[flang][OpenMP] Use OmpDirectiveSpecification in DECLARE_MAPPER #160169
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.
…03-resolve-arguments
llvmbot
added
flang
Member
|
@llvm/pr-subscribers-flang-semantics @llvm/pr-subscribers-flang-openmp Author: Krzysztof Parzyszek (kparzysz) ChangesFull diff: https://github.com/llvm/llvm-project/pull/160169.diff 11 Files Affected:
diff --git a/flang/include/flang/Parser/openmp-utils.h b/flang/include/flang/Parser/openmp-utils.h
index 98e849eef9bbc..19e23e4d9f62b 100644
--- a/flang/include/flang/Parser/openmp-utils.h
+++ b/flang/include/flang/Parser/openmp-utils.h
@@ -43,7 +43,6 @@ MAKE_CONSTR_ID(OmpErrorDirective, D::OMPD_error);
MAKE_CONSTR_ID(OmpMetadirectiveDirective, D::OMPD_metadirective);
MAKE_CONSTR_ID(OpenMPDeclarativeAllocate, D::OMPD_allocate);
MAKE_CONSTR_ID(OpenMPDeclarativeAssumes, D::OMPD_assumes);
-MAKE_CONSTR_ID(OpenMPDeclareMapperConstruct, D::OMPD_declare_mapper);
MAKE_CONSTR_ID(OpenMPDeclareReductionConstruct, D::OMPD_declare_reduction);
MAKE_CONSTR_ID(OpenMPDeclareSimdConstruct, D::OMPD_declare_simd);
MAKE_CONSTR_ID(OpenMPDeclareTargetConstruct, D::OMPD_declare_target);
@@ -105,7 +104,6 @@ struct DirectiveNameScope {
std::is_same_v<T, OmpMetadirectiveDirective> ||
std::is_same_v<T, OpenMPDeclarativeAllocate> ||
std::is_same_v<T, OpenMPDeclarativeAssumes> ||
- std::is_same_v<T, OpenMPDeclareMapperConstruct> ||
std::is_same_v<T, OpenMPDeclareReductionConstruct> ||
std::is_same_v<T, OpenMPDeclareSimdConstruct> ||
std::is_same_v<T, OpenMPDeclareTargetConstruct> ||
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index bd0debe297916..73f8fbdbbb467 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -4951,9 +4951,9 @@ struct OpenMPDeclareTargetConstruct {
// OMP v5.2: 5.8.8
// declare-mapper -> DECLARE MAPPER ([mapper-name :] type :: var) map-clauses
struct OpenMPDeclareMapperConstruct {
- TUPLE_CLASS_BOILERPLATE(OpenMPDeclareMapperConstruct);
+ WRAPPER_CLASS_BOILERPLATE(
+ OpenMPDeclareMapperConstruct, OmpDirectiveSpecification);
CharBlock source;
- std::tuple<Verbatim, OmpMapperSpecifier, OmpClauseList> t;
};
// ref: 5.2: Section 5.5.11 139-141
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 5681be664d450..c549485565bad 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -3444,15 +3444,17 @@ genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
static void
genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
- const parser::OpenMPDeclareMapperConstruct &declareMapperConstruct) {
- mlir::Location loc = converter.genLocation(declareMapperConstruct.source);
+ const parser::OpenMPDeclareMapperConstruct &construct) {
+ mlir::Location loc = converter.genLocation(construct.source);
fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
+ const parser::OmpArgumentList &args = construct.v.Arguments();
+ assert(args.v.size() == 1 && "Expecting single argument");
lower::StatementContext stmtCtx;
- const auto &spec =
- std::get<parser::OmpMapperSpecifier>(declareMapperConstruct.t);
- const auto &mapperName{std::get<std::string>(spec.t)};
- const auto &varType{std::get<parser::TypeSpec>(spec.t)};
- const auto &varName{std::get<parser::Name>(spec.t)};
+ const auto *spec = std::get_if<parser::OmpMapperSpecifier>(&args.v.front().u);
+ assert(spec && "Expecting mapper specifier");
+ const auto &mapperName{std::get<std::string>(spec->t)};
+ const auto &varType{std::get<parser::TypeSpec>(spec->t)};
+ const auto &varName{std::get<parser::Name>(spec->t)};
assert(varType.declTypeSpec->category() ==
semantics::DeclTypeSpec::Category::TypeDerived &&
"Expected derived type");
@@ -3476,9 +3478,7 @@ genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
// Populate the declareMapper region with the map information.
mlir::omp::DeclareMapperInfoOperands clauseOps;
- const auto *clauseList{
- parser::Unwrap<parser::OmpClauseList>(declareMapperConstruct.t)};
- List<Clause> clauses = makeClauses(*clauseList, semaCtx);
+ List<Clause> clauses = makeClauses(construct.v.Clauses(), semaCtx);
ClauseProcessor cp(converter, semaCtx, clauses);
cp.processMap(loc, stmtCtx, clauseOps);
mlir::omp::DeclareMapperInfoOp::create(firOpBuilder, loc, clauseOps.mapVars);
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index 8ab9905123135..dc638ca9d00fe 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -1756,8 +1756,9 @@ TYPE_PARSER(applyFunction<OmpMapperSpecifier>(ConstructOmpMapperSpecifier,
// OpenMP 5.2: 5.8.8 Declare Mapper Construct
TYPE_PARSER(sourced(construct<OpenMPDeclareMapperConstruct>(
- verbatim("DECLARE MAPPER"_tok) || verbatim("DECLARE_MAPPER"_tok),
- parenthesized(Parser<OmpMapperSpecifier>{}), Parser<OmpClauseList>{})))
+ predicated(Parser<OmpDirectiveName>{},
+ IsDirective(llvm::omp::Directive::OMPD_declare_mapper)) >=
+ Parser<OmpDirectiveSpecification>{})))
TYPE_PARSER(construct<OmpReductionCombiner>(Parser<AssignmentStmt>{}) ||
construct<OmpReductionCombiner>(Parser<FunctionReference>{}))
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index b06a8c1fa4374..01b0e32a1164e 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -2547,21 +2547,10 @@ class UnparseVisitor {
Put("\n");
EndOpenMP();
}
- void Unparse(const OpenMPDeclareMapperConstruct &z) {
+ void Unparse(const OpenMPDeclareMapperConstruct &x) {
BeginOpenMP();
- Word("!$OMP DECLARE MAPPER (");
- const auto &spec{std::get<OmpMapperSpecifier>(z.t)};
- const auto &mapperName{std::get<std::string>(spec.t)};
- if (mapperName.find(llvm::omp::OmpDefaultMapperName) == std::string::npos) {
- Walk(mapperName);
- Put(":");
- }
- Walk(std::get<TypeSpec>(spec.t));
- Put("::");
- Walk(std::get<Name>(spec.t));
- Put(")");
-
- Walk(std::get<OmpClauseList>(z.t));
+ Word("!$OMP ");
+ Walk(x.v);
Put("\n");
EndOpenMP();
}
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index c39daef6b0ea9..17954b5826586 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -629,11 +629,6 @@ template <typename Checker> struct DirectiveSpellingVisitor {
checker_(std::get<parser::Verbatim>(x.t).source, Directive::OMPD_assumes);
return false;
}
- bool Pre(const parser::OpenMPDeclareMapperConstruct &x) {
- checker_(
- std::get<parser::Verbatim>(x.t).source, Directive::OMPD_declare_mapper);
- return false;
- }
bool Pre(const parser::OpenMPDeclareReductionConstruct &x) {
checker_(std::get<parser::Verbatim>(x.t).source,
Directive::OMPD_declare_reduction);
@@ -1595,13 +1590,25 @@ void OmpStructureChecker::Leave(const parser::OmpDeclareTargetWithClause &x) {
}
void OmpStructureChecker::Enter(const parser::OpenMPDeclareMapperConstruct &x) {
- const auto &dir{std::get<parser::Verbatim>(x.t)};
- PushContextAndClauseSets(
- dir.source, llvm::omp::Directive::OMPD_declare_mapper);
- const auto &spec{std::get<parser::OmpMapperSpecifier>(x.t)};
- const auto &type = std::get<parser::TypeSpec>(spec.t);
- if (!std::get_if<parser::DerivedTypeSpec>(&type.u)) {
- context_.Say(dir.source, "Type is not a derived type"_err_en_US);
+ const parser::OmpDirectiveName &dirName{x.v.DirName()};
+ PushContextAndClauseSets(dirName.source, dirName.v);
+
+ const parser::OmpArgumentList &args{x.v.Arguments()};
+ if (args.v.size() != 1) {
+ context_.Say(args.source,
+ "DECLARE_MAPPER directive should have a single argument"_err_en_US);
+ return;
+ }
+
+ const parser::OmpArgument &arg{args.v.front()};
+ if (auto *spec{std::get_if<parser::OmpMapperSpecifier>(&arg.u)}) {
+ const auto &type = std::get<parser::TypeSpec>(spec->t);
+ if (!std::get_if<parser::DerivedTypeSpec>(&type.u)) {
+ context_.Say(arg.source, "Type is not a derived type"_err_en_US);
+ }
+ } else {
+ context_.Say(arg.source,
+ "The argument to the DECLARE_MAPPER directive should be a mapper-specifier"_err_en_US);
}
}
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 2d1bec9968593..48cd0e54a155a 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -2331,7 +2331,8 @@ bool OmpAttributeVisitor::Pre(const parser::OpenMPDeclareTargetConstruct &x) {
}
bool OmpAttributeVisitor::Pre(const parser::OpenMPDeclareMapperConstruct &x) {
- PushContext(x.source, llvm::omp::Directive::OMPD_declare_mapper);
+ const parser::OmpDirectiveName &dirName{x.v.DirName()};
+ PushContext(dirName.source, dirName.v);
return true;
}
diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index 358bec20f243c..b6d2ba7c4344d 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -1518,9 +1518,7 @@ class OmpVisitor : public virtual DeclarationVisitor {
bool Pre(const parser::OpenMPDeclareMapperConstruct &x) {
AddOmpSourceRange(x.source);
- ProcessMapperSpecifier(std::get<parser::OmpMapperSpecifier>(x.t),
- std::get<parser::OmpClauseList>(x.t));
- return false;
+ return true;
}
bool Pre(const parser::OpenMPDeclareSimdConstruct &x) {
@@ -1686,6 +1684,7 @@ class OmpVisitor : public virtual DeclarationVisitor {
PopScope();
}
}
+ bool Pre(const parser::OmpMapperSpecifier &x) { return false; }
bool Pre(const parser::OmpDirectiveSpecification &x);
void Post(const parser::OmpDirectiveSpecification &) {
messageHandler().set_currStmtSource(std::nullopt);
diff --git a/flang/test/Parser/OpenMP/declare-mapper-unparse.f90 b/flang/test/Parser/OpenMP/declare-mapper-unparse.f90
index 30d75d02736f3..b53bf5ce10557 100644
--- a/flang/test/Parser/OpenMP/declare-mapper-unparse.f90
+++ b/flang/test/Parser/OpenMP/declare-mapper-unparse.f90
@@ -9,7 +9,7 @@ program main
end type ty
-!CHECK: !$OMP DECLARE MAPPER (mymapper:ty::mapped) MAP(mapped,mapped%x)
+!CHECK: !$OMP DECLARE MAPPER(mymapper:ty::mapped) MAP(mapped,mapped%x)
!$omp declare mapper(mymapper : ty :: mapped) map(mapped, mapped%x)
!PARSE-TREE: OpenMPDeclareMapperConstruct
@@ -24,7 +24,7 @@ program main
!PARSE-TREE: DataRef -> Name = 'mapped'
!PARSE-TREE: Name = 'x'
-!CHECK: !$OMP DECLARE MAPPER (ty::mapped) MAP(mapped,mapped%x)
+!CHECK: !$OMP DECLARE MAPPER(ty::mapped) MAP(mapped,mapped%x)
!$omp declare mapper(ty :: mapped) map(mapped, mapped%x)
!PARSE-TREE: OpenMPDeclareMapperConstruct
diff --git a/flang/test/Parser/OpenMP/openmp6-directive-spellings.f90 b/flang/test/Parser/OpenMP/openmp6-directive-spellings.f90
index c2498c878f559..47237de2d5aff 100644
--- a/flang/test/Parser/OpenMP/openmp6-directive-spellings.f90
+++ b/flang/test/Parser/OpenMP/openmp6-directive-spellings.f90
@@ -51,12 +51,12 @@ subroutine f01
!UNPARSE: TYPE :: t
!UNPARSE: INTEGER :: x
!UNPARSE: END TYPE
-!UNPARSE: !$OMP DECLARE MAPPER (t::v) MAP(v%x)
+!UNPARSE: !$OMP DECLARE_MAPPER(t::v) MAP(v%x)
!UNPARSE: END SUBROUTINE
-!PARSE-TREE: DeclarationConstruct -> SpecificationConstruct -> OpenMPDeclarativeConstruct -> OpenMPDeclareMapperConstruct
-!PARSE-TREE: | Verbatim
-!PARSE-TREE: | OmpMapperSpecifier
+!PARSE-TREE: DeclarationConstruct -> SpecificationConstruct -> OpenMPDeclarativeConstruct -> OpenMPDeclareMapperConstruct -> OmpDirectiveSpecification
+!PARSE-TREE: | OmpDirectiveName -> llvm::omp::Directive = declare mapper
+!PARSE-TREE: | OmpArgumentList -> OmpArgument -> OmpMapperSpecifier
!PARSE-TREE: | | string = 't.omp.default.mapper'
!PARSE-TREE: | | TypeSpec -> DerivedTypeSpec
!PARSE-TREE: | | | Name = 't'
@@ -66,6 +66,7 @@ subroutine f01
!PARSE-TREE: | | | DataRef -> Name = 'v'
!PARSE-TREE: | | | Name = 'x'
!PARSE-TREE: | | bool = 'true'
+!PARSE-TREE: | Flags = None
subroutine f02
type :: t
diff --git a/flang/test/Semantics/OpenMP/declare-mapper04.f90 b/flang/test/Semantics/OpenMP/declare-mapper04.f90
new file mode 100644
index 0000000000000..2f45e230c3513
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/declare-mapper04.f90
@@ -0,0 +1,18 @@
+! RUN: %python %S/../test_errors.py %s %flang -fopenmp -fopenmp-version=60
+
+type :: t1
+ integer :: y
+end type
+
+type :: t2
+ integer :: y
+end type
+
+!ERROR: DECLARE_MAPPER directive should have a single argument
+!$omp declare mapper(m1:t1::x, m2:t2::x) map(x, x%y)
+
+integer :: x(10)
+!ERROR: The argument to the DECLARE_MAPPER directive should be a mapper-specifier
+!$omp declare mapper(x) map(to: x)
+
+end
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
If it happens, it's a bug to be fixed.
Contributor
Author
This was referenced Sep 22, 2025
Base automatically changed from
users/kparzysz/r03-resolve-arguments
to
main
September 23, 2025 13:32
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.
No description provided.