[llvm] [Frontend][OpenMP] Privatizing clauses in construct decomposition (PR #92176)
Krzysztof Parzyszek via llvm-commits
llvm-commits at lists.llvm.org
Wed May 15 06:38:23 PDT 2024
https://github.com/kparzysz updated https://github.com/llvm/llvm-project/pull/92176
>From eb89b138442c1f3db9b6cf62adb46f5f4f881a29 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Tue, 14 May 2024 15:52:39 -0500
Subject: [PATCH 1/4] [Frontend][OpenMP] Privatizing clauses in construct
decomposition
Add remaining clauses with the "privatizing" property to construct
decomposition, specifically to the part handling the `allocate`
clause.
---
llvm/include/llvm/Frontend/OpenMP/ConstructDecompositionT.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/llvm/include/llvm/Frontend/OpenMP/ConstructDecompositionT.h b/llvm/include/llvm/Frontend/OpenMP/ConstructDecompositionT.h
index 5f12c62b832fc..02c88a58e0993 100644
--- a/llvm/include/llvm/Frontend/OpenMP/ConstructDecompositionT.h
+++ b/llvm/include/llvm/Frontend/OpenMP/ConstructDecompositionT.h
@@ -793,9 +793,14 @@ bool ConstructDecompositionT<C, H>::applyClause(
// [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;
>From 31dae1ff4eed56c94012b1903fdfdb1fe78d7ca0 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Tue, 14 May 2024 17:33:35 -0500
Subject: [PATCH 2/4] Remove "shared" attribute from symbols in privatizing
clauses
---
.../Frontend/OpenMP/ConstructCompositionT.h | 20 ++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/llvm/include/llvm/Frontend/OpenMP/ConstructCompositionT.h b/llvm/include/llvm/Frontend/OpenMP/ConstructCompositionT.h
index 9dcb115a0c514..f6ee963bd8855 100644
--- a/llvm/include/llvm/Frontend/OpenMP/ConstructCompositionT.h
+++ b/llvm/include/llvm/Frontend/OpenMP/ConstructCompositionT.h
@@ -343,13 +343,31 @@ template <typename C> void ConstructCompositionT<C>::mergeDSA() {
}
}
- // Check reductions as well, clear "shared" if set.
+ // Check other privatizing clauses as well, clear "shared" if set.
+ for (auto &clause : clauseSets[llvm::omp::Clause::OMPC_in_reduction]) {
+ using InReductionTy = tomp::clause::InReductionT<TypeTy, IdTy, ExprTy>;
+ using ListTy = typename InReductionTy::List;
+ for (auto &object : std::get<ListTy>(std::get<InReductionTy>(clause.u).t))
+ getDsa(object).second &= ~DSA::Shared;
+ }
+ for (auto &clause : clauseSets[llvm::omp::Clause::OMPC_linear]) {
+ using LinearTy = tomp::clause::LinearT<TypeTy, IdTy, ExprTy>;
+ using ListTy = typename LinearTy::List;
+ for (auto &object : std::get<ListTy>(std::get<LinearTy>(clause.u).t))
+ getDsa(object).second &= ~DSA::Shared;
+ }
for (auto &clause : clauseSets[llvm::omp::Clause::OMPC_reduction]) {
using ReductionTy = tomp::clause::ReductionT<TypeTy, IdTy, ExprTy>;
using ListTy = typename ReductionTy::List;
for (auto &object : std::get<ListTy>(std::get<ReductionTy>(clause.u).t))
getDsa(object).second &= ~DSA::Shared;
}
+ for (auto &clause : clauseSets[llvm::omp::Clause::OMPC_task_reduction]) {
+ using TaskReductionTy = tomp::clause::TaskReductionT<TypeTy, IdTy, ExprTy>;
+ using ListTy = typename TaskReductionTy::List;
+ for (auto &object : std::get<ListTy>(std::get<TaskReductionTy>(clause.u).t))
+ getDsa(object).second &= ~DSA::Shared;
+ }
tomp::ListT<ObjectTy> privateObj, sharedObj, firstpObj, lastpObj, lastpcObj;
for (auto &[object, dsa] : objectDsa) {
>From bbd50ab89eaa870aab4211676f887b4c2ef90060 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Wed, 15 May 2024 07:39:57 -0500
Subject: [PATCH 3/4] Add test for recognizing privatizing clauses (they're
required by allocate)
---
.../Frontend/OpenMPDecompositionTest.cpp | 123 ++++++++++++++++--
1 file changed, 115 insertions(+), 8 deletions(-)
diff --git a/llvm/unittests/Frontend/OpenMPDecompositionTest.cpp b/llvm/unittests/Frontend/OpenMPDecompositionTest.cpp
index df48e9cc0ff4a..8c2d008e38f63 100644
--- a/llvm/unittests/Frontend/OpenMPDecompositionTest.cpp
+++ b/llvm/unittests/Frontend/OpenMPDecompositionTest.cpp
@@ -288,6 +288,14 @@ std::string stringify(const omp::DirectiveWithClauses &DWC) {
// --- Tests ----------------------------------------------------------
+namespace red {
+// Make is easier to construct reduction operators from built-in intrinsics.
+omp::clause::ReductionOperator
+makeOp(omp::clause::DefinedOperator::IntrinsicOperator Op) {
+ return omp::clause::ReductionOperator{omp::clause::DefinedOperator{Op}};
+}
+} // namespace red
+
namespace {
using namespace llvm::omp;
@@ -699,6 +707,92 @@ TEST_F(OpenMPDecompositionTest, Order1) {
TEST_F(OpenMPDecompositionTest, Allocate1) {
omp::Object x{"x"};
+ // Allocate + firstprivate
+ omp::List<omp::Clause> Clauses{
+ {OMPC_allocate,
+ omp::clause::Allocate{{std::nullopt, std::nullopt, std::nullopt, {x}}}},
+ {OMPC_firstprivate, omp::clause::Firstprivate{{x}}},
+ };
+
+ omp::ConstructDecomposition Dec(AnyVersion, Helper, OMPD_parallel_sections,
+ Clauses);
+ ASSERT_EQ(Dec.output.size(), 2u);
+
+ std::string Dir0 = stringify(Dec.output[0]);
+ std::string Dir1 = stringify(Dec.output[1]);
+ ASSERT_EQ(Dir0, "parallel shared(x)"); // (33)
+ ASSERT_EQ(Dir1, "sections firstprivate(x) allocate(, , , (x))"); // (33)
+}
+
+TEST_F(OpenMPDecompositionTest, Allocate2) {
+ omp::Object x{"x"};
+ auto Add = red::makeOp(omp::clause::DefinedOperator::IntrinsicOperator::Add);
+
+ // Allocate + in_reduction
+ omp::List<omp::Clause> Clauses{
+ {OMPC_allocate,
+ omp::clause::Allocate{{std::nullopt, std::nullopt, std::nullopt, {x}}}},
+ {OMPC_in_reduction, omp::clause::InReduction{{{Add}, {x}}}},
+ };
+
+ omp::ConstructDecomposition Dec(AnyVersion, Helper, OMPD_target_parallel,
+ Clauses);
+ ASSERT_EQ(Dec.output.size(), 2u);
+
+ std::string Dir0 = stringify(Dec.output[0]);
+ std::string Dir1 = stringify(Dec.output[1]);
+ ASSERT_EQ(Dir0, "target in_reduction((3), (x)) allocate(, , , (x))"); // (33)
+ ASSERT_EQ(Dir1, "parallel"); // (33)
+}
+
+TEST_F(OpenMPDecompositionTest, Allocate3) {
+ omp::Object x{"x"};
+
+ // Allocate + linear
+ omp::List<omp::Clause> Clauses{
+ {OMPC_allocate,
+ omp::clause::Allocate{{std::nullopt, std::nullopt, std::nullopt, {x}}}},
+ {OMPC_linear,
+ omp::clause::Linear{{std::nullopt, std::nullopt, std::nullopt, {x}}}},
+ };
+
+ omp::ConstructDecomposition Dec(AnyVersion, Helper, OMPD_parallel_for,
+ Clauses);
+ ASSERT_EQ(Dec.output.size(), 2u);
+
+ std::string Dir0 = stringify(Dec.output[0]);
+ std::string Dir1 = stringify(Dec.output[1]);
+ // The "shared" clause is duplicated---this isn't harmful, but it
+ // should be fixed eventually.
+ ASSERT_EQ(Dir0, "parallel shared(x) shared(x)"); // (33)
+ ASSERT_EQ(Dir1, "for linear(, , , (x)) firstprivate(x) lastprivate(, (x)) "
+ "allocate(, , , (x))"); // (33)
+}
+
+TEST_F(OpenMPDecompositionTest, Allocate4) {
+ omp::Object x{"x"};
+
+ // Allocate + lastprivate
+ omp::List<omp::Clause> Clauses{
+ {OMPC_allocate,
+ omp::clause::Allocate{{std::nullopt, std::nullopt, std::nullopt, {x}}}},
+ {OMPC_lastprivate, omp::clause::Lastprivate{{std::nullopt, {x}}}},
+ };
+
+ omp::ConstructDecomposition Dec(AnyVersion, Helper, OMPD_parallel_sections,
+ Clauses);
+ ASSERT_EQ(Dec.output.size(), 2u);
+
+ std::string Dir0 = stringify(Dec.output[0]);
+ std::string Dir1 = stringify(Dec.output[1]);
+ ASSERT_EQ(Dir0, "parallel shared(x)"); // (33)
+ ASSERT_EQ(Dir1, "sections lastprivate(, (x)) allocate(, , , (x))"); // (33)
+}
+
+TEST_F(OpenMPDecompositionTest, Allocate5) {
+ omp::Object x{"x"};
+
+ // Allocate + private
omp::List<omp::Clause> Clauses{
{OMPC_allocate,
omp::clause::Allocate{{std::nullopt, std::nullopt, std::nullopt, {x}}}},
@@ -715,6 +809,27 @@ TEST_F(OpenMPDecompositionTest, Allocate1) {
ASSERT_EQ(Dir1, "sections private(x) allocate(, , , (x))"); // (33)
}
+TEST_F(OpenMPDecompositionTest, Allocate6) {
+ omp::Object x{"x"};
+ auto Add = red::makeOp(omp::clause::DefinedOperator::IntrinsicOperator::Add);
+
+ // Allocate + reduction
+ omp::List<omp::Clause> Clauses{
+ {OMPC_allocate,
+ omp::clause::Allocate{{std::nullopt, std::nullopt, std::nullopt, {x}}}},
+ {OMPC_reduction, omp::clause::Reduction{{std::nullopt, {Add}, {x}}}},
+ };
+
+ omp::ConstructDecomposition Dec(AnyVersion, Helper, OMPD_parallel_sections,
+ Clauses);
+ ASSERT_EQ(Dec.output.size(), 2u);
+
+ std::string Dir0 = stringify(Dec.output[0]);
+ std::string Dir1 = stringify(Dec.output[1]);
+ ASSERT_EQ(Dir0, "parallel shared(x)"); // (33)
+ ASSERT_EQ(Dir1, "sections reduction(, (3), (x)) allocate(, , , (x))"); // (33)
+}
+
// REDUCTION
// [5.2:134:17-18]
// Directives: do, for, loop, parallel, scope, sections, simd, taskloop, teams
@@ -741,14 +856,6 @@ TEST_F(OpenMPDecompositionTest, Allocate1) {
// clause on the construct, then the effect is as if the list item in the
// reduction clause appears as a list item in a map clause with a map-type of
// tofrom.
-namespace red {
-// Make is easier to construct reduction operators from built-in intrinsics.
-omp::clause::ReductionOperator
-makeOp(omp::clause::DefinedOperator::IntrinsicOperator Op) {
- return omp::clause::ReductionOperator{omp::clause::DefinedOperator{Op}};
-}
-} // namespace red
-
TEST_F(OpenMPDecompositionTest, Reduction1) {
omp::Object x{"x"};
auto Add = red::makeOp(omp::clause::DefinedOperator::IntrinsicOperator::Add);
>From 1a6b637d17492facf8d98aa8dfb4a6dec73e645b Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Wed, 15 May 2024 08:38:15 -0500
Subject: [PATCH 4/4] Update
llvm/unittests/Frontend/OpenMPDecompositionTest.cpp
Co-authored-by: Tom Eccles <t at freedommail.info>
---
llvm/unittests/Frontend/OpenMPDecompositionTest.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/unittests/Frontend/OpenMPDecompositionTest.cpp b/llvm/unittests/Frontend/OpenMPDecompositionTest.cpp
index 8c2d008e38f63..8157e41e833a9 100644
--- a/llvm/unittests/Frontend/OpenMPDecompositionTest.cpp
+++ b/llvm/unittests/Frontend/OpenMPDecompositionTest.cpp
@@ -289,7 +289,7 @@ std::string stringify(const omp::DirectiveWithClauses &DWC) {
// --- Tests ----------------------------------------------------------
namespace red {
-// Make is easier to construct reduction operators from built-in intrinsics.
+// Make it easier to construct reduction operators from built-in intrinsics.
omp::clause::ReductionOperator
makeOp(omp::clause::DefinedOperator::IntrinsicOperator Op) {
return omp::clause::ReductionOperator{omp::clause::DefinedOperator{Op}};
More information about the llvm-commits
mailing list