https://github.com/kparzysz updated https://github.com/llvm/llvm-project/pull/160169
>From db95ffd7044e5155c7a5ec2a0022a928ee261a55 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek <[email protected]> Date: Fri, 19 Sep 2025 08:52:08 -0500 Subject: [PATCH 1/4] [flang][OpenMP] Use OmpDirectiveSpecification in DECLARE_MAPPER --- flang/include/flang/Parser/openmp-utils.h | 2 -- flang/include/flang/Parser/parse-tree.h | 4 +-- flang/lib/Lower/OpenMP/OpenMP.cpp | 20 ++++++------ flang/lib/Parser/openmp-parsers.cpp | 5 +-- flang/lib/Parser/unparse.cpp | 17 ++-------- flang/lib/Semantics/check-omp-structure.cpp | 31 ++++++++++++------- flang/lib/Semantics/resolve-directives.cpp | 3 +- flang/lib/Semantics/resolve-names.cpp | 5 ++- .../Parser/OpenMP/declare-mapper-unparse.f90 | 4 +-- .../OpenMP/openmp6-directive-spellings.f90 | 9 +++--- .../Semantics/OpenMP/declare-mapper04.f90 | 18 +++++++++++ 11 files changed, 66 insertions(+), 52 deletions(-) create mode 100644 flang/test/Semantics/OpenMP/declare-mapper04.f90 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 >From 7e3d8a74746878c864ba7821089a6dd28b053824 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek <[email protected]> Date: Mon, 22 Sep 2025 13:36:17 -0500 Subject: [PATCH 2/4] format --- flang/lib/Lower/OpenMP/OpenMP.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp index c549485565bad..d2e865b3e1d0c 100644 --- a/flang/lib/Lower/OpenMP/OpenMP.cpp +++ b/flang/lib/Lower/OpenMP/OpenMP.cpp @@ -3441,10 +3441,10 @@ genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable, TODO(converter.getCurrentLocation(), "OpenMPDeclareSimdConstruct"); } -static void -genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable, - semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval, - const parser::OpenMPDeclareMapperConstruct &construct) { +static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable, + semantics::SemanticsContext &semaCtx, + lower::pft::Evaluation &eval, + const parser::OpenMPDeclareMapperConstruct &construct) { mlir::Location loc = converter.genLocation(construct.source); fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder(); const parser::OmpArgumentList &args = construct.v.Arguments(); >From 3947c8326f562db035cbcb964d533c9077e552f9 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek <[email protected]> Date: Mon, 22 Sep 2025 14:49:15 -0500 Subject: [PATCH 3/4] Abort when walk reaches OmpMapperSpecifier If it happens, it's a bug to be fixed. --- flang/lib/Semantics/resolve-names.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp index b6d2ba7c4344d..69ca86ec65242 100644 --- a/flang/lib/Semantics/resolve-names.cpp +++ b/flang/lib/Semantics/resolve-names.cpp @@ -1684,7 +1684,11 @@ class OmpVisitor : public virtual DeclarationVisitor { PopScope(); } } - bool Pre(const parser::OmpMapperSpecifier &x) { return false; } + bool Pre(const parser::OmpMapperSpecifier &x) { + // OmpMapperSpecifier is handled explicitly, and the Walk infrastructure + // should not reach the point where it calls this function. + llvm_unreachable("This function should not be reached by 'Walk'"); + } bool Pre(const parser::OmpDirectiveSpecification &x); void Post(const parser::OmpDirectiveSpecification &) { messageHandler().set_currStmtSource(std::nullopt); >From 7fe518ccc679cc1ed97a5ca8a0da9c2da5a60177 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek <[email protected]> Date: Mon, 22 Sep 2025 14:58:08 -0500 Subject: [PATCH 4/4] Better wording --- flang/lib/Semantics/resolve-names.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp index 69ca86ec65242..86ada0f8cdc85 100644 --- a/flang/lib/Semantics/resolve-names.cpp +++ b/flang/lib/Semantics/resolve-names.cpp @@ -1685,9 +1685,9 @@ class OmpVisitor : public virtual DeclarationVisitor { } } bool Pre(const parser::OmpMapperSpecifier &x) { - // OmpMapperSpecifier is handled explicitly, and the Walk infrastructure - // should not reach the point where it calls this function. - llvm_unreachable("This function should not be reached by 'Walk'"); + // OmpMapperSpecifier is handled explicitly, and the AST traversal + // should not reach a point where it calls this function. + llvm_unreachable("This function should not be reached by AST traversal"); } bool Pre(const parser::OmpDirectiveSpecification &x); void Post(const parser::OmpDirectiveSpecification &) { _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
