From 76b1c6594d1e73f7d4a7011db3247a0c37b51339 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Fri, 11 Jul 2025 07:45:34 -0500 Subject: [PATCH 1/4] [Frontend][OpenMP] Move isPrivatizingClause to OMP.h, NFC --- .../Frontend/OpenMP/ConstructDecompositionT.h | 18 +----------------- llvm/include/llvm/Frontend/OpenMP/OMP.h | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/llvm/include/llvm/Frontend/OpenMP/ConstructDecompositionT.h b/llvm/include/llvm/Frontend/OpenMP/ConstructDecompositionT.h index cdc80c88b7425..611bfe3f8aced 100644 --- a/llvm/include/llvm/Frontend/OpenMP/ConstructDecompositionT.h +++ b/llvm/include/llvm/Frontend/OpenMP/ConstructDecompositionT.h @@ -795,25 +795,9 @@ bool ConstructDecompositionT::applyClause( // assigned to which leaf constructs. // [5.2:340:33] - auto canMakePrivateCopy = [](llvm::omp::Clause id) { - switch (id) { - // Clauses with "privatization" property: - case llvm::omp::Clause::OMPC_firstprivate: - case llvm::omp::Clause::OMPC_in_reduction: - case llvm::omp::Clause::OMPC_lastprivate: - case llvm::omp::Clause::OMPC_linear: - case llvm::omp::Clause::OMPC_private: - case llvm::omp::Clause::OMPC_reduction: - case llvm::omp::Clause::OMPC_task_reduction: - return true; - default: - return false; - } - }; - bool applied = applyIf(node, [&](const auto &leaf) { return llvm::any_of(leaf.clauses, [&](const ClauseTy *n) { - return canMakePrivateCopy(n->id); + return llvm::omp::isPrivatizingClause(n->id); }); }); diff --git a/llvm/include/llvm/Frontend/OpenMP/OMP.h b/llvm/include/llvm/Frontend/OpenMP/OMP.h index 35dafc6d246f0..d44c33301bde7 100644 --- a/llvm/include/llvm/Frontend/OpenMP/OMP.h +++ b/llvm/include/llvm/Frontend/OpenMP/OMP.h @@ -48,6 +48,22 @@ static constexpr inline bool canHaveIterator(Clause C) { } } +// Can clause C create a private copy of a variable. +static constexpr inline bool isPrivatizingClause(Clause C) { + switch (C) { + case OMPC_firstprivate: + case OMPC_in_reduction: + case OMPC_lastprivate: + case OMPC_linear: + case OMPC_private: + case OMPC_reduction: + case OMPC_task_reduction: + return true; + default: + return false; + } +} + static constexpr unsigned FallbackVersion = 52; LLVM_ABI ArrayRef getOpenMPVersions(); From 82829a6f51fd96f720a64255442d96133a09b3dc Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Fri, 11 Jul 2025 13:28:02 -0500 Subject: [PATCH 2/4] [utils][TableGen] Make some non-bitmask enums iterable Additionally, add sentinel values ::First_ and ::Last_ to each one of those enums. This will allow using `enum_seq_inclusive` to generate the list of enum-typed values of any generated scoped (non-bitmask) enum. --- llvm/test/TableGen/directive1.td | 25 +++++++++++++++++++ llvm/test/TableGen/directive2.td | 25 +++++++++++++++++++ .../OpenMPDirectiveNameParserTest.cpp | 21 +++++----------- .../utils/TableGen/Basic/DirectiveEmitter.cpp | 20 +++++++++++++-- 4 files changed, 74 insertions(+), 17 deletions(-) diff --git a/llvm/test/TableGen/directive1.td b/llvm/test/TableGen/directive1.td index 1d2bd51204e4f..3eda077eeabf7 100644 --- a/llvm/test/TableGen/directive1.td +++ b/llvm/test/TableGen/directive1.td @@ -53,6 +53,7 @@ def TDL_DirA : Directive<[Spelling<"dira">]> { // CHECK-EMPTY: // CHECK-NEXT: #include "llvm/ADT/ArrayRef.h" // CHECK-NEXT: #include "llvm/ADT/BitmaskEnum.h" +// CHECK-NEXT: #include "llvm/ADT/Sequence.h" // CHECK-NEXT: #include "llvm/ADT/StringRef.h" // CHECK-NEXT: #include "llvm/Frontend/Directive/Spelling.h" // CHECK-NEXT: #include "llvm/Support/Compiler.h" @@ -66,22 +67,26 @@ def TDL_DirA : Directive<[Spelling<"dira">]> { // CHECK-EMPTY: // CHECK-NEXT: enum class Association { // CHECK-NEXT: Block, +// CHECK-NEXT: First_ = Block, // CHECK-NEXT: Declaration, // CHECK-NEXT: Delimited, // CHECK-NEXT: Loop, // CHECK-NEXT: None, // CHECK-NEXT: Separating, +// CHECK-NEXT: Last_ = Separating, // CHECK-NEXT: }; // CHECK-EMPTY: // CHECK-NEXT: static constexpr std::size_t Association_enumSize = 6; // CHECK-EMPTY: // CHECK-NEXT: enum class Category { // CHECK-NEXT: Declarative, +// CHECK-NEXT: First_ = Declarative, // CHECK-NEXT: Executable, // CHECK-NEXT: Informational, // CHECK-NEXT: Meta, // CHECK-NEXT: Subsidiary, // CHECK-NEXT: Utility, +// CHECK-NEXT: Last_ = Utility, // CHECK-NEXT: }; // CHECK-EMPTY: // CHECK-NEXT: static constexpr std::size_t Category_enumSize = 6; @@ -96,6 +101,8 @@ def TDL_DirA : Directive<[Spelling<"dira">]> { // CHECK-EMPTY: // CHECK-NEXT: enum class Directive { // CHECK-NEXT: TDLD_dira, +// CHECK-NEXT: First_ = TDLD_dira, +// CHECK-NEXT: Last_ = TDLD_dira, // CHECK-NEXT: }; // CHECK-EMPTY: // CHECK-NEXT: static constexpr std::size_t Directive_enumSize = 1; @@ -104,8 +111,10 @@ def TDL_DirA : Directive<[Spelling<"dira">]> { // CHECK-EMPTY: // CHECK-NEXT: enum class Clause { // CHECK-NEXT: TDLC_clausea, +// CHECK-NEXT: First_ = TDLC_clausea, // CHECK-NEXT: TDLC_clauseb, // CHECK-NEXT: TDLC_clausec, +// CHECK-NEXT: Last_ = TDLC_clausec, // CHECK-NEXT: }; // CHECK-EMPTY: // CHECK-NEXT: static constexpr std::size_t Clause_enumSize = 3; @@ -151,6 +160,22 @@ def TDL_DirA : Directive<[Spelling<"dira">]> { // CHECK-NEXT: LLVM_ABI StringRef getTdlAKindName(AKind x); // CHECK-EMPTY: // CHECK-NEXT: } // namespace tdl +// CHECK-EMPTY: +// CHECK-NEXT: template <> struct enum_iteration_traits { +// CHECK-NEXT: static constexpr bool is_iterable = true; +// CHECK-NEXT: }; +// CHECK-EMPTY: +// CHECK-NEXT: template <> struct enum_iteration_traits { +// CHECK-NEXT: static constexpr bool is_iterable = true; +// CHECK-NEXT: }; +// CHECK-EMPTY: +// CHECK-NEXT: template <> struct enum_iteration_traits { +// CHECK-NEXT: static constexpr bool is_iterable = true; +// CHECK-NEXT: }; +// CHECK-EMPTY: +// CHECK-NEXT: template <> struct enum_iteration_traits { +// CHECK-NEXT: static constexpr bool is_iterable = true; +// CHECK-NEXT: }; // CHECK-NEXT: } // namespace llvm // CHECK-NEXT: #endif // LLVM_Tdl_INC diff --git a/llvm/test/TableGen/directive2.td b/llvm/test/TableGen/directive2.td index 3a64bb3900a31..a25197c3efd93 100644 --- a/llvm/test/TableGen/directive2.td +++ b/llvm/test/TableGen/directive2.td @@ -46,6 +46,7 @@ def TDL_DirA : Directive<[Spelling<"dira">]> { // CHECK-NEXT: #define LLVM_Tdl_INC // CHECK-EMPTY: // CHECK-NEXT: #include "llvm/ADT/ArrayRef.h" +// CHECK-NEXT: #include "llvm/ADT/Sequence.h" // CHECK-NEXT: #include "llvm/ADT/StringRef.h" // CHECK-NEXT: #include "llvm/Frontend/Directive/Spelling.h" // CHECK-NEXT: #include "llvm/Support/Compiler.h" @@ -57,22 +58,26 @@ def TDL_DirA : Directive<[Spelling<"dira">]> { // CHECK-EMPTY: // CHECK-NEXT: enum class Association { // CHECK-NEXT: Block, +// CHECK-NEXT: First_ = Block, // CHECK-NEXT: Declaration, // CHECK-NEXT: Delimited, // CHECK-NEXT: Loop, // CHECK-NEXT: None, // CHECK-NEXT: Separating, +// CHECK-NEXT: Last_ = Separating, // CHECK-NEXT: }; // CHECK-EMPTY: // CHECK-NEXT: static constexpr std::size_t Association_enumSize = 6; // CHECK-EMPTY: // CHECK-NEXT: enum class Category { // CHECK-NEXT: Declarative, +// CHECK-NEXT: First_ = Declarative, // CHECK-NEXT: Executable, // CHECK-NEXT: Informational, // CHECK-NEXT: Meta, // CHECK-NEXT: Subsidiary, // CHECK-NEXT: Utility, +// CHECK-NEXT: Last_ = Utility, // CHECK-NEXT: }; // CHECK-EMPTY: // CHECK-NEXT: static constexpr std::size_t Category_enumSize = 6; @@ -87,15 +92,19 @@ def TDL_DirA : Directive<[Spelling<"dira">]> { // CHECK-EMPTY: // CHECK-NEXT: enum class Directive { // CHECK-NEXT: TDLD_dira, +// CHECK-NEXT: First_ = TDLD_dira, +// CHECK-NEXT: Last_ = TDLD_dira, // CHECK-NEXT: }; // CHECK-EMPTY: // CHECK-NEXT: static constexpr std::size_t Directive_enumSize = 1; // CHECK-EMPTY: // CHECK-NEXT: enum class Clause { // CHECK-NEXT: TDLC_clausea, +// CHECK-NEXT: First_ = TDLC_clausea, // CHECK-NEXT: TDLC_clauseb, // CHECK-NEXT: TDLC_clausec, // CHECK-NEXT: TDLC_claused, +// CHECK-NEXT: Last_ = TDLC_claused, // CHECK-NEXT: }; // CHECK-EMPTY: // CHECK-NEXT: static constexpr std::size_t Clause_enumSize = 4; @@ -124,6 +133,22 @@ def TDL_DirA : Directive<[Spelling<"dira">]> { // CHECK-NEXT: LLVM_ABI Category getDirectiveCategory(Directive D); // CHECK-NEXT: LLVM_ABI SourceLanguage getDirectiveLanguages(Directive D); // CHECK-NEXT: } // namespace tdl +// CHECK-EMPTY: +// CHECK-NEXT: template <> struct enum_iteration_traits { +// CHECK-NEXT: static constexpr bool is_iterable = true; +// CHECK-NEXT: }; +// CHECK-EMPTY: +// CHECK-NEXT: template <> struct enum_iteration_traits { +// CHECK-NEXT: static constexpr bool is_iterable = true; +// CHECK-NEXT: }; +// CHECK-EMPTY: +// CHECK-NEXT: template <> struct enum_iteration_traits { +// CHECK-NEXT: static constexpr bool is_iterable = true; +// CHECK-NEXT: }; +// CHECK-EMPTY: +// CHECK-NEXT: template <> struct enum_iteration_traits { +// CHECK-NEXT: static constexpr bool is_iterable = true; +// CHECK-NEXT: }; // CHECK-NEXT: } // namespace llvm // CHECK-NEXT: #endif // LLVM_Tdl_INC diff --git a/llvm/unittests/Frontend/OpenMPDirectiveNameParserTest.cpp b/llvm/unittests/Frontend/OpenMPDirectiveNameParserTest.cpp index 0363a08cc0f03..10329820bef76 100644 --- a/llvm/unittests/Frontend/OpenMPDirectiveNameParserTest.cpp +++ b/llvm/unittests/Frontend/OpenMPDirectiveNameParserTest.cpp @@ -48,12 +48,6 @@ static std::string &prepareParamName(std::string &Name) { return Name; } -namespace llvm { -template <> struct enum_iteration_traits { - static constexpr bool is_iterable = true; -}; -} // namespace llvm - // Test tokenizing. class Tokenize : public testing::TestWithParam {}; @@ -87,12 +81,10 @@ getParamName1(const testing::TestParamInfo &Info) { return prepareParamName(Name); } -INSTANTIATE_TEST_SUITE_P( - DirectiveNameParserTest, Tokenize, - testing::ValuesIn( - llvm::enum_seq(static_cast(0), - static_cast(omp::Directive_enumSize))), - getParamName1); +INSTANTIATE_TEST_SUITE_P(DirectiveNameParserTest, Tokenize, + testing::ValuesIn(llvm::enum_seq_inclusive( + omp::Directive::First_, omp::Directive::Last_)), + getParamName1); // Test parsing of valid names. @@ -131,9 +123,8 @@ getParamName2(const testing::TestParamInfo &Info) { INSTANTIATE_TEST_SUITE_P( DirectiveNameParserTest, ParseValid, - testing::Combine(testing::ValuesIn(llvm::enum_seq( - static_cast(0), - static_cast(omp::Directive_enumSize))), + testing::Combine(testing::ValuesIn(llvm::enum_seq_inclusive( + omp::Directive::First_, omp::Directive::Last_)), testing::ValuesIn(omp::getOpenMPVersions())), getParamName2); diff --git a/llvm/utils/TableGen/Basic/DirectiveEmitter.cpp b/llvm/utils/TableGen/Basic/DirectiveEmitter.cpp index 177eecebce9a5..b48b9f6b7c41c 100644 --- a/llvm/utils/TableGen/Basic/DirectiveEmitter.cpp +++ b/llvm/utils/TableGen/Basic/DirectiveEmitter.cpp @@ -106,9 +106,15 @@ static void generateEnumClass(ArrayRef Records, raw_ostream &OS, bool ExportEnums) { OS << "\n"; OS << "enum class " << Enum << " {\n"; - for (const Record *R : Records) { - OS << " " << getIdentifierName(R, Prefix) << ",\n"; + std::string N; + for (auto [I, R] : llvm::enumerate(Records)) { + N = getIdentifierName(R, Prefix); + OS << " " << N << ",\n"; + // Make the sentinel names less likely to conflict with actual names... + if (I == 0) + OS << " First_ = " << N << ",\n"; } + OS << " Last_ = " << N << ",\n"; OS << "};\n"; OS << "\n"; OS << "static constexpr std::size_t " << Enum @@ -282,6 +288,7 @@ static void emitDirectivesDecl(const RecordKeeper &Records, raw_ostream &OS) { if (DirLang.hasEnableBitmaskEnumInNamespace()) OS << "#include \"llvm/ADT/BitmaskEnum.h\"\n"; + OS << "#include \"llvm/ADT/Sequence.h\"\n"; OS << "#include \"llvm/ADT/StringRef.h\"\n"; OS << "#include \"llvm/Frontend/Directive/Spelling.h\"\n"; OS << "#include \"llvm/Support/Compiler.h\"\n"; @@ -375,6 +382,15 @@ static void emitDirectivesDecl(const RecordKeeper &Records, raw_ostream &OS) { for (auto Ns : reverse(Namespaces)) OS << "} // namespace " << Ns << "\n"; + // These specializations need to be in ::llvm. + for (StringRef Enum : {"Association", "Category", "Directive", "Clause"}) { + OS << "\n"; + OS << "template <> struct enum_iteration_traits<" + << DirLang.getCppNamespace() << "::" << Enum << "> {\n"; + OS << " static constexpr bool is_iterable = true;\n"; + OS << "};\n"; + } + OS << "} // namespace llvm\n"; OS << "#endif // LLVM_" << Lang << "_INC\n"; From d3ed6f948468c4d27c5ac9999436612e650f9460 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Mon, 14 Jul 2025 10:18:07 -0500 Subject: [PATCH 3/4] [flang][OpenMP] Move extractOmpDirective to Utils.cpp, NFC --- flang/lib/Lower/OpenMP/OpenMP.cpp | 84 ------------------------------- flang/lib/Lower/OpenMP/Utils.cpp | 84 +++++++++++++++++++++++++++++++ flang/lib/Lower/OpenMP/Utils.h | 3 ++ 3 files changed, 87 insertions(+), 84 deletions(-) diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp index 4458f62eea95a..fcb20fdf187ff 100644 --- a/flang/lib/Lower/OpenMP/OpenMP.cpp +++ b/flang/lib/Lower/OpenMP/OpenMP.cpp @@ -372,90 +372,6 @@ extractMappedBaseValues(llvm::ArrayRef vars, }); } -/// Get the directive enumeration value corresponding to the given OpenMP -/// construct PFT node. -llvm::omp::Directive -extractOmpDirective(const parser::OpenMPConstruct &ompConstruct) { - return common::visit( - common::visitors{ - [](const parser::OpenMPAllocatorsConstruct &c) { - return llvm::omp::OMPD_allocators; - }, - [](const parser::OpenMPAssumeConstruct &c) { - return llvm::omp::OMPD_assume; - }, - [](const parser::OpenMPAtomicConstruct &c) { - return llvm::omp::OMPD_atomic; - }, - [](const parser::OpenMPBlockConstruct &c) { - return std::get( - std::get(c.t).t) - .v; - }, - [](const parser::OpenMPCriticalConstruct &c) { - return llvm::omp::OMPD_critical; - }, - [](const parser::OpenMPDeclarativeAllocate &c) { - return llvm::omp::OMPD_allocate; - }, - [](const parser::OpenMPDispatchConstruct &c) { - return llvm::omp::OMPD_dispatch; - }, - [](const parser::OpenMPExecutableAllocate &c) { - return llvm::omp::OMPD_allocate; - }, - [](const parser::OpenMPLoopConstruct &c) { - return std::get( - std::get(c.t).t) - .v; - }, - [](const parser::OpenMPSectionConstruct &c) { - return llvm::omp::OMPD_section; - }, - [](const parser::OpenMPSectionsConstruct &c) { - return std::get( - std::get(c.t).t) - .v; - }, - [](const parser::OpenMPStandaloneConstruct &c) { - return common::visit( - common::visitors{ - [](const parser::OpenMPSimpleStandaloneConstruct &c) { - return c.v.DirId(); - }, - [](const parser::OpenMPFlushConstruct &c) { - return llvm::omp::OMPD_flush; - }, - [](const parser::OpenMPCancelConstruct &c) { - return llvm::omp::OMPD_cancel; - }, - [](const parser::OpenMPCancellationPointConstruct &c) { - return llvm::omp::OMPD_cancellation_point; - }, - [](const parser::OmpMetadirectiveDirective &c) { - return llvm::omp::OMPD_metadirective; - }, - [](const parser::OpenMPDepobjConstruct &c) { - return llvm::omp::OMPD_depobj; - }, - [](const parser::OpenMPInteropConstruct &c) { - return llvm::omp::OMPD_interop; - }}, - c.u); - }, - [](const parser::OpenMPUtilityConstruct &c) { - return common::visit( - common::visitors{[](const parser::OmpErrorDirective &c) { - return llvm::omp::OMPD_error; - }, - [](const parser::OmpNothingDirective &c) { - return llvm::omp::OMPD_nothing; - }}, - c.u); - }}, - ompConstruct.u); -} - /// Populate the global \see hostEvalInfo after processing clauses for the given /// \p eval OpenMP target construct, or nested constructs, if these must be /// evaluated outside of the target region per the spec. diff --git a/flang/lib/Lower/OpenMP/Utils.cpp b/flang/lib/Lower/OpenMP/Utils.cpp index 2e53f01f1da6a..b194150c0f7f0 100644 --- a/flang/lib/Lower/OpenMP/Utils.cpp +++ b/flang/lib/Lower/OpenMP/Utils.cpp @@ -661,6 +661,90 @@ bool collectLoopRelatedInfo( return found; } + +/// Get the directive enumeration value corresponding to the given OpenMP +/// construct PFT node. +llvm::omp::Directive +extractOmpDirective(const parser::OpenMPConstruct &ompConstruct) { + return common::visit( + common::visitors{ + [](const parser::OpenMPAllocatorsConstruct &c) { + return llvm::omp::OMPD_allocators; + }, + [](const parser::OpenMPAssumeConstruct &c) { + return llvm::omp::OMPD_assume; + }, + [](const parser::OpenMPAtomicConstruct &c) { + return llvm::omp::OMPD_atomic; + }, + [](const parser::OpenMPBlockConstruct &c) { + return std::get( + std::get(c.t).t) + .v; + }, + [](const parser::OpenMPCriticalConstruct &c) { + return llvm::omp::OMPD_critical; + }, + [](const parser::OpenMPDeclarativeAllocate &c) { + return llvm::omp::OMPD_allocate; + }, + [](const parser::OpenMPDispatchConstruct &c) { + return llvm::omp::OMPD_dispatch; + }, + [](const parser::OpenMPExecutableAllocate &c) { + return llvm::omp::OMPD_allocate; + }, + [](const parser::OpenMPLoopConstruct &c) { + return std::get( + std::get(c.t).t) + .v; + }, + [](const parser::OpenMPSectionConstruct &c) { + return llvm::omp::OMPD_section; + }, + [](const parser::OpenMPSectionsConstruct &c) { + return std::get( + std::get(c.t).t) + .v; + }, + [](const parser::OpenMPStandaloneConstruct &c) { + return common::visit( + common::visitors{ + [](const parser::OpenMPSimpleStandaloneConstruct &c) { + return c.v.DirId(); + }, + [](const parser::OpenMPFlushConstruct &c) { + return llvm::omp::OMPD_flush; + }, + [](const parser::OpenMPCancelConstruct &c) { + return llvm::omp::OMPD_cancel; + }, + [](const parser::OpenMPCancellationPointConstruct &c) { + return llvm::omp::OMPD_cancellation_point; + }, + [](const parser::OmpMetadirectiveDirective &c) { + return llvm::omp::OMPD_metadirective; + }, + [](const parser::OpenMPDepobjConstruct &c) { + return llvm::omp::OMPD_depobj; + }, + [](const parser::OpenMPInteropConstruct &c) { + return llvm::omp::OMPD_interop; + }}, + c.u); + }, + [](const parser::OpenMPUtilityConstruct &c) { + return common::visit( + common::visitors{[](const parser::OmpErrorDirective &c) { + return llvm::omp::OMPD_error; + }, + [](const parser::OmpNothingDirective &c) { + return llvm::omp::OMPD_nothing; + }}, + c.u); + }}, + ompConstruct.u); +} } // namespace omp } // namespace lower } // namespace Fortran diff --git a/flang/lib/Lower/OpenMP/Utils.h b/flang/lib/Lower/OpenMP/Utils.h index 1526bd4e90233..8e3ad5c3452e2 100644 --- a/flang/lib/Lower/OpenMP/Utils.h +++ b/flang/lib/Lower/OpenMP/Utils.h @@ -166,6 +166,9 @@ bool collectLoopRelatedInfo( lower::pft::Evaluation &eval, const omp::List &clauses, mlir::omp::LoopRelatedClauseOps &result, llvm::SmallVectorImpl &iv); + +llvm::omp::Directive +extractOmpDirective(const parser::OpenMPConstruct &ompConstruct); } // namespace omp } // namespace lower } // namespace Fortran From 4f58a01c96812fdb8086a9c9ad35f260451af8dc Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Fri, 11 Jul 2025 08:21:12 -0500 Subject: [PATCH 4/4] [flang][OpenMP] Generalize isOpenMPPrivatizingConstruct Instead of treating all block and all loop constructs as privatizing, actually check if the construct allows a privatizing clause. --- .../lib/Lower/OpenMP/DataSharingProcessor.cpp | 57 +++++++++++++++---- flang/lib/Lower/OpenMP/DataSharingProcessor.h | 12 +++- flang/test/Lower/OpenMP/taskgroup02.f90 | 5 +- llvm/include/llvm/Frontend/OpenMP/OMP.h | 4 ++ 4 files changed, 61 insertions(+), 17 deletions(-) diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp index 3fae3f3a0ddfd..675a58e4f35a1 100644 --- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp +++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp @@ -26,6 +26,8 @@ #include "flang/Optimizer/HLFIR/HLFIROps.h" #include "flang/Semantics/attr.h" #include "flang/Semantics/tools.h" +#include "llvm/ADT/Sequence.h" +#include "llvm/ADT/SmallSet.h" namespace Fortran { namespace lower { @@ -49,7 +51,7 @@ DataSharingProcessor::DataSharingProcessor( firOpBuilder(converter.getFirOpBuilder()), clauses(clauses), eval(eval), shouldCollectPreDeterminedSymbols(shouldCollectPreDeterminedSymbols), useDelayedPrivatization(useDelayedPrivatization), symTable(symTable), - visitor() { + visitor(semaCtx) { eval.visit([&](const auto &functionParserNode) { parser::Walk(functionParserNode, visitor); }); @@ -424,24 +426,55 @@ getSource(const semantics::SemanticsContext &semaCtx, return source; } +static void collectPrivatizingConstructs( + llvm::SmallSet &constructs, unsigned version) { + using Clause = llvm::omp::Clause; + using Directive = llvm::omp::Directive; + + static const Clause privatizingClauses[] = { + Clause::OMPC_private, + Clause::OMPC_lastprivate, + Clause::OMPC_firstprivate, + Clause::OMPC_in_reduction, + Clause::OMPC_reduction, + Clause::OMPC_linear, + // TODO: Clause::OMPC_induction, + Clause::OMPC_task_reduction, + Clause::OMPC_detach, + Clause::OMPC_use_device_ptr, + Clause::OMPC_is_device_ptr, + }; + + for (auto dir : llvm::enum_seq_inclusive(Directive::First_, + Directive::Last_)) { + bool allowsPrivatizing = llvm::any_of(privatizingClauses, [&](Clause cls) { + return llvm::omp::isAllowedClauseForDirective(dir, cls, version); + }); + if (allowsPrivatizing) + constructs.insert(dir); + } +} + bool DataSharingProcessor::isOpenMPPrivatizingConstruct( - const parser::OpenMPConstruct &omp) { - return common::visit( - [](auto &&s) { - using BareS = llvm::remove_cvref_t; - return std::is_same_v || - std::is_same_v || - std::is_same_v; - }, - omp.u); + const parser::OpenMPConstruct &omp, unsigned version) { + static llvm::SmallSet privatizing; + [[maybe_unused]] static bool init = + (collectPrivatizingConstructs(privatizing, version), true); + + // As of OpenMP 6.0, privatizing constructs (with the test being if they + // allow a privatizing clause) are: dispatch, distribute, do, for, loop, + // parallel, scope, sections, simd, single, target, target_data, task, + // taskgroup, taskloop, and teams. + return llvm::is_contained(privatizing, extractOmpDirective(omp)); } bool DataSharingProcessor::isOpenMPPrivatizingEvaluation( const pft::Evaluation &eval) const { - return eval.visit([](auto &&s) { + unsigned version = semaCtx.langOptions().OpenMPVersion; + return eval.visit([=](auto &&s) { using BareS = llvm::remove_cvref_t; if constexpr (std::is_same_v) { - return isOpenMPPrivatizingConstruct(s); + return isOpenMPPrivatizingConstruct(s, version); } else { return false; } diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.h b/flang/lib/Lower/OpenMP/DataSharingProcessor.h index ee2fc70d2e673..bc422f410403a 100644 --- a/flang/lib/Lower/OpenMP/DataSharingProcessor.h +++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.h @@ -36,6 +36,8 @@ class DataSharingProcessor { /// at any point in time. This is used to track Symbol definition scopes in /// order to tell which OMP scope defined vs. references a certain Symbol. struct OMPConstructSymbolVisitor { + OMPConstructSymbolVisitor(semantics::SemanticsContext &ctx) + : version(ctx.langOptions().OpenMPVersion) {} template bool Pre(const T &) { return true; @@ -45,13 +47,13 @@ class DataSharingProcessor { bool Pre(const parser::OpenMPConstruct &omp) { // Skip constructs that may not have privatizations. - if (isOpenMPPrivatizingConstruct(omp)) + if (isOpenMPPrivatizingConstruct(omp, version)) constructs.push_back(&omp); return true; } void Post(const parser::OpenMPConstruct &omp) { - if (isOpenMPPrivatizingConstruct(omp)) + if (isOpenMPPrivatizingConstruct(omp, version)) constructs.pop_back(); } @@ -68,6 +70,9 @@ class DataSharingProcessor { /// construct that defines symbol. bool isSymbolDefineBy(const semantics::Symbol *symbol, lower::pft::Evaluation &eval) const; + + private: + unsigned version; }; mlir::OpBuilder::InsertPoint lastPrivIP; @@ -115,7 +120,8 @@ class DataSharingProcessor { mlir::OpBuilder::InsertPoint *lastPrivIP); void insertDeallocs(); - static bool isOpenMPPrivatizingConstruct(const parser::OpenMPConstruct &omp); + static bool isOpenMPPrivatizingConstruct(const parser::OpenMPConstruct &omp, + unsigned version); bool isOpenMPPrivatizingEvaluation(const pft::Evaluation &eval) const; public: diff --git a/flang/test/Lower/OpenMP/taskgroup02.f90 b/flang/test/Lower/OpenMP/taskgroup02.f90 index 1e996a030c23a..4c470b7aa82d1 100644 --- a/flang/test/Lower/OpenMP/taskgroup02.f90 +++ b/flang/test/Lower/OpenMP/taskgroup02.f90 @@ -3,8 +3,9 @@ ! Check that variables are not privatized twice when TASKGROUP is used. !CHECK-LABEL: func.func @_QPsub() { -!CHECK: omp.parallel { -!CHECK: %[[PAR_I:.*]]:2 = hlfir.declare %{{.*}} {uniq_name = "_QFsubEi"} +!CHECK: omp.parallel private(@_QFsubEi_private_i32 %[[SUB_I:.*]]#0 -> %[[ARG:.*]] : !fir.ref) +!CHECK: %[[ALLOCA:.*]] = fir.alloca i32 +!CHECK: %[[PAR_I:.*]]:2 = hlfir.declare %[[ALLOCA]] {uniq_name = "_QFsubEi"} !CHECK: omp.master { !CHECK: omp.taskgroup { !CHECK-NEXT: omp.task private(@_QFsubEi_firstprivate_i32 %[[PAR_I]]#0 -> %[[TASK_I:.*]] : !fir.ref) { diff --git a/llvm/include/llvm/Frontend/OpenMP/OMP.h b/llvm/include/llvm/Frontend/OpenMP/OMP.h index d44c33301bde7..9d0a55432e1ae 100644 --- a/llvm/include/llvm/Frontend/OpenMP/OMP.h +++ b/llvm/include/llvm/Frontend/OpenMP/OMP.h @@ -51,13 +51,17 @@ static constexpr inline bool canHaveIterator(Clause C) { // Can clause C create a private copy of a variable. static constexpr inline bool isPrivatizingClause(Clause C) { switch (C) { + case OMPC_detach: case OMPC_firstprivate: + // TODO case OMPC_induction: case OMPC_in_reduction: + case OMPC_is_device_ptr: case OMPC_lastprivate: case OMPC_linear: case OMPC_private: case OMPC_reduction: case OMPC_task_reduction: + case OMPC_use_device_ptr: return true; default: return false;