From 8bc59ff7116dcaa215e4fbb2ee6a67304e0a7351 Mon Sep 17 00:00:00 2001 From: Mats Petersson Date: Mon, 21 Oct 2024 16:24:29 +0000 Subject: [PATCH 1/7] [Flang][OMP]Add support for DECLARE MAPPER parsing and semantics Will hit a TODO in the lowering, which there are tests added to check for this happening. --- flang/include/flang/Parser/dump-parse-tree.h | 2 + flang/include/flang/Parser/parse-tree.h | 16 ++++++- flang/lib/Lower/OpenMP/OpenMP.cpp | 7 +++ flang/lib/Parser/openmp-parsers.cpp | 12 +++++ flang/lib/Parser/unparse.cpp | 23 +++++++++ flang/lib/Semantics/resolve-directives.cpp | 9 ++++ flang/lib/Semantics/resolve-names.cpp | 26 +++++++++- .../Lower/OpenMP/Todo/omp-declare-mapper.f90 | 47 +++++++++++++++++++ .../Parser/OpenMP/declare-mapper-unparse.f90 | 28 +++++++++++ 9 files changed, 168 insertions(+), 2 deletions(-) create mode 100644 flang/test/Lower/OpenMP/Todo/omp-declare-mapper.f90 create mode 100644 flang/test/Parser/OpenMP/declare-mapper-unparse.f90 diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h index bfeb23de53539..474185177317d 100644 --- a/flang/include/flang/Parser/dump-parse-tree.h +++ b/flang/include/flang/Parser/dump-parse-tree.h @@ -504,6 +504,7 @@ class ParseTreeDumper { NODE(parser, OmpDeclareTargetSpecifier) NODE(parser, OmpDeclareTargetWithClause) NODE(parser, OmpDeclareTargetWithList) + NODE(parser, OmpDeclareMapperSpecifier) NODE(parser, OmpDefaultClause) NODE_ENUM(OmpDefaultClause, Type) NODE(parser, OmpDefaultmapClause) @@ -612,6 +613,7 @@ class ParseTreeDumper { NODE(parser, OpenMPDeclareReductionConstruct) NODE(parser, OpenMPDeclareSimdConstruct) NODE(parser, OpenMPDeclareTargetConstruct) + NODE(parser, OpenMPDeclareMapperConstruct) NODE(parser, OmpMemoryOrderClause) NODE(parser, OmpAtomicClause) NODE(parser, OmpAtomicClauseList) diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h index d2c5b45d99581..77decb44e27e8 100644 --- a/flang/include/flang/Parser/parse-tree.h +++ b/flang/include/flang/Parser/parse-tree.h @@ -3872,6 +3872,19 @@ struct OpenMPDeclareTargetConstruct { std::tuple t; }; +struct OmpDeclareMapperSpecifier { + TUPLE_CLASS_BOILERPLATE(OmpDeclareMapperSpecifier); + std::tuple, TypeSpec, Name> t; +}; + +struct OpenMPDeclareMapperConstruct { + TUPLE_CLASS_BOILERPLATE(OpenMPDeclareMapperConstruct); + CharBlock source; + std::tuple>> + t; +}; + // 2.16 declare-reduction -> DECLARE REDUCTION (reduction-identifier : type-list // : combiner) [initializer-clause] struct OmpReductionCombiner { @@ -3924,7 +3937,8 @@ struct OpenMPDeclarativeConstruct { CharBlock source; std::variant + OpenMPThreadprivate, OpenMPRequiresConstruct, + OpenMPDeclareMapperConstruct> u; }; diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp index 4f9e2347308aa..83a6967c32a82 100644 --- a/flang/lib/Lower/OpenMP/OpenMP.cpp +++ b/flang/lib/Lower/OpenMP/OpenMP.cpp @@ -2597,6 +2597,13 @@ 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) { + TODO(converter.getCurrentLocation(), "OpenMPDeclareMapperConstruct"); +} + static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable, semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval, diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp index 7a0ecc59a2c5c..39915b78c66e7 100644 --- a/flang/lib/Parser/openmp-parsers.cpp +++ b/flang/lib/Parser/openmp-parsers.cpp @@ -836,6 +836,16 @@ TYPE_PARSER( TYPE_PARSER(sourced(construct( verbatim("DECLARE TARGET"_tok), Parser{}))) +// declare-mapper-specifier +TYPE_PARSER(construct( + maybe(name / ":" / !":"_tok), typeSpec / "::", name)) + +// ?.? (not 4.5) Declare Mapper Construct +TYPE_PARSER(sourced( + construct(verbatim("DECLARE MAPPER"_tok), + "(" >> Parser{} / ")", + many("MAP" >> parenthesized(many(Parser{})))))) + TYPE_PARSER(construct(Parser{}) || construct( construct( @@ -944,6 +954,8 @@ TYPE_PARSER(startOmpLine >> withMessage("expected OpenMP construct"_err_en_US, sourced(construct( Parser{}) || + construct( + Parser{}) || construct( Parser{}) || construct( diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp index bbb126dcdb6d5..343c21df5619b 100644 --- a/flang/lib/Parser/unparse.cpp +++ b/flang/lib/Parser/unparse.cpp @@ -2665,6 +2665,29 @@ class UnparseVisitor { EndOpenMP(); return false; }, + [&](const OpenMPDeclareMapperConstruct &z) { + Word("DECLARE MAPPER ("); + const auto &spec{std::get(z.t)}; + if (auto mapname{std::get>(spec.t)}) { + Walk(mapname); + Put(":"); + } + Walk(std::get(spec.t)); + Put("::"); + Walk(std::get(spec.t)); + Put(")"); + + const auto &list{ + std::get>>( + z.t)}; + for (const auto &m : list) { + Put(" MAP("); + Walk(m); + Put(")"); + } + Put("\n"); + return false; + }, [&](const OpenMPDeclareReductionConstruct &) { Word("DECLARE REDUCTION "); return true; diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp index c2b5b9673239b..7748fa3a6eec3 100644 --- a/flang/lib/Semantics/resolve-directives.cpp +++ b/flang/lib/Semantics/resolve-directives.cpp @@ -431,6 +431,9 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor { bool Pre(const parser::OpenMPDeclareTargetConstruct &); void Post(const parser::OpenMPDeclareTargetConstruct &) { PopContext(); } + bool Pre(const parser::OpenMPDeclareMapperConstruct &); + void Post(const parser::OpenMPDeclareMapperConstruct &) { PopContext(); } + bool Pre(const parser::OpenMPThreadprivate &); void Post(const parser::OpenMPThreadprivate &) { PopContext(); } @@ -1944,6 +1947,12 @@ bool OmpAttributeVisitor::Pre(const parser::OpenMPDeclareTargetConstruct &x) { return true; } +bool OmpAttributeVisitor::Pre(const parser::OpenMPDeclareMapperConstruct &x) { + PushContext(x.source, llvm::omp::Directive::OMPD_declare_mapper); + + return true; +} + bool OmpAttributeVisitor::Pre(const parser::OpenMPThreadprivate &x) { PushContext(x.source, llvm::omp::Directive::OMPD_threadprivate); const auto &list{std::get(x.t)}; diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp index e0a8246ebc752..f93a6f7d1c742 100644 --- a/flang/lib/Semantics/resolve-names.cpp +++ b/flang/lib/Semantics/resolve-names.cpp @@ -1058,6 +1058,7 @@ class DeclarationVisitor : public ArraySpecVisitor, const parser::Name *ResolveDesignator(const parser::Designator &); int GetVectorElementKind( TypeCategory category, const std::optional &kind); + std::optional ResolveDerivedType(const parser::Name &); protected: bool BeginDecl(); @@ -1205,7 +1206,6 @@ class DeclarationVisitor : public ArraySpecVisitor, Symbol &DeclareProcEntity( const parser::Name &, Attrs, const Symbol *interface); void SetType(const parser::Name &, const DeclTypeSpec &); - std::optional ResolveDerivedType(const parser::Name &); std::optional ResolveExtendsType( const parser::Name &, const parser::Name *); Symbol *MakeTypeSymbol(const SourceName &, Details &&); @@ -1468,6 +1468,10 @@ class OmpVisitor : public virtual DeclarationVisitor { AddOmpSourceRange(x.source); return true; } + + bool Pre(const parser::OpenMPDeclareMapperConstruct &); + void Post(const parser::OpenMPDeclareMapperConstruct &) { PopScope(); }; + void Post(const parser::OmpBeginLoopDirective &) { messageHandler().set_currStmtSource(std::nullopt); } @@ -1605,6 +1609,26 @@ void OmpVisitor::Post(const parser::OpenMPBlockConstruct &x) { } } +bool OmpVisitor::Pre(const parser::OpenMPDeclareMapperConstruct &x) { + AddOmpSourceRange(x.source); + BeginDeclTypeSpec(); + PushScope(Scope::Kind::OtherConstruct, nullptr); + const auto &spec{std::get(x.t)}; + if (const auto &mapperName{ + std::get>(spec.t)}) { + Symbol *mapperSym{&MakeSymbol(*mapperName, Attrs{})}; + mapperName->symbol = mapperSym; + } + Walk(std::get(spec.t)); + const auto &varName{std::get(spec.t)}; + DeclareObjectEntity(varName); + + Walk(std::get>>(x.t)); + + EndDeclTypeSpec(); + return false; +} + // Walk the parse tree and resolve names to symbols. class ResolveNamesVisitor : public virtual ScopeHandler, public ModuleVisitor, diff --git a/flang/test/Lower/OpenMP/Todo/omp-declare-mapper.f90 b/flang/test/Lower/OpenMP/Todo/omp-declare-mapper.f90 new file mode 100644 index 0000000000000..ebec4b4adb42a --- /dev/null +++ b/flang/test/Lower/OpenMP/Todo/omp-declare-mapper.f90 @@ -0,0 +1,47 @@ +! This test checks lowering of OpenMP declare mapper Directive. + +! RUN: split-file %s %t +! RUN: not %flang_fc1 -emit-fir -fopenmp %t/omp-declare-mapper-1.f90 2>&1 | FileCheck %t/omp-declare-mapper-1.f90 +! RUN not %flang_fc1 -emit-fir -fopenmp %t/omp-declare-mapper-2.f90 2>&1 | FileCheck %t/omp-declare-mapper-2.f90 + +!--- omp-declare-mapper-1.f90 +subroutine declare_mapper_1 + integer,parameter :: nvals = 250 + type my_type + integer :: num_vals + integer, allocatable :: values(:) + end type + + type my_type2 + type (my_type) :: my_type_var + type (my_type) :: temp + real,dimension(nvals) :: unmapped + real,dimension(nvals) :: arr + end type + type (my_type2) :: t + real :: x, y(nvals) + !$omp declare mapper (my_type :: var) map (var, var%values (1:var%num_vals)) +!CHECK: not yet implemented: OpenMPDeclareMapperConstruct +end subroutine declare_mapper_1 + + +!--- omp-declare-mapper-2.f90 +subroutine declare_mapper_2 + integer,parameter :: nvals = 250 + type my_type + integer :: num_vals + integer, allocatable :: values(:) + end type + + type my_type2 + type (my_type) :: my_type_var + type (my_type) :: temp + real,dimension(nvals) :: unmapped + real,dimension(nvals) :: arr + end type + type (my_type2) :: t + real :: x, y(nvals) + !$omp declare mapper (my_mapper : my_type2 :: v) map (v%arr, x, y(:)) & + !$omp& map (alloc : v%temp) +!CHECK: not yet implemented: OpenMPDeclareMapperConstruct +end subroutine declare_mapper_2 diff --git a/flang/test/Parser/OpenMP/declare-mapper-unparse.f90 b/flang/test/Parser/OpenMP/declare-mapper-unparse.f90 new file mode 100644 index 0000000000000..2842960cd02a0 --- /dev/null +++ b/flang/test/Parser/OpenMP/declare-mapper-unparse.f90 @@ -0,0 +1,28 @@ +! RUN: %flang_fc1 -fdebug-unparse-no-sema -fopenmp %s | FileCheck --ignore-case %s +! RUN: %flang_fc1 -fdebug-dump-parse-tree-no-sema -fopenmp %s | FileCheck --check-prefix="PARSE-TREE" %s +program main +!CHECK-LABEL: program main + implicit none + + type ty + integer :: x + end type ty + + +!CHECK: !$OMP DECLARE MAPPER (mymapper:ty::mapped) MAP(mapped,mapped%x) + !$omp declare mapper(mymapper : ty :: mapped) map(mapped, mapped%x) + +!PARSE-TREE: OpenMPDeclareMapperConstruct +!PARSE-TREE: OmpDeclareMapperSpecifier +!PARSE-TREE: Name = 'mymapper' +!PARSE-TREE: TypeSpec -> DerivedTypeSpec +!PARSE-TREE: Name = 'ty' +!PARSE-TREE: Name = 'mapped' +!PARSE-TREE: OmpMapClause +!PARSE-TREE: OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'mapped' +!PARSE-TREE: OmpObject -> Designator -> DataRef -> StructureComponent +!PARSE-TREE: DataRef -> Name = 'mapped' +!PARSE-TREE: Name = 'x' + +end program main +!CHECK-LABEL: end program main From 66b01706eef8c151550846ac9ae0340c1340b4c8 Mon Sep 17 00:00:00 2001 From: Mats Petersson Date: Fri, 8 Nov 2024 14:31:30 +0000 Subject: [PATCH 2/7] Review updates --- flang/include/flang/Parser/parse-tree.h | 12 +++++------ flang/lib/Parser/openmp-parsers.cpp | 9 ++++----- flang/lib/Parser/unparse.cpp | 9 +-------- flang/lib/Semantics/check-omp-structure.cpp | 20 +++++++++++++++++++ flang/lib/Semantics/check-omp-structure.h | 2 ++ flang/lib/Semantics/resolve-directives.cpp | 1 - flang/lib/Semantics/resolve-names.cpp | 17 ++++++++++------ .../Lower/OpenMP/Todo/omp-declare-mapper.f90 | 4 ++-- llvm/include/llvm/Frontend/OpenMP/OMP.td | 4 ++-- 9 files changed, 47 insertions(+), 31 deletions(-) diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h index 77decb44e27e8..8ae317a4f0e42 100644 --- a/flang/include/flang/Parser/parse-tree.h +++ b/flang/include/flang/Parser/parse-tree.h @@ -3880,9 +3880,7 @@ struct OmpDeclareMapperSpecifier { struct OpenMPDeclareMapperConstruct { TUPLE_CLASS_BOILERPLATE(OpenMPDeclareMapperConstruct); CharBlock source; - std::tuple>> - t; + std::tuple t; }; // 2.16 declare-reduction -> DECLARE REDUCTION (reduction-identifier : type-list @@ -3935,10 +3933,10 @@ struct OpenMPDeclarativeAllocate { struct OpenMPDeclarativeConstruct { UNION_CLASS_BOILERPLATE(OpenMPDeclarativeConstruct); CharBlock source; - std::variant + std::variant u; }; diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp index 39915b78c66e7..4867bf1d95d1f 100644 --- a/flang/lib/Parser/openmp-parsers.cpp +++ b/flang/lib/Parser/openmp-parsers.cpp @@ -840,11 +840,10 @@ TYPE_PARSER(sourced(construct( TYPE_PARSER(construct( maybe(name / ":" / !":"_tok), typeSpec / "::", name)) -// ?.? (not 4.5) Declare Mapper Construct -TYPE_PARSER(sourced( - construct(verbatim("DECLARE MAPPER"_tok), - "(" >> Parser{} / ")", - many("MAP" >> parenthesized(many(Parser{})))))) +// OpenMP 5.2: 5.8.8 Declare Mapper Construct +TYPE_PARSER(sourced(construct( + verbatim("DECLARE MAPPER"_tok), + "(" >> Parser{} / ")", Parser{}))) TYPE_PARSER(construct(Parser{}) || construct( diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp index 343c21df5619b..dde567fdb02af 100644 --- a/flang/lib/Parser/unparse.cpp +++ b/flang/lib/Parser/unparse.cpp @@ -2677,14 +2677,7 @@ class UnparseVisitor { Walk(std::get(spec.t)); Put(")"); - const auto &list{ - std::get>>( - z.t)}; - for (const auto &m : list) { - Put(" MAP("); - Walk(m); - Put(")"); - } + Walk(std::get(z.t)); Put("\n"); return false; }, diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp index 014604627f2cd..193db88000a42 100644 --- a/flang/lib/Semantics/check-omp-structure.cpp +++ b/flang/lib/Semantics/check-omp-structure.cpp @@ -1434,6 +1434,26 @@ 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 (const auto derivedTypeSpec{ + std::get_if(&type.u)}) { + if (derivedTypeSpec->derivedTypeSpec->typeSymbol().attrs().test( + Attr::ABSTRACT)) + context_.Say(dir.source, "Type must not be abstract"_err_en_US); + } else { + context_.Say(dir.source, "Type is not a derived type"_err_en_US); + } +} + +void OmpStructureChecker::Leave(const parser::OpenMPDeclareMapperConstruct &) { + dirContext_.pop_back(); +} + void OmpStructureChecker::Enter(const parser::OpenMPDeclareTargetConstruct &x) { const auto &dir{std::get(x.t)}; PushContext(dir.source, llvm::omp::Directive::OMPD_declare_target); diff --git a/flang/lib/Semantics/check-omp-structure.h b/flang/lib/Semantics/check-omp-structure.h index d9236be8bced4..77b978ddd6207 100644 --- a/flang/lib/Semantics/check-omp-structure.h +++ b/flang/lib/Semantics/check-omp-structure.h @@ -90,6 +90,8 @@ class OmpStructureChecker void Leave(const parser::OpenMPDeclareSimdConstruct &); void Enter(const parser::OpenMPDeclarativeAllocate &); void Leave(const parser::OpenMPDeclarativeAllocate &); + void Enter(const parser::OpenMPDeclareMapperConstruct &); + void Leave(const parser::OpenMPDeclareMapperConstruct &); void Enter(const parser::OpenMPDeclareTargetConstruct &); void Leave(const parser::OpenMPDeclareTargetConstruct &); void Enter(const parser::OpenMPDepobjConstruct &); diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp index 7748fa3a6eec3..0a87728537387 100644 --- a/flang/lib/Semantics/resolve-directives.cpp +++ b/flang/lib/Semantics/resolve-directives.cpp @@ -1949,7 +1949,6 @@ bool OmpAttributeVisitor::Pre(const parser::OpenMPDeclareTargetConstruct &x) { bool OmpAttributeVisitor::Pre(const parser::OpenMPDeclareMapperConstruct &x) { PushContext(x.source, llvm::omp::Directive::OMPD_declare_mapper); - return true; } diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp index f93a6f7d1c742..0bbcf28e8ff6e 100644 --- a/flang/lib/Semantics/resolve-names.cpp +++ b/flang/lib/Semantics/resolve-names.cpp @@ -1058,7 +1058,6 @@ class DeclarationVisitor : public ArraySpecVisitor, const parser::Name *ResolveDesignator(const parser::Designator &); int GetVectorElementKind( TypeCategory category, const std::optional &kind); - std::optional ResolveDerivedType(const parser::Name &); protected: bool BeginDecl(); @@ -1206,6 +1205,7 @@ class DeclarationVisitor : public ArraySpecVisitor, Symbol &DeclareProcEntity( const parser::Name &, Attrs, const Symbol *interface); void SetType(const parser::Name &, const DeclTypeSpec &); + std::optional ResolveDerivedType(const parser::Name &); std::optional ResolveExtendsType( const parser::Name &, const parser::Name *); Symbol *MakeTypeSymbol(const SourceName &, Details &&); @@ -1609,21 +1609,26 @@ void OmpVisitor::Post(const parser::OpenMPBlockConstruct &x) { } } +// This "manually" walks the tree of the cosntruct, because the order +// elements are resolved by the normal visitor will try to resolve +// the map clauses attached to the directive without having resolved +// the type, so the type is figured out using the implicit rules. bool OmpVisitor::Pre(const parser::OpenMPDeclareMapperConstruct &x) { AddOmpSourceRange(x.source); BeginDeclTypeSpec(); PushScope(Scope::Kind::OtherConstruct, nullptr); const auto &spec{std::get(x.t)}; - if (const auto &mapperName{ - std::get>(spec.t)}) { + if (const auto &mapperName{std::get>(spec.t)}) { Symbol *mapperSym{&MakeSymbol(*mapperName, Attrs{})}; mapperName->symbol = mapperSym; + } else if (0) { + Symbol *mapperSym{&MakeSymbol("default", Attrs{})}; } - Walk(std::get(spec.t)); - const auto &varName{std::get(spec.t)}; + Walk(std::get(spec.t)); + const auto &varName{std::get(spec.t)}; DeclareObjectEntity(varName); - Walk(std::get>>(x.t)); + Walk(std::get(x.t)); EndDeclTypeSpec(); return false; diff --git a/flang/test/Lower/OpenMP/Todo/omp-declare-mapper.f90 b/flang/test/Lower/OpenMP/Todo/omp-declare-mapper.f90 index ebec4b4adb42a..5ae48ff736048 100644 --- a/flang/test/Lower/OpenMP/Todo/omp-declare-mapper.f90 +++ b/flang/test/Lower/OpenMP/Todo/omp-declare-mapper.f90 @@ -1,8 +1,8 @@ ! This test checks lowering of OpenMP declare mapper Directive. ! RUN: split-file %s %t -! RUN: not %flang_fc1 -emit-fir -fopenmp %t/omp-declare-mapper-1.f90 2>&1 | FileCheck %t/omp-declare-mapper-1.f90 -! RUN not %flang_fc1 -emit-fir -fopenmp %t/omp-declare-mapper-2.f90 2>&1 | FileCheck %t/omp-declare-mapper-2.f90 +! RUN: not %flang_fc1 -emit-fir -fopenmp -fopenmp-version=50 %t/omp-declare-mapper-1.f90 2>&1 | FileCheck %t/omp-declare-mapper-1.f90 +! RUN not %flang_fc1 -emit-fir -fopenmp -fopenmp-version=50 %t/omp-declare-mapper-2.f90 2>&1 | FileCheck %t/omp-declare-mapper-2.f90 !--- omp-declare-mapper-1.f90 subroutine declare_mapper_1 diff --git a/llvm/include/llvm/Frontend/OpenMP/OMP.td b/llvm/include/llvm/Frontend/OpenMP/OMP.td index 36834939d9b45..3bbe4c744004a 100644 --- a/llvm/include/llvm/Frontend/OpenMP/OMP.td +++ b/llvm/include/llvm/Frontend/OpenMP/OMP.td @@ -612,8 +612,8 @@ def OMP_Critical : Directive<"critical"> { let category = CA_Executable; } def OMP_DeclareMapper : Directive<"declare mapper"> { - let allowedClauses = [ - VersionedClause, + let requiredClauses = [ + VersionedClause, ]; let association = AS_None; let category = CA_Declarative; From 3d810ef09f4faba31f29fb47d78d1d9804f5568a Mon Sep 17 00:00:00 2001 From: Mats Petersson Date: Mon, 11 Nov 2024 13:56:07 +0000 Subject: [PATCH 3/7] Move PushContext so mapper name is available in the outer context. Fix mapper name creation to not "fail" with type missing by using ConstructName detail flag. Add tests for abstract and "not derived" type checks. --- flang/lib/Semantics/resolve-names.cpp | 12 ++++++++---- flang/test/Semantics/OpenMP/declare-mapper01.f90 | 8 ++++++++ flang/test/Semantics/OpenMP/declare-mapper02.f90 | 10 ++++++++++ 3 files changed, 26 insertions(+), 4 deletions(-) create mode 100644 flang/test/Semantics/OpenMP/declare-mapper01.f90 create mode 100644 flang/test/Semantics/OpenMP/declare-mapper02.f90 diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp index 0bbcf28e8ff6e..bda6edfc9881b 100644 --- a/flang/lib/Semantics/resolve-names.cpp +++ b/flang/lib/Semantics/resolve-names.cpp @@ -1616,14 +1616,18 @@ void OmpVisitor::Post(const parser::OpenMPBlockConstruct &x) { bool OmpVisitor::Pre(const parser::OpenMPDeclareMapperConstruct &x) { AddOmpSourceRange(x.source); BeginDeclTypeSpec(); - PushScope(Scope::Kind::OtherConstruct, nullptr); const auto &spec{std::get(x.t)}; + Symbol *mapperSym{nullptr}; if (const auto &mapperName{std::get>(spec.t)}) { - Symbol *mapperSym{&MakeSymbol(*mapperName, Attrs{})}; + mapperSym = + &MakeSymbol(*mapperName, MiscDetails{MiscDetails::Kind::ConstructName}); mapperName->symbol = mapperSym; - } else if (0) { - Symbol *mapperSym{&MakeSymbol("default", Attrs{})}; + } else { + mapperSym = &MakeSymbol( + "default", Attrs{}, MiscDetails{MiscDetails::Kind::ConstructName}); } + + PushScope(Scope::Kind::OtherConstruct, nullptr); Walk(std::get(spec.t)); const auto &varName{std::get(spec.t)}; DeclareObjectEntity(varName); diff --git a/flang/test/Semantics/OpenMP/declare-mapper01.f90 b/flang/test/Semantics/OpenMP/declare-mapper01.f90 new file mode 100644 index 0000000000000..02ce129e8f04d --- /dev/null +++ b/flang/test/Semantics/OpenMP/declare-mapper01.f90 @@ -0,0 +1,8 @@ +! RUN: %python %S/../test_errors.py %s %flang -fopenmp -fopenmp-version=50 + ! Test the source code starting with omp syntax + +integer :: y + +!ERROR: Type is not a derived type +!$omp declare mapper(mm : integer::x) map(x, y) +end diff --git a/flang/test/Semantics/OpenMP/declare-mapper02.f90 b/flang/test/Semantics/OpenMP/declare-mapper02.f90 new file mode 100644 index 0000000000000..00d0a46177ec3 --- /dev/null +++ b/flang/test/Semantics/OpenMP/declare-mapper02.f90 @@ -0,0 +1,10 @@ +! RUN: %python %S/../test_errors.py %s %flang -fopenmp -fopenmp-version=50 +! Test the source code starting with omp syntax + +type, abstract :: t1 + integer :: y +end type t1 + +!ERROR: Type must not be abstract +!$omp declare mapper(mm : t1::x) map(x, x%y) +end From 98aca98beb34bf71222f5130605abce66c34d30a Mon Sep 17 00:00:00 2001 From: Mats Petersson Date: Mon, 11 Nov 2024 18:05:27 +0000 Subject: [PATCH 4/7] Update version in OMP.td --- llvm/include/llvm/Frontend/OpenMP/OMP.td | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/include/llvm/Frontend/OpenMP/OMP.td b/llvm/include/llvm/Frontend/OpenMP/OMP.td index 3bbe4c744004a..51e538ab7077b 100644 --- a/llvm/include/llvm/Frontend/OpenMP/OMP.td +++ b/llvm/include/llvm/Frontend/OpenMP/OMP.td @@ -613,7 +613,7 @@ def OMP_Critical : Directive<"critical"> { } def OMP_DeclareMapper : Directive<"declare mapper"> { let requiredClauses = [ - VersionedClause, + VersionedClause, ]; let association = AS_None; let category = CA_Declarative; From a5c72cc3590ae8ef8b3e109428db57df27ebdb4c Mon Sep 17 00:00:00 2001 From: Mats Petersson Date: Tue, 12 Nov 2024 10:41:44 +0000 Subject: [PATCH 5/7] Fix test and remove redundant check --- flang/lib/Semantics/check-omp-structure.cpp | 7 +------ flang/test/Semantics/OpenMP/declare-mapper02.f90 | 2 +- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp index 193db88000a42..988f57d3b617e 100644 --- a/flang/lib/Semantics/check-omp-structure.cpp +++ b/flang/lib/Semantics/check-omp-structure.cpp @@ -1440,12 +1440,7 @@ void OmpStructureChecker::Enter(const parser::OpenMPDeclareMapperConstruct &x) { dir.source, llvm::omp::Directive::OMPD_declare_mapper); const auto &spec{std::get(x.t)}; const auto &type = std::get(spec.t); - if (const auto derivedTypeSpec{ - std::get_if(&type.u)}) { - if (derivedTypeSpec->derivedTypeSpec->typeSymbol().attrs().test( - Attr::ABSTRACT)) - context_.Say(dir.source, "Type must not be abstract"_err_en_US); - } else { + if (!std::get_if(&type.u)) { context_.Say(dir.source, "Type is not a derived type"_err_en_US); } } diff --git a/flang/test/Semantics/OpenMP/declare-mapper02.f90 b/flang/test/Semantics/OpenMP/declare-mapper02.f90 index 00d0a46177ec3..842943b2bf960 100644 --- a/flang/test/Semantics/OpenMP/declare-mapper02.f90 +++ b/flang/test/Semantics/OpenMP/declare-mapper02.f90 @@ -5,6 +5,6 @@ integer :: y end type t1 -!ERROR: Type must not be abstract +!ERROR: ABSTRACT derived type may not be used here !$omp declare mapper(mm : t1::x) map(x, x%y) end From 085454a8faee59dc404e42389b57fde25dfc2a3f Mon Sep 17 00:00:00 2001 From: Mats Petersson Date: Tue, 12 Nov 2024 11:30:03 +0000 Subject: [PATCH 6/7] Add documentation for parsing --- flang/include/flang/Parser/parse-tree.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h index 8ae317a4f0e42..f12b98b6239d9 100644 --- a/flang/include/flang/Parser/parse-tree.h +++ b/flang/include/flang/Parser/parse-tree.h @@ -3877,6 +3877,8 @@ struct OmpDeclareMapperSpecifier { std::tuple, TypeSpec, Name> t; }; +// OMP v5.2: 5.8.8 +// declare-mapper -> DECLARE MAPPER ([mapper-name :] type :: var) map-clauses struct OpenMPDeclareMapperConstruct { TUPLE_CLASS_BOILERPLATE(OpenMPDeclareMapperConstruct); CharBlock source; From 3e563fdbe4d988908f0c52853b13d15665fbb893 Mon Sep 17 00:00:00 2001 From: Mats Petersson Date: Tue, 12 Nov 2024 19:55:40 +0000 Subject: [PATCH 7/7] Add further tests, fix comments --- flang/lib/Semantics/resolve-names.cpp | 13 +++++----- flang/lib/Semantics/unparse-with-symbols.cpp | 8 +++++++ .../Parser/OpenMP/declare-mapper-unparse.f90 | 14 +++++++++++ .../OpenMP/declare-mapper-symbols.f90 | 24 +++++++++++++++++++ .../Semantics/OpenMP/declare-mapper01.f90 | 2 +- .../Semantics/OpenMP/declare-mapper02.f90 | 2 +- .../Semantics/OpenMP/declare-mapper03.f90 | 16 +++++++++++++ 7 files changed, 71 insertions(+), 8 deletions(-) create mode 100644 flang/test/Semantics/OpenMP/declare-mapper-symbols.f90 create mode 100644 flang/test/Semantics/OpenMP/declare-mapper03.f90 diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp index bda6edfc9881b..9b0e204ac4e91 100644 --- a/flang/lib/Semantics/resolve-names.cpp +++ b/flang/lib/Semantics/resolve-names.cpp @@ -1470,7 +1470,6 @@ class OmpVisitor : public virtual DeclarationVisitor { } bool Pre(const parser::OpenMPDeclareMapperConstruct &); - void Post(const parser::OpenMPDeclareMapperConstruct &) { PopScope(); }; void Post(const parser::OmpBeginLoopDirective &) { messageHandler().set_currStmtSource(std::nullopt); @@ -1609,10 +1608,10 @@ void OmpVisitor::Post(const parser::OpenMPBlockConstruct &x) { } } -// This "manually" walks the tree of the cosntruct, because the order -// elements are resolved by the normal visitor will try to resolve -// the map clauses attached to the directive without having resolved -// the type, so the type is figured out using the implicit rules. +// This "manually" walks the tree of the construct, because we need +// to resolve the type before the map clauses are processed - when +// just following the natural flow, the map clauses gets processed before +// the type has been fully processed. bool OmpVisitor::Pre(const parser::OpenMPDeclareMapperConstruct &x) { AddOmpSourceRange(x.source); BeginDeclTypeSpec(); @@ -1623,8 +1622,9 @@ bool OmpVisitor::Pre(const parser::OpenMPDeclareMapperConstruct &x) { &MakeSymbol(*mapperName, MiscDetails{MiscDetails::Kind::ConstructName}); mapperName->symbol = mapperSym; } else { + const parser::CharBlock defaultName{"default", 7}; mapperSym = &MakeSymbol( - "default", Attrs{}, MiscDetails{MiscDetails::Kind::ConstructName}); + defaultName, Attrs{}, MiscDetails{MiscDetails::Kind::ConstructName}); } PushScope(Scope::Kind::OtherConstruct, nullptr); @@ -1635,6 +1635,7 @@ bool OmpVisitor::Pre(const parser::OpenMPDeclareMapperConstruct &x) { Walk(std::get(x.t)); EndDeclTypeSpec(); + PopScope(); return false; } diff --git a/flang/lib/Semantics/unparse-with-symbols.cpp b/flang/lib/Semantics/unparse-with-symbols.cpp index c451f885c0627..02afb89ae57fa 100644 --- a/flang/lib/Semantics/unparse-with-symbols.cpp +++ b/flang/lib/Semantics/unparse-with-symbols.cpp @@ -53,6 +53,14 @@ class SymbolDumpVisitor { void Post(const parser::OpenMPThreadprivate &) { currStmt_ = std::nullopt; } void Post(const parser::Name &name); + bool Pre(const parser::OpenMPDeclareMapperConstruct &x) { + currStmt_ = x.source; + return true; + } + void Post(const parser::OpenMPDeclareMapperConstruct &) { + currStmt_ = std::nullopt; + } + private: std::optional currStmt_; // current statement we are processing std::multimap symbols_; // location to symbol diff --git a/flang/test/Parser/OpenMP/declare-mapper-unparse.f90 b/flang/test/Parser/OpenMP/declare-mapper-unparse.f90 index 2842960cd02a0..5ba147d20955e 100644 --- a/flang/test/Parser/OpenMP/declare-mapper-unparse.f90 +++ b/flang/test/Parser/OpenMP/declare-mapper-unparse.f90 @@ -24,5 +24,19 @@ program main !PARSE-TREE: DataRef -> Name = 'mapped' !PARSE-TREE: Name = 'x' +!CHECK: !$OMP DECLARE MAPPER (ty::mapped) MAP(mapped,mapped%x) + !$omp declare mapper(ty :: mapped) map(mapped, mapped%x) + +!PARSE-TREE: OpenMPDeclareMapperConstruct +!PARSE-TREE: OmpDeclareMapperSpecifier +!PARSE-TREE: TypeSpec -> DerivedTypeSpec +!PARSE-TREE: Name = 'ty' +!PARSE-TREE: Name = 'mapped' +!PARSE-TREE: OmpMapClause +!PARSE-TREE: OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'mapped' +!PARSE-TREE: OmpObject -> Designator -> DataRef -> StructureComponent +!PARSE-TREE: DataRef -> Name = 'mapped' +!PARSE-TREE: Name = 'x' + end program main !CHECK-LABEL: end program main diff --git a/flang/test/Semantics/OpenMP/declare-mapper-symbols.f90 b/flang/test/Semantics/OpenMP/declare-mapper-symbols.f90 new file mode 100644 index 0000000000000..b4e03bd1632e5 --- /dev/null +++ b/flang/test/Semantics/OpenMP/declare-mapper-symbols.f90 @@ -0,0 +1,24 @@ +! RUN: %flang_fc1 -fdebug-dump-symbols -fopenmp -fopenmp-version=50 %s | FileCheck %s + +program main +!CHECK-LABEL: MainProgram scope: main + implicit none + + type ty + integer :: x + end type ty + !$omp declare mapper(mymapper : ty :: mapped) map(mapped, mapped%x) + !$omp declare mapper(ty :: maptwo) map(maptwo, maptwo%x) + +!! Note, symbols come out in their respective scope, but not in declaration order. +!CHECK: default: Misc ConstructName +!CHECK: mymapper: Misc ConstructName +!CHECK: ty: DerivedType components: x +!CHECK: DerivedType scope: ty +!CHECK: OtherConstruct scope: +!CHECK: mapped (OmpMapToFrom) {{.*}} ObjectEntity type: TYPE(ty) +!CHECK: OtherConstruct scope: +!CHECK: maptwo (OmpMapToFrom) {{.*}} ObjectEntity type: TYPE(ty) + +end program main + diff --git a/flang/test/Semantics/OpenMP/declare-mapper01.f90 b/flang/test/Semantics/OpenMP/declare-mapper01.f90 index 02ce129e8f04d..0712b80769a45 100644 --- a/flang/test/Semantics/OpenMP/declare-mapper01.f90 +++ b/flang/test/Semantics/OpenMP/declare-mapper01.f90 @@ -1,5 +1,5 @@ ! RUN: %python %S/../test_errors.py %s %flang -fopenmp -fopenmp-version=50 - ! Test the source code starting with omp syntax +! Test the declare mapper with non-derived type. integer :: y diff --git a/flang/test/Semantics/OpenMP/declare-mapper02.f90 b/flang/test/Semantics/OpenMP/declare-mapper02.f90 index 842943b2bf960..a62a7f8d0a392 100644 --- a/flang/test/Semantics/OpenMP/declare-mapper02.f90 +++ b/flang/test/Semantics/OpenMP/declare-mapper02.f90 @@ -1,5 +1,5 @@ ! RUN: %python %S/../test_errors.py %s %flang -fopenmp -fopenmp-version=50 -! Test the source code starting with omp syntax +! Test the declare mapper construct with abstract type. type, abstract :: t1 integer :: y diff --git a/flang/test/Semantics/OpenMP/declare-mapper03.f90 b/flang/test/Semantics/OpenMP/declare-mapper03.f90 new file mode 100644 index 0000000000000..b70b8a67f33e0 --- /dev/null +++ b/flang/test/Semantics/OpenMP/declare-mapper03.f90 @@ -0,0 +1,16 @@ +! RUN: %python %S/../test_errors.py %s %flang -fopenmp -fopenmp-version=50 +! Test the declare mapper construct with two default mappers. + +type :: t1 + integer :: y +end type t1 + +type :: t2 + real :: y, z +end type t2 + +!error: 'default' is already declared in this scoping unit + +!$omp declare mapper(t1::x) map(x, x%y) +!$omp declare mapper(t2::w) map(w, w%y, w%z) +end