From f98244392b4e3d4075c03528dcec0b268ba13ab7 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Wed, 2 Oct 2024 15:36:45 -0500 Subject: [PATCH] [flang][OpenMP] Parse lastprivate modifier, add TODO to lowering (#110568) Parse the lastprivate clause with a modifier. Codegen for it is not yet implemented. --- flang/include/flang/Parser/dump-parse-tree.h | 2 + flang/include/flang/Parser/parse-tree.h | 9 ++++ flang/lib/Lower/OpenMP/Clauses.cpp | 19 +++++-- .../lib/Lower/OpenMP/DataSharingProcessor.cpp | 2 + flang/lib/Lower/OpenMP/OpenMP.cpp | 4 +- flang/lib/Lower/OpenMP/Utils.cpp | 13 +++++ flang/lib/Lower/OpenMP/Utils.h | 3 ++ flang/lib/Parser/openmp-parsers.cpp | 8 ++- flang/lib/Parser/unparse.cpp | 8 +++ flang/lib/Semantics/check-omp-structure.cpp | 40 +++++++++----- flang/lib/Semantics/resolve-directives.cpp | 3 +- .../OpenMP/Todo/lastprivate-conditional.f90 | 12 +++++ .../test/Parser/OpenMP/lastprivate-clause.f90 | 54 +++++++++++++++++++ llvm/include/llvm/Frontend/OpenMP/OMP.td | 2 +- 14 files changed, 160 insertions(+), 19 deletions(-) create mode 100644 flang/test/Lower/OpenMP/Todo/lastprivate-conditional.f90 create mode 100644 flang/test/Parser/OpenMP/lastprivate-clause.f90 diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h index 0b4ebcbaa24c40..bf00e6b43d0668 100644 --- a/flang/include/flang/Parser/dump-parse-tree.h +++ b/flang/include/flang/Parser/dump-parse-tree.h @@ -522,6 +522,8 @@ class ParseTreeDumper { NODE(parser, OmpEndSectionsDirective) NODE(parser, OmpIfClause) NODE_ENUM(OmpIfClause, DirectiveNameModifier) + NODE_ENUM(OmpLastprivateClause, LastprivateModifier) + NODE(parser, OmpLastprivateClause) NODE(parser, OmpLinearClause) NODE(OmpLinearClause, WithModifier) NODE(OmpLinearClause, WithoutModifier) diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h index 548fcc81984b2a..7057ac2267aa15 100644 --- a/flang/include/flang/Parser/parse-tree.h +++ b/flang/include/flang/Parser/parse-tree.h @@ -3648,6 +3648,15 @@ struct OmpAtomicDefaultMemOrderClause { OmpAtomicDefaultMemOrderClause, common::OmpAtomicDefaultMemOrderType); }; +// OMP 5.0 2.19.4.5 lastprivate-clause -> +// LASTPRIVATE ([lastprivate-modifier :] list) +// lastprivate-modifier -> CONDITIONAL +struct OmpLastprivateClause { + TUPLE_CLASS_BOILERPLATE(OmpLastprivateClause); + ENUM_CLASS(LastprivateModifier, Conditional); + std::tuple, OmpObjectList> t; +}; + // OpenMP Clauses struct OmpClause { UNION_CLASS_BOILERPLATE(OmpClause); diff --git a/flang/lib/Lower/OpenMP/Clauses.cpp b/flang/lib/Lower/OpenMP/Clauses.cpp index efac7757ca5855..c93767eb8a5507 100644 --- a/flang/lib/Lower/OpenMP/Clauses.cpp +++ b/flang/lib/Lower/OpenMP/Clauses.cpp @@ -783,9 +783,22 @@ IsDevicePtr make(const parser::OmpClause::IsDevicePtr &inp, Lastprivate make(const parser::OmpClause::Lastprivate &inp, semantics::SemanticsContext &semaCtx) { - // inp.v -> parser::OmpObjectList - return Lastprivate{{/*LastprivateModifier=*/std::nullopt, - /*List=*/makeObjects(inp.v, semaCtx)}}; + // inp.v -> parser::OmpLastprivateClause + using wrapped = parser::OmpLastprivateClause; + + CLAUSET_ENUM_CONVERT( // + convert, parser::OmpLastprivateClause::LastprivateModifier, + Lastprivate::LastprivateModifier, + // clang-format off + MS(Conditional, Conditional) + // clang-format on + ); + + auto &t0 = std::get>(inp.v.t); + auto &t1 = std::get(inp.v.t); + + return Lastprivate{{/*LastprivateModifier=*/maybeApply(convert, t0), + /*List=*/makeObjects(t1, semaCtx)}}; } Linear make(const parser::OmpClause::Linear &inp, diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp index 23a171c6576389..d3e892be74fd83 100644 --- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp +++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp @@ -151,6 +151,8 @@ void DataSharingProcessor::collectSymbolsForPrivatization() { explicitlyPrivatizedSymbols); } else if (const auto &lastPrivateClause = std::get_if(&clause.u)) { + lastprivateModifierNotSupported(*lastPrivateClause, + converter.getCurrentLocation()); const ObjectList &objects = std::get(lastPrivateClause->t); collectOmpObjectListSymbol(objects, explicitlyPrivatizedSymbols); } diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp index 17ebf93edcce1f..60c83586e468b6 100644 --- a/flang/lib/Lower/OpenMP/OpenMP.cpp +++ b/flang/lib/Lower/OpenMP/OpenMP.cpp @@ -1563,7 +1563,9 @@ genSectionsOp(lower::AbstractConverter &converter, lower::SymMap &symTable, for (const Clause &clause : item->clauses) { if (clause.id == llvm::omp::Clause::OMPC_lastprivate) { - lastprivates.push_back(&std::get(clause.u)); + auto &lastp = std::get(clause.u); + lastprivateModifierNotSupported(lastp, converter.getCurrentLocation()); + lastprivates.push_back(&lastp); } else { switch (clause.id) { case llvm::omp::Clause::OMPC_firstprivate: diff --git a/flang/lib/Lower/OpenMP/Utils.cpp b/flang/lib/Lower/OpenMP/Utils.cpp index 8073b24a1d5b45..47bc12e1b8a030 100644 --- a/flang/lib/Lower/OpenMP/Utils.cpp +++ b/flang/lib/Lower/OpenMP/Utils.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -356,6 +357,18 @@ semantics::Symbol *getOmpObjectSymbol(const parser::OmpObject &ompObject) { return sym; } +void lastprivateModifierNotSupported(const omp::clause::Lastprivate &lastp, + mlir::Location loc) { + using Lastprivate = omp::clause::Lastprivate; + auto &maybeMod = + std::get>(lastp.t); + if (maybeMod) { + assert(*maybeMod == Lastprivate::LastprivateModifier::Conditional && + "Unexpected lastprivate modifier"); + TODO(loc, "lastprivate clause with CONDITIONAL modifier"); + } +} + } // namespace omp } // namespace lower } // namespace Fortran diff --git a/flang/lib/Lower/OpenMP/Utils.h b/flang/lib/Lower/OpenMP/Utils.h index 0b4fe9044bfa7b..658d062e67b271 100644 --- a/flang/lib/Lower/OpenMP/Utils.h +++ b/flang/lib/Lower/OpenMP/Utils.h @@ -100,6 +100,9 @@ void genObjectList(const ObjectList &objects, lower::AbstractConverter &converter, llvm::SmallVectorImpl &operands); +void lastprivateModifierNotSupported(const omp::clause::Lastprivate &lastp, + mlir::Location loc); + } // namespace omp } // namespace lower } // namespace Fortran diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp index 52789d6e5f0f6b..cc2930cbd7ded5 100644 --- a/flang/lib/Parser/openmp-parsers.cpp +++ b/flang/lib/Parser/openmp-parsers.cpp @@ -232,6 +232,12 @@ TYPE_PARSER(construct( TYPE_PARSER( construct(designator) || construct("/" >> name / "/")) +// OMP 5.0 2.19.4.5 LASTPRIVATE ([lastprivate-modifier :] list) +TYPE_PARSER(construct( + maybe("CONDITIONAL" >> + pure(OmpLastprivateClause::LastprivateModifier::Conditional) / ":"), + Parser{})) + TYPE_PARSER( "ACQUIRE" >> construct(construct()) || "ACQ_REL" >> construct(construct()) || @@ -289,7 +295,7 @@ TYPE_PARSER( "IS_DEVICE_PTR" >> construct(construct( parenthesized(Parser{}))) || "LASTPRIVATE" >> construct(construct( - parenthesized(Parser{}))) || + parenthesized(Parser{}))) || "LINEAR" >> construct(construct( parenthesized(Parser{}))) || "LINK" >> construct(construct( diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp index 2511a5dda9d095..ab01d82537c03f 100644 --- a/flang/lib/Parser/unparse.cpp +++ b/flang/lib/Parser/unparse.cpp @@ -2067,6 +2067,12 @@ class UnparseVisitor { }, x.u); } + void Unparse(const OmpLastprivateClause &x) { + Walk( + std::get>(x.t), + ":"); + Walk(std::get(x.t)); + } void Unparse(const OmpMapType::Always &) { Word("ALWAYS,"); } void Unparse(const OmpMapClause &x) { Walk(std::get>(x.t), ":"); @@ -2764,6 +2770,8 @@ class UnparseVisitor { WALK_NESTED_ENUM(OmpDefaultClause, Type) // OMP DEFAULT WALK_NESTED_ENUM(OmpDefaultmapClause, ImplicitBehavior) // OMP DEFAULTMAP WALK_NESTED_ENUM(OmpDefaultmapClause, VariableCategory) // OMP DEFAULTMAP + WALK_NESTED_ENUM( + OmpLastprivateClause, LastprivateModifier) // OMP lastprivate-modifier WALK_NESTED_ENUM(OmpScheduleModifierType, ModType) // OMP schedule-modifier WALK_NESTED_ENUM(OmpLinearModifier, Type) // OMP linear-modifier WALK_NESTED_ENUM(OmpDependenceType, Type) // OMP dependence-type diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp index 741227741f9412..5ef504aa72326e 100644 --- a/flang/lib/Semantics/check-omp-structure.cpp +++ b/flang/lib/Semantics/check-omp-structure.cpp @@ -3170,11 +3170,13 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Copyprivate &x) { void OmpStructureChecker::Enter(const parser::OmpClause::Lastprivate &x) { CheckAllowedClause(llvm::omp::Clause::OMPC_lastprivate); - CheckIsVarPartOfAnotherVar(GetContext().clauseSource, x.v, "LASTPRIVATE"); + const auto &objectList{std::get(x.v.t)}; + CheckIsVarPartOfAnotherVar( + GetContext().clauseSource, objectList, "LASTPRIVATE"); DirectivesClauseTriple dirClauseTriple; SymbolSourceMap currSymbols; - GetSymbolsInObjectList(x.v, currSymbols); + GetSymbolsInObjectList(objectList, currSymbols); CheckDefinableObjects(currSymbols, GetClauseKindForParserClass(x)); CheckCopyingPolymorphicAllocatable( currSymbols, llvm::omp::Clause::OMPC_lastprivate); @@ -3189,6 +3191,21 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Lastprivate &x) { CheckPrivateSymbolsInOuterCxt( currSymbols, dirClauseTriple, GetClauseKindForParserClass(x)); + + using LastprivateModifier = parser::OmpLastprivateClause::LastprivateModifier; + const auto &maybeMod{std::get>(x.v.t)}; + if (maybeMod) { + unsigned version{context_.langOptions().OpenMPVersion}; + unsigned allowedInVersion = 50; + if (version < allowedInVersion) { + std::string thisVersion{ + std::to_string(version / 10) + "." + std::to_string(version % 10)}; + context_.Say(GetContext().clauseSource, + "LASTPRIVATE clause with CONDITIONAL modifier is not " + "allowed in OpenMP v%s, try -fopenmp-version=%d"_err_en_US, + thisVersion, allowedInVersion); + } + } } void OmpStructureChecker::Enter(const parser::OmpClause::Copyin &x) { @@ -3608,18 +3625,17 @@ const parser::OmpObjectList *OmpStructureChecker::GetOmpObjectList( const parser::OmpClause &clause) { // Clauses with OmpObjectList as its data member - using MemberObjectListClauses = - std::tuple; + using MemberObjectListClauses = std::tuple; // Clauses with OmpObjectList in the tuple - using TupleObjectListClauses = - std::tuple; + using TupleObjectListClauses = std::tuple; // TODO:: Generate the tuples using TableGen. // Handle other constructs with OmpObjectList such as OpenMPThreadprivate. diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp index 2b9bf0824d5340..2442bdfbdbe639 100644 --- a/flang/lib/Semantics/resolve-directives.cpp +++ b/flang/lib/Semantics/resolve-directives.cpp @@ -455,7 +455,8 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor { return false; } bool Pre(const parser::OmpClause::Lastprivate &x) { - ResolveOmpObjectList(x.v, Symbol::Flag::OmpLastPrivate); + const auto &objList{std::get(x.v.t)}; + ResolveOmpObjectList(objList, Symbol::Flag::OmpLastPrivate); return false; } bool Pre(const parser::OmpClause::Copyin &x) { diff --git a/flang/test/Lower/OpenMP/Todo/lastprivate-conditional.f90 b/flang/test/Lower/OpenMP/Todo/lastprivate-conditional.f90 new file mode 100644 index 00000000000000..2b96093da3a8fa --- /dev/null +++ b/flang/test/Lower/OpenMP/Todo/lastprivate-conditional.f90 @@ -0,0 +1,12 @@ +! RUN: %not_todo_cmd bbc -emit-fir -fopenmp -fopenmp-version=50 -o - %s 2>&1 | FileCheck %s +! RUN: %not_todo_cmd %flang_fc1 -emit-fir -fopenmp -fopenmp-version=50 -o - %s 2>&1 | FileCheck %s + +! CHECK: not yet implemented: lastprivate clause with CONDITIONAL modifier +subroutine foo() + integer :: x, i + x = 1 + !$omp parallel do lastprivate(conditional: x) + do i = 1, 100 + x = x + 1 + enddo +end diff --git a/flang/test/Parser/OpenMP/lastprivate-clause.f90 b/flang/test/Parser/OpenMP/lastprivate-clause.f90 new file mode 100644 index 00000000000000..382f02c75e7f14 --- /dev/null +++ b/flang/test/Parser/OpenMP/lastprivate-clause.f90 @@ -0,0 +1,54 @@ +! RUN: %flang_fc1 -fdebug-unparse -fopenmp -fopenmp-version=50 %s | FileCheck --ignore-case --check-prefix="UNPARSE" %s +! RUN: %flang_fc1 -fdebug-dump-parse-tree -fopenmp -fopenmp-version=50 %s | FileCheck --check-prefix="PARSE-TREE" %s + +subroutine foo1() + integer :: x, i + x = 1 + !$omp parallel do lastprivate(x) + do i = 1, 100 + x = x + 1 + enddo +end + +!UNPARSE: SUBROUTINE foo1 +!UNPARSE: INTEGER x, i +!UNPARSE: x=1_4 +!UNPARSE: !$OMP PARALLEL DO LASTPRIVATE(x) +!UNPARSE: DO i=1_4,100_4 +!UNPARSE: x=x+1_4 +!UNPARSE: END DO +!UNPARSE: END SUBROUTINE + +!PARSE-TREE: SubroutineStmt +!PARSE-TREE: Name = 'foo1' +!PARSE-TREE: OmpLoopDirective -> llvm::omp::Directive = parallel do +!PARSE-TREE: OmpClauseList -> OmpClause -> Lastprivate -> OmpLastprivateClause +!PARSE-TREE: OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'x' +!PARSE-TREE: EndSubroutineStmt + + +subroutine foo2() + integer :: x, i + x = 1 + !$omp parallel do lastprivate(conditional: x) + do i = 1, 100 + x = x + 1 + enddo +end + +!UNPARSE: SUBROUTINE foo2 +!UNPARSE: INTEGER x, i +!UNPARSE: x=1_4 +!UNPARSE: !$OMP PARALLEL DO LASTPRIVATE(CONDITIONAL:x) +!UNPARSE: DO i=1_4,100_4 +!UNPARSE: x=x+1_4 +!UNPARSE: END DO +!UNPARSE: END SUBROUTINE + +!PARSE-TREE: SubroutineStmt +!PARSE-TREE: Name = 'foo2' +!PARSE-TREE: OmpLoopDirective -> llvm::omp::Directive = parallel do +!PARSE-TREE: OmpClauseList -> OmpClause -> Lastprivate -> OmpLastprivateClause +!PARSE-TREE: LastprivateModifier = Conditional +!PARSE-TREE: OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'x' +!PARSE-TREE: EndSubroutineStmt diff --git a/llvm/include/llvm/Frontend/OpenMP/OMP.td b/llvm/include/llvm/Frontend/OpenMP/OMP.td index 01515108ca1857..fcf087d1f9c6e4 100644 --- a/llvm/include/llvm/Frontend/OpenMP/OMP.td +++ b/llvm/include/llvm/Frontend/OpenMP/OMP.td @@ -229,7 +229,7 @@ def OMPC_IsDevicePtr : Clause<"is_device_ptr"> { } def OMPC_LastPrivate : Clause<"lastprivate"> { let clangClass = "OMPLastprivateClause"; - let flangClass = "OmpObjectList"; + let flangClass = "OmpLastprivateClause"; } def OMPC_Linear : Clause<"linear"> { let clangClass = "OMPLinearClause";