diff --git a/flang/include/flang/Parser/openmp-utils.h b/flang/include/flang/Parser/openmp-utils.h index 2e4fa4093b87c..34eb6ac3436bc 100644 --- a/flang/include/flang/Parser/openmp-utils.h +++ b/flang/include/flang/Parser/openmp-utils.h @@ -41,7 +41,6 @@ struct ConstructId { MAKE_CONSTR_ID(OmpDeclareVariantDirective, D::OMPD_declare_variant); 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); @@ -96,7 +95,6 @@ struct DirectiveNameScope { } else if constexpr (std::is_same_v || std::is_same_v || std::is_same_v || - std::is_same_v || std::is_same_v || std::is_same_v || std::is_same_v || diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h index 516dd298f8beb..8d2d085a749f5 100644 --- a/flang/include/flang/Parser/parse-tree.h +++ b/flang/include/flang/Parser/parse-tree.h @@ -4953,9 +4953,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 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..d2e865b3e1d0c 100644 --- a/flang/lib/Lower/OpenMP/OpenMP.cpp +++ b/flang/lib/Lower/OpenMP/OpenMP.cpp @@ -3441,18 +3441,20 @@ 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 &declareMapperConstruct) { - mlir::Location loc = converter.genLocation(declareMapperConstruct.source); +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(); + assert(args.v.size() == 1 && "Expecting single argument"); lower::StatementContext stmtCtx; - const auto &spec = - std::get(declareMapperConstruct.t); - const auto &mapperName{std::get(spec.t)}; - const auto &varType{std::get(spec.t)}; - const auto &varName{std::get(spec.t)}; + const auto *spec = std::get_if(&args.v.front().u); + assert(spec && "Expecting mapper specifier"); + const auto &mapperName{std::get(spec->t)}; + const auto &varType{std::get(spec->t)}; + const auto &varName{std::get(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(declareMapperConstruct.t)}; - List clauses = makeClauses(*clauseList, semaCtx); + List 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 fbdb2a2faa715..e12ed06520bf1 100644 --- a/flang/lib/Parser/openmp-parsers.cpp +++ b/flang/lib/Parser/openmp-parsers.cpp @@ -1764,8 +1764,9 @@ TYPE_PARSER(applyFunction(ConstructOmpMapperSpecifier, // OpenMP 5.2: 5.8.8 Declare Mapper Construct TYPE_PARSER(sourced(construct( - verbatim("DECLARE MAPPER"_tok) || verbatim("DECLARE_MAPPER"_tok), - parenthesized(Parser{}), Parser{}))) + predicated(Parser{}, + IsDirective(llvm::omp::Directive::OMPD_declare_mapper)) >= + Parser{}))) TYPE_PARSER(construct(Parser{}) || construct(Parser{})) diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp index cf7915886be09..77ed7951496c1 100644 --- a/flang/lib/Parser/unparse.cpp +++ b/flang/lib/Parser/unparse.cpp @@ -2559,21 +2559,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(z.t)}; - const auto &mapperName{std::get(spec.t)}; - if (mapperName.find(llvm::omp::OmpDefaultMapperName) == std::string::npos) { - Walk(mapperName); - Put(":"); - } - Walk(std::get(spec.t)); - Put("::"); - Walk(std::get(spec.t)); - Put(")"); - - Walk(std::get(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 ae14afdd9962d..0cbcbc3a8f50e 100644 --- a/flang/lib/Semantics/check-omp-structure.cpp +++ b/flang/lib/Semantics/check-omp-structure.cpp @@ -624,11 +624,6 @@ template struct DirectiveSpellingVisitor { checker_(std::get(x.t).source, Directive::OMPD_assumes); return false; } - bool Pre(const parser::OpenMPDeclareMapperConstruct &x) { - checker_( - std::get(x.t).source, Directive::OMPD_declare_mapper); - return false; - } bool Pre(const parser::OpenMPDeclareReductionConstruct &x) { checker_(std::get(x.t).source, Directive::OMPD_declare_reduction); @@ -1603,13 +1598,25 @@ void OmpStructureChecker::Leave(const parser::OmpDeclareTargetWithClause &x) { } void OmpStructureChecker::Enter(const parser::OpenMPDeclareMapperConstruct &x) { - const auto &dir{std::get(x.t)}; - PushContextAndClauseSets( - dir.source, llvm::omp::Directive::OMPD_declare_mapper); - const auto &spec{std::get(x.t)}; - const auto &type = std::get(spec.t); - if (!std::get_if(&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(&arg.u)}) { + const auto &type = std::get(spec->t); + if (!std::get_if(&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 1f8d9285c1c4b..f1f78620532f5 100644 --- a/flang/lib/Semantics/resolve-directives.cpp +++ b/flang/lib/Semantics/resolve-directives.cpp @@ -2346,7 +2346,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 396025ff0a007..f665f9c8bf5c0 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(x)); } - void Post(const parser::OmpBeginDirective &) { - messageHandler().set_currStmtSource(std::nullopt); + void Post(const parser::OmpBeginDirective &x) { + Post(static_cast(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(x)); } - void Post(const parser::OmpEndDirective &) { - messageHandler().set_currStmtSource(std::nullopt); + void Post(const parser::OmpEndDirective &x) { + Post(static_cast(x)); } bool Pre(const parser::OpenMPLoopConstruct &) { @@ -1522,15 +1504,21 @@ 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(x)); + } + void Post(const parser::OmpBeginLoopDirective &x) { + Post(static_cast(x)); + } + bool Pre(const parser::OmpEndLoopDirective &x) { + return Pre(static_cast(x)); + } + void Post(const parser::OmpEndLoopDirective &x) { + Post(static_cast(x)); } bool Pre(const parser::OpenMPDeclareMapperConstruct &x) { AddOmpSourceRange(x.source); - ProcessMapperSpecifier(std::get(x.t), - std::get(x.t)); - return false; + return true; } bool Pre(const parser::OpenMPDeclareSimdConstruct &x) { @@ -1580,35 +1568,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(x)); } - void Post(const parser::OmpBeginSectionsDirective &) { - messageHandler().set_currStmtSource(std::nullopt); + void Post(const parser::OmpBeginSectionsDirective &x) { + Post(static_cast(x)); } bool Pre(const parser::OmpEndSectionsDirective &x) { - AddOmpSourceRange(x.source); - return true; + return Pre(static_cast(x)); } - void Post(const parser::OmpEndSectionsDirective &) { - messageHandler().set_currStmtSource(std::nullopt); + void Post(const parser::OmpEndSectionsDirective &x) { + Post(static_cast(x)); } bool Pre(const parser::OpenMPThreadprivate &) { SkipImplicitTyping(true); @@ -1717,7 +1692,15 @@ class OmpVisitor : public virtual DeclarationVisitor { PopScope(); } } + bool Pre(const parser::OmpMapperSpecifier &x) { + // 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 &) { + messageHandler().set_currStmtSource(std::nullopt); + } bool Pre(const parser::OmpTypeSpecifier &x) { BeginDeclTypeSpec(); @@ -1727,6 +1710,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); @@ -1994,30 +1987,40 @@ bool OmpVisitor::Pre(const parser::OmpDirectiveSpecification &x) { const parser::OmpArgumentList &args{x.Arguments()}; const parser::OmpClauseList &clauses{x.Clauses()}; + bool visitClauses{true}; + + for (const parser::OmpArgument &arg : args.v) { + common::visit( // + common::visitors{ + [&](const parser::OmpMapperSpecifier &spec) { + ProcessMapperSpecifier(spec, clauses); + visitClauses = false; + }, + [&](const parser::OmpReductionSpecifier &spec) { + ProcessReductionSpecifier(spec, clauses, declaratives_.back()); + visitClauses = false; + }, + [&](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); + } - 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(&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(&first.u)}) { - ProcessReductionSpecifier(*spec, clauses, declaratives_.back()); - } - } - break; - default: - // Default processing. - Walk(args); + if (visitClauses) { Walk(clauses); - break; } + return false; } 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