From eb89b138442c1f3db9b6cf62adb46f5f4f881a29 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek 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::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 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 void ConstructCompositionT::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; + using ListTy = typename InReductionTy::List; + for (auto &object : std::get(std::get(clause.u).t)) + getDsa(object).second &= ~DSA::Shared; + } + for (auto &clause : clauseSets[llvm::omp::Clause::OMPC_linear]) { + using LinearTy = tomp::clause::LinearT; + using ListTy = typename LinearTy::List; + for (auto &object : std::get(std::get(clause.u).t)) + getDsa(object).second &= ~DSA::Shared; + } for (auto &clause : clauseSets[llvm::omp::Clause::OMPC_reduction]) { using ReductionTy = tomp::clause::ReductionT; using ListTy = typename ReductionTy::List; for (auto &object : std::get(std::get(clause.u).t)) getDsa(object).second &= ~DSA::Shared; } + for (auto &clause : clauseSets[llvm::omp::Clause::OMPC_task_reduction]) { + using TaskReductionTy = tomp::clause::TaskReductionT; + using ListTy = typename TaskReductionTy::List; + for (auto &object : std::get(std::get(clause.u).t)) + getDsa(object).second &= ~DSA::Shared; + } tomp::ListT privateObj, sharedObj, firstpObj, lastpObj, lastpcObj; for (auto &[object, dsa] : objectDsa) { From bbd50ab89eaa870aab4211676f887b4c2ef90060 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek 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 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 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 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 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 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 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 Date: Wed, 15 May 2024 08:38:15 -0500 Subject: [PATCH 4/4] Update llvm/unittests/Frontend/OpenMPDecompositionTest.cpp Co-authored-by: Tom Eccles --- 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}};