https://github.com/kparzysz created https://github.com/llvm/llvm-project/pull/159632
Since ODS doesn't store a list of OmpObjects (i.e. not as OmpObjectList), some semantics-checking functions needed to be updated to operate on a single object at a time. >From 7bb9fb5b3b9a2dfcd1d00f01c86fe26c5d14c30f Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek <[email protected]> Date: Thu, 18 Sep 2025 08:49:38 -0500 Subject: [PATCH] [flang][OpenMP] Use OmpDirectiveSpecification in THREADPRIVATE Since ODS doesn't store a list of OmpObjects (i.e. not as OmpObjectList), some semantics-checking functions needed to be updated to operate on a single object at a time. --- flang/include/flang/Parser/openmp-utils.h | 4 +- flang/include/flang/Parser/parse-tree.h | 3 +- flang/include/flang/Semantics/openmp-utils.h | 3 +- flang/lib/Parser/openmp-parsers.cpp | 7 +- flang/lib/Parser/unparse.cpp | 7 +- flang/lib/Semantics/check-omp-structure.cpp | 89 +++++++++++--------- flang/lib/Semantics/check-omp-structure.h | 3 + flang/lib/Semantics/openmp-utils.cpp | 22 +++-- flang/lib/Semantics/resolve-directives.cpp | 11 ++- 9 files changed, 86 insertions(+), 63 deletions(-) diff --git a/flang/include/flang/Parser/openmp-utils.h b/flang/include/flang/Parser/openmp-utils.h index 032fb8996fe48..1372945427955 100644 --- a/flang/include/flang/Parser/openmp-utils.h +++ b/flang/include/flang/Parser/openmp-utils.h @@ -49,7 +49,6 @@ MAKE_CONSTR_ID(OpenMPDeclareSimdConstruct, D::OMPD_declare_simd); MAKE_CONSTR_ID(OpenMPDeclareTargetConstruct, D::OMPD_declare_target); MAKE_CONSTR_ID(OpenMPExecutableAllocate, D::OMPD_allocate); MAKE_CONSTR_ID(OpenMPRequiresConstruct, D::OMPD_requires); -MAKE_CONSTR_ID(OpenMPThreadprivate, D::OMPD_threadprivate); #undef MAKE_CONSTR_ID @@ -111,8 +110,7 @@ struct DirectiveNameScope { std::is_same_v<T, OpenMPDeclareSimdConstruct> || std::is_same_v<T, OpenMPDeclareTargetConstruct> || std::is_same_v<T, OpenMPExecutableAllocate> || - std::is_same_v<T, OpenMPRequiresConstruct> || - std::is_same_v<T, OpenMPThreadprivate>) { + std::is_same_v<T, OpenMPRequiresConstruct>) { return MakeName(std::get<Verbatim>(x.t).source, ConstructId<T>::id); } else { return GetFromTuple( diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h index 09a45476420df..8cb6d2e744876 100644 --- a/flang/include/flang/Parser/parse-tree.h +++ b/flang/include/flang/Parser/parse-tree.h @@ -5001,9 +5001,8 @@ struct OpenMPRequiresConstruct { // 2.15.2 threadprivate -> THREADPRIVATE (variable-name-list) struct OpenMPThreadprivate { - TUPLE_CLASS_BOILERPLATE(OpenMPThreadprivate); + WRAPPER_CLASS_BOILERPLATE(OpenMPThreadprivate, OmpDirectiveSpecification); CharBlock source; - std::tuple<Verbatim, OmpObjectList> t; }; // 2.11.3 allocate -> ALLOCATE (variable-name-list) [clause] diff --git a/flang/include/flang/Semantics/openmp-utils.h b/flang/include/flang/Semantics/openmp-utils.h index 68318d6093a1e..65441728c5549 100644 --- a/flang/include/flang/Semantics/openmp-utils.h +++ b/flang/include/flang/Semantics/openmp-utils.h @@ -58,9 +58,10 @@ const parser::DataRef *GetDataRefFromObj(const parser::OmpObject &object); const parser::ArrayElement *GetArrayElementFromObj( const parser::OmpObject &object); const Symbol *GetObjectSymbol(const parser::OmpObject &object); -const Symbol *GetArgumentSymbol(const parser::OmpArgument &argument); std::optional<parser::CharBlock> GetObjectSource( const parser::OmpObject &object); +const Symbol *GetArgumentSymbol(const parser::OmpArgument &argument); +const parser::OmpObject *GetArgumentObject(const parser::OmpArgument &argument); bool IsCommonBlock(const Symbol &sym); bool IsExtendedListItem(const Symbol &sym); diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp index 66526ba00b5ed..60ce71cf983f6 100644 --- a/flang/lib/Parser/openmp-parsers.cpp +++ b/flang/lib/Parser/openmp-parsers.cpp @@ -1791,8 +1791,11 @@ TYPE_PARSER(sourced(construct<OpenMPRequiresConstruct>( verbatim("REQUIRES"_tok), Parser<OmpClauseList>{}))) // 2.15.2 Threadprivate directive -TYPE_PARSER(sourced(construct<OpenMPThreadprivate>( - verbatim("THREADPRIVATE"_tok), parenthesized(Parser<OmpObjectList>{})))) +TYPE_PARSER(sourced( // + construct<OpenMPThreadprivate>( + predicated(OmpDirectiveNameParser{}, + IsDirective(llvm::omp::Directive::OMPD_threadprivate)) >= + Parser<OmpDirectiveSpecification>{}))) // 2.11.3 Declarative Allocate directive TYPE_PARSER( diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp index 189a34ee1dc56..db46525ac57b1 100644 --- a/flang/lib/Parser/unparse.cpp +++ b/flang/lib/Parser/unparse.cpp @@ -2611,12 +2611,11 @@ class UnparseVisitor { } void Unparse(const OpenMPThreadprivate &x) { BeginOpenMP(); - Word("!$OMP THREADPRIVATE ("); - Walk(std::get<parser::OmpObjectList>(x.t)); - Put(")\n"); + Word("!$OMP "); + Walk(x.v); + Put("\n"); EndOpenMP(); } - bool Pre(const OmpMessageClause &x) { Walk(x.v); return false; diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp index 1ee5385fb38a1..507957dfecb3d 100644 --- a/flang/lib/Semantics/check-omp-structure.cpp +++ b/flang/lib/Semantics/check-omp-structure.cpp @@ -669,11 +669,6 @@ template <typename Checker> struct DirectiveSpellingVisitor { checker_(x.v.DirName().source, Directive::OMPD_groupprivate); return false; } - bool Pre(const parser::OpenMPThreadprivate &x) { - checker_( - std::get<parser::Verbatim>(x.t).source, Directive::OMPD_threadprivate); - return false; - } bool Pre(const parser::OpenMPRequiresConstruct &x) { checker_(std::get<parser::Verbatim>(x.t).source, Directive::OMPD_requires); return false; @@ -1306,15 +1301,20 @@ void OmpStructureChecker::CheckThreadprivateOrDeclareTargetVar( } } +void OmpStructureChecker::CheckThreadprivateOrDeclareTargetVar( + const parser::OmpObject &object) { + common::visit( // + common::visitors{ + [&](auto &&s) { CheckThreadprivateOrDeclareTargetVar(s); }, + [&](const parser::OmpObject::Invalid &invalid) {}, + }, + object.u); +} + void OmpStructureChecker::CheckThreadprivateOrDeclareTargetVar( const parser::OmpObjectList &objList) { for (const auto &ompObject : objList.v) { - common::visit( // - common::visitors{ - [&](auto &&s) { CheckThreadprivateOrDeclareTargetVar(s); }, - [&](const parser::OmpObject::Invalid &invalid) {}, - }, - ompObject.u); + CheckThreadprivateOrDeclareTargetVar(ompObject); } } @@ -1374,18 +1374,20 @@ void OmpStructureChecker::Leave(const parser::OpenMPGroupprivate &x) { dirContext_.pop_back(); } -void OmpStructureChecker::Enter(const parser::OpenMPThreadprivate &c) { - const auto &dir{std::get<parser::Verbatim>(c.t)}; - PushContextAndClauseSets( - dir.source, llvm::omp::Directive::OMPD_threadprivate); +void OmpStructureChecker::Enter(const parser::OpenMPThreadprivate &x) { + const parser::OmpDirectiveName &dirName{x.v.DirName()}; + PushContextAndClauseSets(dirName.source, dirName.v); } -void OmpStructureChecker::Leave(const parser::OpenMPThreadprivate &c) { - const auto &dir{std::get<parser::Verbatim>(c.t)}; - const auto &objectList{std::get<parser::OmpObjectList>(c.t)}; - CheckSymbolNames(dir.source, objectList); - CheckVarIsNotPartOfAnotherVar(dir.source, objectList); - CheckThreadprivateOrDeclareTargetVar(objectList); +void OmpStructureChecker::Leave(const parser::OpenMPThreadprivate &x) { + const parser::OmpDirectiveSpecification &dirSpec{x.v}; + for (const parser::OmpArgument &arg : x.v.Arguments().v) { + if (auto *object{GetArgumentObject(arg)}) { + CheckSymbolName(dirSpec.source, *object); + CheckVarIsNotPartOfAnotherVar(dirSpec.source, *object); + CheckThreadprivateOrDeclareTargetVar(*object); + } + } dirContext_.pop_back(); } @@ -1684,30 +1686,35 @@ void OmpStructureChecker::Enter(const parser::OmpDeclareTargetWithList &x) { } } -void OmpStructureChecker::CheckSymbolNames( - const parser::CharBlock &source, const parser::OmpObjectList &objList) { - for (const auto &ompObject : objList.v) { - common::visit( - common::visitors{ - [&](const parser::Designator &designator) { - if (const auto *name{parser::Unwrap<parser::Name>(ompObject)}) { - if (!name->symbol) { - context_.Say(source, - "The given %s directive clause has an invalid argument"_err_en_US, - ContextDirectiveAsFortran()); - } - } - }, - [&](const parser::Name &name) { - if (!name.symbol) { +void OmpStructureChecker::CheckSymbolName( + const parser::CharBlock &source, const parser::OmpObject &object) { + common::visit( + common::visitors{ + [&](const parser::Designator &designator) { + if (const auto *name{parser::Unwrap<parser::Name>(object)}) { + if (!name->symbol) { context_.Say(source, "The given %s directive clause has an invalid argument"_err_en_US, ContextDirectiveAsFortran()); } - }, - [&](const parser::OmpObject::Invalid &invalid) {}, - }, - ompObject.u); + } + }, + [&](const parser::Name &name) { + if (!name.symbol) { + context_.Say(source, + "The given %s directive clause has an invalid argument"_err_en_US, + ContextDirectiveAsFortran()); + } + }, + [&](const parser::OmpObject::Invalid &invalid) {}, + }, + object.u); +} + +void OmpStructureChecker::CheckSymbolNames( + const parser::CharBlock &source, const parser::OmpObjectList &objList) { + for (const auto &ompObject : objList.v) { + CheckSymbolName(source, ompObject); } } diff --git a/flang/lib/Semantics/check-omp-structure.h b/flang/lib/Semantics/check-omp-structure.h index ce074f5f3f86e..6de69e1a8e4f1 100644 --- a/flang/lib/Semantics/check-omp-structure.h +++ b/flang/lib/Semantics/check-omp-structure.h @@ -228,7 +228,10 @@ class OmpStructureChecker const parser::OmpObjectList &objList, llvm::StringRef clause = ""); void CheckThreadprivateOrDeclareTargetVar(const parser::Designator &); void CheckThreadprivateOrDeclareTargetVar(const parser::Name &); + void CheckThreadprivateOrDeclareTargetVar(const parser::OmpObject &); void CheckThreadprivateOrDeclareTargetVar(const parser::OmpObjectList &); + void CheckSymbolName( + const parser::CharBlock &source, const parser::OmpObject &object); void CheckSymbolNames( const parser::CharBlock &source, const parser::OmpObjectList &objList); void CheckIntentInPointer(SymbolSourceMap &, const llvm::omp::Clause); diff --git a/flang/lib/Semantics/openmp-utils.cpp b/flang/lib/Semantics/openmp-utils.cpp index e75149f21d117..3dff541ffbda0 100644 --- a/flang/lib/Semantics/openmp-utils.cpp +++ b/flang/lib/Semantics/openmp-utils.cpp @@ -105,6 +105,16 @@ const Symbol *GetObjectSymbol(const parser::OmpObject &object) { return nullptr; } +std::optional<parser::CharBlock> GetObjectSource( + const parser::OmpObject &object) { + if (auto *name{std::get_if<parser::Name>(&object.u)}) { + return name->source; + } else if (auto *desg{std::get_if<parser::Designator>(&object.u)}) { + return GetLastName(*desg).source; + } + return std::nullopt; +} + const Symbol *GetArgumentSymbol(const parser::OmpArgument &argument) { if (auto *locator{std::get_if<parser::OmpLocator>(&argument.u)}) { if (auto *object{std::get_if<parser::OmpObject>(&locator->u)}) { @@ -114,14 +124,12 @@ const Symbol *GetArgumentSymbol(const parser::OmpArgument &argument) { return nullptr; } -std::optional<parser::CharBlock> GetObjectSource( - const parser::OmpObject &object) { - if (auto *name{std::get_if<parser::Name>(&object.u)}) { - return name->source; - } else if (auto *desg{std::get_if<parser::Designator>(&object.u)}) { - return GetLastName(*desg).source; +const parser::OmpObject *GetArgumentObject( + const parser::OmpArgument &argument) { + if (auto *locator{std::get_if<parser::OmpLocator>(&argument.u)}) { + return std::get_if<parser::OmpObject>(&locator->u); } - return std::nullopt; + return nullptr; } bool IsCommonBlock(const Symbol &sym) { diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp index 28c74b8c1908b..c178151b08248 100644 --- a/flang/lib/Semantics/resolve-directives.cpp +++ b/flang/lib/Semantics/resolve-directives.cpp @@ -2344,9 +2344,14 @@ bool OmpAttributeVisitor::Pre( } bool OmpAttributeVisitor::Pre(const parser::OpenMPThreadprivate &x) { - PushContext(x.source, llvm::omp::Directive::OMPD_threadprivate); - const auto &list{std::get<parser::OmpObjectList>(x.t)}; - ResolveOmpObjectList(list, Symbol::Flag::OmpThreadprivate); + const parser::OmpDirectiveName &dirName{x.v.DirName()}; + PushContext(dirName.source, dirName.v); + + for (const parser::OmpArgument &arg : x.v.Arguments().v) { + if (auto *object{omp::GetArgumentObject(arg)}) { + ResolveOmpObject(*object, Symbol::Flag::OmpThreadprivate); + } + } return true; } _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
