[flang-commits] [flang] [llvm] [flang][OpenMP] Generalize isOpenMPPrivatizingConstruct (PR #148654)
Krzysztof Parzyszek via flang-commits
flang-commits at lists.llvm.org
Thu Jul 17 10:12:51 PDT 2025
https://github.com/kparzysz updated https://github.com/llvm/llvm-project/pull/148654
>From 76b1c6594d1e73f7d4a7011db3247a0c37b51339 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
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<C, H>::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<unsigned> getOpenMPVersions();
>From 82829a6f51fd96f720a64255442d96133a09b3dc Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
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 <Enum>::First_ and <Enum>::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<tdl::Association> {
+// CHECK-NEXT: static constexpr bool is_iterable = true;
+// CHECK-NEXT: };
+// CHECK-EMPTY:
+// CHECK-NEXT: template <> struct enum_iteration_traits<tdl::Category> {
+// CHECK-NEXT: static constexpr bool is_iterable = true;
+// CHECK-NEXT: };
+// CHECK-EMPTY:
+// CHECK-NEXT: template <> struct enum_iteration_traits<tdl::Directive> {
+// CHECK-NEXT: static constexpr bool is_iterable = true;
+// CHECK-NEXT: };
+// CHECK-EMPTY:
+// CHECK-NEXT: template <> struct enum_iteration_traits<tdl::Clause> {
+// 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<tdl::Association> {
+// CHECK-NEXT: static constexpr bool is_iterable = true;
+// CHECK-NEXT: };
+// CHECK-EMPTY:
+// CHECK-NEXT: template <> struct enum_iteration_traits<tdl::Category> {
+// CHECK-NEXT: static constexpr bool is_iterable = true;
+// CHECK-NEXT: };
+// CHECK-EMPTY:
+// CHECK-NEXT: template <> struct enum_iteration_traits<tdl::Directive> {
+// CHECK-NEXT: static constexpr bool is_iterable = true;
+// CHECK-NEXT: };
+// CHECK-EMPTY:
+// CHECK-NEXT: template <> struct enum_iteration_traits<tdl::Clause> {
+// 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<omp::Directive> {
- static constexpr bool is_iterable = true;
-};
-} // namespace llvm
-
// Test tokenizing.
class Tokenize : public testing::TestWithParam<omp::Directive> {};
@@ -87,12 +81,10 @@ getParamName1(const testing::TestParamInfo<Tokenize::ParamType> &Info) {
return prepareParamName(Name);
}
-INSTANTIATE_TEST_SUITE_P(
- DirectiveNameParserTest, Tokenize,
- testing::ValuesIn(
- llvm::enum_seq(static_cast<omp::Directive>(0),
- static_cast<omp::Directive>(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<ParseValid::ParamType> &Info) {
INSTANTIATE_TEST_SUITE_P(
DirectiveNameParserTest, ParseValid,
- testing::Combine(testing::ValuesIn(llvm::enum_seq(
- static_cast<omp::Directive>(0),
- static_cast<omp::Directive>(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<const Record *> 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 <Krzysztof.Parzyszek at amd.com>
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<mlir::Value> 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<parser::OmpBlockDirective>(
- std::get<parser::OmpBeginBlockDirective>(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<parser::OmpLoopDirective>(
- std::get<parser::OmpBeginLoopDirective>(c.t).t)
- .v;
- },
- [](const parser::OpenMPSectionConstruct &c) {
- return llvm::omp::OMPD_section;
- },
- [](const parser::OpenMPSectionsConstruct &c) {
- return std::get<parser::OmpSectionsDirective>(
- std::get<parser::OmpBeginSectionsDirective>(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<parser::OmpBlockDirective>(
+ std::get<parser::OmpBeginBlockDirective>(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<parser::OmpLoopDirective>(
+ std::get<parser::OmpBeginLoopDirective>(c.t).t)
+ .v;
+ },
+ [](const parser::OpenMPSectionConstruct &c) {
+ return llvm::omp::OMPD_section;
+ },
+ [](const parser::OpenMPSectionsConstruct &c) {
+ return std::get<parser::OmpSectionsDirective>(
+ std::get<parser::OmpBeginSectionsDirective>(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<omp::Clause> &clauses,
mlir::omp::LoopRelatedClauseOps &result,
llvm::SmallVectorImpl<const semantics::Symbol *> &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 <Krzysztof.Parzyszek at amd.com>
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<llvm::omp::Directive, 16> &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>(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<decltype(s)>;
- return std::is_same_v<BareS, parser::OpenMPBlockConstruct> ||
- std::is_same_v<BareS, parser::OpenMPLoopConstruct> ||
- std::is_same_v<BareS, parser::OpenMPSectionsConstruct>;
- },
- omp.u);
+ const parser::OpenMPConstruct &omp, unsigned version) {
+ static llvm::SmallSet<llvm::omp::Directive, 16> 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<decltype(s)>;
if constexpr (std::is_same_v<BareS, parser::OpenMPConstruct>) {
- 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 <typename T>
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<i32>)
+!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<i32>) {
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;
More information about the flang-commits
mailing list