Skip to content

[Flang][OMP]Add support for DECLARE MAPPER parsing and semantics #115160

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Nov 14, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions flang/include/flang/Parser/dump-parse-tree.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
20 changes: 17 additions & 3 deletions flang/include/flang/Parser/parse-tree.h
Original file line number Diff line number Diff line change
Expand Up @@ -3872,6 +3872,19 @@ struct OpenMPDeclareTargetConstruct {
std::tuple<Verbatim, OmpDeclareTargetSpecifier> t;
};

struct OmpDeclareMapperSpecifier {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ultra nit: some of the declarations in this file have a comment giving the OpenMP 5.2 section number and what the parsing looks like (e.g. see DECLARE REDUCTION below). It would be nice to see one here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now fixed.

TUPLE_CLASS_BOILERPLATE(OmpDeclareMapperSpecifier);
std::tuple<std::optional<Name>, 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;
std::tuple<Verbatim, OmpDeclareMapperSpecifier, OmpClauseList> t;
};

// 2.16 declare-reduction -> DECLARE REDUCTION (reduction-identifier : type-list
// : combiner) [initializer-clause]
struct OmpReductionCombiner {
Expand Down Expand Up @@ -3922,9 +3935,10 @@ struct OpenMPDeclarativeAllocate {
struct OpenMPDeclarativeConstruct {
UNION_CLASS_BOILERPLATE(OpenMPDeclarativeConstruct);
CharBlock source;
std::variant<OpenMPDeclarativeAllocate, OpenMPDeclareReductionConstruct,
OpenMPDeclareSimdConstruct, OpenMPDeclareTargetConstruct,
OpenMPThreadprivate, OpenMPRequiresConstruct>
std::variant<OpenMPDeclarativeAllocate, OpenMPDeclareMapperConstruct,
OpenMPDeclareReductionConstruct, OpenMPDeclareSimdConstruct,
OpenMPDeclareTargetConstruct, OpenMPThreadprivate,
OpenMPRequiresConstruct>
u;
};

Expand Down
7 changes: 7 additions & 0 deletions flang/lib/Lower/OpenMP/OpenMP.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
11 changes: 11 additions & 0 deletions flang/lib/Parser/openmp-parsers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -836,6 +836,15 @@ TYPE_PARSER(
TYPE_PARSER(sourced(construct<OpenMPDeclareTargetConstruct>(
verbatim("DECLARE TARGET"_tok), Parser<OmpDeclareTargetSpecifier>{})))

// declare-mapper-specifier
TYPE_PARSER(construct<OmpDeclareMapperSpecifier>(
maybe(name / ":" / !":"_tok), typeSpec / "::", name))

// OpenMP 5.2: 5.8.8 Declare Mapper Construct
TYPE_PARSER(sourced(construct<OpenMPDeclareMapperConstruct>(
verbatim("DECLARE MAPPER"_tok),
"(" >> Parser<OmpDeclareMapperSpecifier>{} / ")", Parser<OmpClauseList>{})))

TYPE_PARSER(construct<OmpReductionCombiner>(Parser<AssignmentStmt>{}) ||
construct<OmpReductionCombiner>(
construct<OmpReductionCombiner::FunctionCombiner>(
Expand Down Expand Up @@ -944,6 +953,8 @@ TYPE_PARSER(startOmpLine >>
withMessage("expected OpenMP construct"_err_en_US,
sourced(construct<OpenMPDeclarativeConstruct>(
Parser<OpenMPDeclareReductionConstruct>{}) ||
construct<OpenMPDeclarativeConstruct>(
Parser<OpenMPDeclareMapperConstruct>{}) ||
construct<OpenMPDeclarativeConstruct>(
Parser<OpenMPDeclareSimdConstruct>{}) ||
construct<OpenMPDeclarativeConstruct>(
Expand Down
16 changes: 16 additions & 0 deletions flang/lib/Parser/unparse.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2665,6 +2665,22 @@ class UnparseVisitor {
EndOpenMP();
return false;
},
[&](const OpenMPDeclareMapperConstruct &z) {
Word("DECLARE MAPPER (");
const auto &spec{std::get<OmpDeclareMapperSpecifier>(z.t)};
if (auto mapname{std::get<std::optional<Name>>(spec.t)}) {
Walk(mapname);
Put(":");
}
Walk(std::get<TypeSpec>(spec.t));
Put("::");
Walk(std::get<Name>(spec.t));
Put(")");

Walk(std::get<OmpClauseList>(z.t));
Put("\n");
return false;
},
[&](const OpenMPDeclareReductionConstruct &) {
Word("DECLARE REDUCTION ");
return true;
Expand Down
15 changes: 15 additions & 0 deletions flang/lib/Semantics/check-omp-structure.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1434,6 +1434,21 @@ 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::OmpDeclareMapperSpecifier>(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);
}
}

void OmpStructureChecker::Leave(const parser::OpenMPDeclareMapperConstruct &) {
dirContext_.pop_back();
}

void OmpStructureChecker::Enter(const parser::OpenMPDeclareTargetConstruct &x) {
const auto &dir{std::get<parser::Verbatim>(x.t)};
PushContext(dir.source, llvm::omp::Directive::OMPD_declare_target);
Expand Down
2 changes: 2 additions & 0 deletions flang/lib/Semantics/check-omp-structure.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 &);
Expand Down
8 changes: 8 additions & 0 deletions flang/lib/Semantics/resolve-directives.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,9 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
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(); }

Expand Down Expand Up @@ -1944,6 +1947,11 @@ 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<parser::OmpObjectList>(x.t)};
Expand Down
33 changes: 33 additions & 0 deletions flang/lib/Semantics/resolve-names.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -1605,6 +1609,35 @@ 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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a few comments saying what this function is doing and why everything is done in Pre?
The change here requires a debug-dump-symbols or an unparse with symbols test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see a debug-dump-symbols test (%flang_fc1 -fopenmp -fdebug-dump-symbols -o - %s 2>&1 | FileCheck %s) or an unparse with symbols test (! RUN: %python %S/../test_symbols.py %s %flang_fc1 -fopenmp).

AddOmpSourceRange(x.source);
BeginDeclTypeSpec();
const auto &spec{std::get<parser::OmpDeclareMapperSpecifier>(x.t)};
Symbol *mapperSym{nullptr};
if (const auto &mapperName{std::get<std::optional<parser::Name>>(spec.t)}) {
mapperSym =
&MakeSymbol(*mapperName, MiscDetails{MiscDetails::Kind::ConstructName});
mapperName->symbol = mapperSym;
} else {
mapperSym = &MakeSymbol(
"default", Attrs{}, MiscDetails{MiscDetails::Kind::ConstructName});
}

PushScope(Scope::Kind::OtherConstruct, nullptr);
Walk(std::get<parser::TypeSpec>(spec.t));
const auto &varName{std::get<parser::ObjectName>(spec.t)};
DeclareObjectEntity(varName);

Walk(std::get<parser::OmpClauseList>(x.t));

EndDeclTypeSpec();
return false;
}

// Walk the parse tree and resolve names to symbols.
class ResolveNamesVisitor : public virtual ScopeHandler,
public ModuleVisitor,
Expand Down
47 changes: 47 additions & 0 deletions flang/test/Lower/OpenMP/Todo/omp-declare-mapper.f90
Original file line number Diff line number Diff line change
@@ -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 -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
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
28 changes: 28 additions & 0 deletions flang/test/Parser/OpenMP/declare-mapper-unparse.f90
Original file line number Diff line number Diff line change
@@ -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
8 changes: 8 additions & 0 deletions flang/test/Semantics/OpenMP/declare-mapper01.f90
Original file line number Diff line number Diff line change
@@ -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
10 changes: 10 additions & 0 deletions flang/test/Semantics/OpenMP/declare-mapper02.f90
Original file line number Diff line number Diff line change
@@ -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: ABSTRACT derived type may not be used here
!$omp declare mapper(mm : t1::x) map(x, x%y)
end
4 changes: 2 additions & 2 deletions llvm/include/llvm/Frontend/OpenMP/OMP.td
Original file line number Diff line number Diff line change
Expand Up @@ -612,8 +612,8 @@ def OMP_Critical : Directive<"critical"> {
let category = CA_Executable;
}
def OMP_DeclareMapper : Directive<"declare mapper"> {
let allowedClauses = [
VersionedClause<OMPC_Map>,
let requiredClauses = [
VersionedClause<OMPC_Map, 45>,
];
let association = AS_None;
let category = CA_Declarative;
Expand Down
Loading