[llvm-branch-commits] [flang] [Flang][OpenMP] Move loop privatization out of dispatch (PR #106066)
via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Mon Aug 26 05:41:32 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-flang-fir-hlfir
Author: Sergio Afonso (skatrak)
<details>
<summary>Changes</summary>
This patch moves the creation of `DataSharingProcessor` instances for loop constructs out of `genOMPDispatch()` and into their corresponding codegen functions. This is a necessary first step to enable a proper handling of privatization on composite constructs.
Some tests are updated due to a change of order between clause processing and privatization.
---
Patch is 34.71 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/106066.diff
6 Files Affected:
- (modified) flang/lib/Lower/OpenMP/OpenMP.cpp (+84-58)
- (modified) flang/test/Lower/OpenMP/parallel-reduction3.f90 (+7-7)
- (modified) flang/test/Lower/OpenMP/wsloop-reduction-array-assumed-shape.f90 (+7-7)
- (modified) flang/test/Lower/OpenMP/wsloop-reduction-array.f90 (+9-9)
- (modified) flang/test/Lower/OpenMP/wsloop-reduction-array2.f90 (+9-9)
- (modified) flang/test/Lower/OpenMP/wsloop-reduction-multiple-clauses.f90 (+11-11)
``````````diff
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index d614db8b68ef65..307cf47247b743 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -1044,7 +1044,6 @@ static void genDistributeClauses(lower::AbstractConverter &converter,
cp.processAllocate(clauseOps);
cp.processDistSchedule(stmtCtx, clauseOps);
cp.processOrder(clauseOps);
- // TODO Support delayed privatization.
}
static void genFlushClauses(lower::AbstractConverter &converter,
@@ -1128,7 +1127,6 @@ static void genSimdClauses(lower::AbstractConverter &converter,
cp.processSafelen(clauseOps);
cp.processSimdlen(clauseOps);
- // TODO Support delayed privatization.
cp.processTODO<clause::Linear, clause::Nontemporal>(
loc, llvm::omp::Directive::OMPD_simd);
}
@@ -1299,7 +1297,6 @@ static void genWsloopClauses(
cp.processOrdered(clauseOps);
cp.processReduction(loc, clauseOps, &reductionTypes, &reductionSyms);
cp.processSchedule(stmtCtx, clauseOps);
- // TODO Support delayed privatization.
cp.processTODO<clause::Allocate, clause::Linear>(
loc, llvm::omp::Directive::OMPD_do);
@@ -1918,17 +1915,25 @@ genTeamsOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
// also be a leaf of a composite construct
//===----------------------------------------------------------------------===//
-static void genStandaloneDistribute(
- lower::AbstractConverter &converter, lower::SymMap &symTable,
- semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
- mlir::Location loc, const ConstructQueue &queue,
- ConstructQueue::const_iterator item, DataSharingProcessor &dsp) {
+static void genStandaloneDistribute(lower::AbstractConverter &converter,
+ lower::SymMap &symTable,
+ semantics::SemanticsContext &semaCtx,
+ lower::pft::Evaluation &eval,
+ mlir::Location loc,
+ const ConstructQueue &queue,
+ ConstructQueue::const_iterator item) {
lower::StatementContext stmtCtx;
mlir::omp::DistributeOperands distributeClauseOps;
genDistributeClauses(converter, semaCtx, stmtCtx, item->clauses, loc,
distributeClauseOps);
+ // TODO: Support delayed privatization.
+ DataSharingProcessor dsp(converter, semaCtx, item->clauses, eval,
+ /*shouldCollectPreDeterminedSymbols=*/true,
+ /*useDelayedPrivatization=*/false, &symTable);
+ dsp.processStep1();
+
mlir::omp::LoopNestOperands loopNestClauseOps;
llvm::SmallVector<const semantics::Symbol *> iv;
genLoopNestClauses(converter, semaCtx, eval, item->clauses, loc,
@@ -1949,8 +1954,7 @@ static void genStandaloneDo(lower::AbstractConverter &converter,
semantics::SemanticsContext &semaCtx,
lower::pft::Evaluation &eval, mlir::Location loc,
const ConstructQueue &queue,
- ConstructQueue::const_iterator item,
- DataSharingProcessor &dsp) {
+ ConstructQueue::const_iterator item) {
lower::StatementContext stmtCtx;
mlir::omp::WsloopOperands wsloopClauseOps;
@@ -1959,6 +1963,12 @@ static void genStandaloneDo(lower::AbstractConverter &converter,
genWsloopClauses(converter, semaCtx, stmtCtx, item->clauses, loc,
wsloopClauseOps, reductionTypes, reductionSyms);
+ // TODO: Support delayed privatization.
+ DataSharingProcessor dsp(converter, semaCtx, item->clauses, eval,
+ /*shouldCollectPreDeterminedSymbols=*/true,
+ /*useDelayedPrivatization=*/false, &symTable);
+ dsp.processStep1();
+
mlir::omp::LoopNestOperands loopNestClauseOps;
llvm::SmallVector<const semantics::Symbol *> iv;
genLoopNestClauses(converter, semaCtx, eval, item->clauses, loc,
@@ -1998,11 +2008,16 @@ static void genStandaloneSimd(lower::AbstractConverter &converter,
semantics::SemanticsContext &semaCtx,
lower::pft::Evaluation &eval, mlir::Location loc,
const ConstructQueue &queue,
- ConstructQueue::const_iterator item,
- DataSharingProcessor &dsp) {
+ ConstructQueue::const_iterator item) {
mlir::omp::SimdOperands simdClauseOps;
genSimdClauses(converter, semaCtx, item->clauses, loc, simdClauseOps);
+ // TODO: Support delayed privatization.
+ DataSharingProcessor dsp(converter, semaCtx, item->clauses, eval,
+ /*shouldCollectPreDeterminedSymbols=*/true,
+ /*useDelayedPrivatization=*/false, &symTable);
+ dsp.processStep1();
+
mlir::omp::LoopNestOperands loopNestClauseOps;
llvm::SmallVector<const semantics::Symbol *> iv;
genLoopNestClauses(converter, semaCtx, eval, item->clauses, loc,
@@ -2018,11 +2033,13 @@ static void genStandaloneSimd(lower::AbstractConverter &converter,
llvm::omp::Directive::OMPD_simd, dsp);
}
-static void genStandaloneTaskloop(
- lower::AbstractConverter &converter, lower::SymMap &symTable,
- semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
- mlir::Location loc, const ConstructQueue &queue,
- ConstructQueue::const_iterator item, DataSharingProcessor &dsp) {
+static void genStandaloneTaskloop(lower::AbstractConverter &converter,
+ lower::SymMap &symTable,
+ semantics::SemanticsContext &semaCtx,
+ lower::pft::Evaluation &eval,
+ mlir::Location loc,
+ const ConstructQueue &queue,
+ ConstructQueue::const_iterator item) {
TODO(loc, "Taskloop construct");
}
@@ -2034,7 +2051,7 @@ static void genCompositeDistributeParallelDo(
lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
mlir::Location loc, const ConstructQueue &queue,
- ConstructQueue::const_iterator item, DataSharingProcessor &dsp) {
+ ConstructQueue::const_iterator item) {
assert(std::distance(item, queue.end()) == 3 && "Invalid leaf constructs");
TODO(loc, "Composite DISTRIBUTE PARALLEL DO");
}
@@ -2043,16 +2060,18 @@ static void genCompositeDistributeParallelDoSimd(
lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
mlir::Location loc, const ConstructQueue &queue,
- ConstructQueue::const_iterator item, DataSharingProcessor &dsp) {
+ ConstructQueue::const_iterator item) {
assert(std::distance(item, queue.end()) == 4 && "Invalid leaf constructs");
TODO(loc, "Composite DISTRIBUTE PARALLEL DO SIMD");
}
-static void genCompositeDistributeSimd(
- lower::AbstractConverter &converter, lower::SymMap &symTable,
- semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
- mlir::Location loc, const ConstructQueue &queue,
- ConstructQueue::const_iterator item, DataSharingProcessor &dsp) {
+static void genCompositeDistributeSimd(lower::AbstractConverter &converter,
+ lower::SymMap &symTable,
+ semantics::SemanticsContext &semaCtx,
+ lower::pft::Evaluation &eval,
+ mlir::Location loc,
+ const ConstructQueue &queue,
+ ConstructQueue::const_iterator item) {
lower::StatementContext stmtCtx;
assert(std::distance(item, queue.end()) == 2 && "Invalid leaf constructs");
@@ -2067,6 +2086,12 @@ static void genCompositeDistributeSimd(
mlir::omp::SimdOperands simdClauseOps;
genSimdClauses(converter, semaCtx, simdItem->clauses, loc, simdClauseOps);
+ // TODO: Support delayed privatization.
+ DataSharingProcessor dsp(converter, semaCtx, simdItem->clauses, eval,
+ /*shouldCollectPreDeterminedSymbols=*/true,
+ /*useDelayedPrivatization=*/false, &symTable);
+ dsp.processStep1();
+
// Pass the innermost leaf construct's clauses because that's where COLLAPSE
// is placed by construct decomposition.
mlir::omp::LoopNestOperands loopNestClauseOps;
@@ -2103,8 +2128,7 @@ static void genCompositeDoSimd(lower::AbstractConverter &converter,
semantics::SemanticsContext &semaCtx,
lower::pft::Evaluation &eval, mlir::Location loc,
const ConstructQueue &queue,
- ConstructQueue::const_iterator item,
- DataSharingProcessor &dsp) {
+ ConstructQueue::const_iterator item) {
lower::StatementContext stmtCtx;
assert(std::distance(item, queue.end()) == 2 && "Invalid leaf constructs");
@@ -2121,6 +2145,12 @@ static void genCompositeDoSimd(lower::AbstractConverter &converter,
mlir::omp::SimdOperands simdClauseOps;
genSimdClauses(converter, semaCtx, simdItem->clauses, loc, simdClauseOps);
+ // TODO: Support delayed privatization.
+ DataSharingProcessor dsp(converter, semaCtx, simdItem->clauses, eval,
+ /*shouldCollectPreDeterminedSymbols=*/true,
+ /*useDelayedPrivatization=*/false, &symTable);
+ dsp.processStep1();
+
// Pass the innermost leaf construct's clauses because that's where COLLAPSE
// is placed by construct decomposition.
mlir::omp::LoopNestOperands loopNestClauseOps;
@@ -2151,11 +2181,13 @@ static void genCompositeDoSimd(lower::AbstractConverter &converter,
llvm::omp::Directive::OMPD_do_simd, dsp);
}
-static void genCompositeTaskloopSimd(
- lower::AbstractConverter &converter, lower::SymMap &symTable,
- semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
- mlir::Location loc, const ConstructQueue &queue,
- ConstructQueue::const_iterator item, DataSharingProcessor &dsp) {
+static void genCompositeTaskloopSimd(lower::AbstractConverter &converter,
+ lower::SymMap &symTable,
+ semantics::SemanticsContext &semaCtx,
+ lower::pft::Evaluation &eval,
+ mlir::Location loc,
+ const ConstructQueue &queue,
+ ConstructQueue::const_iterator item) {
assert(std::distance(item, queue.end()) == 2 && "Invalid leaf constructs");
TODO(loc, "Composite TASKLOOP SIMD");
}
@@ -2164,30 +2196,35 @@ static void genCompositeTaskloopSimd(
// Dispatch
//===----------------------------------------------------------------------===//
-static bool genOMPCompositeDispatch(
- lower::AbstractConverter &converter, lower::SymMap &symTable,
- semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
- mlir::Location loc, const ConstructQueue &queue,
- ConstructQueue::const_iterator item, DataSharingProcessor &dsp) {
+static bool genOMPCompositeDispatch(lower::AbstractConverter &converter,
+ lower::SymMap &symTable,
+ semantics::SemanticsContext &semaCtx,
+ lower::pft::Evaluation &eval,
+ mlir::Location loc,
+ const ConstructQueue &queue,
+ ConstructQueue::const_iterator item) {
using llvm::omp::Directive;
using lower::omp::matchLeafSequence;
+ // TODO: Privatization for composite constructs is currently only done based
+ // on the clauses for their last leaf construct, which may not always be
+ // correct. Consider per-leaf privatization of composite constructs once
+ // delayed privatization is supported by all participating ops.
if (matchLeafSequence(item, queue, Directive::OMPD_distribute_parallel_do))
genCompositeDistributeParallelDo(converter, symTable, semaCtx, eval, loc,
- queue, item, dsp);
+ queue, item);
else if (matchLeafSequence(item, queue,
Directive::OMPD_distribute_parallel_do_simd))
genCompositeDistributeParallelDoSimd(converter, symTable, semaCtx, eval,
- loc, queue, item, dsp);
+ loc, queue, item);
else if (matchLeafSequence(item, queue, Directive::OMPD_distribute_simd))
genCompositeDistributeSimd(converter, symTable, semaCtx, eval, loc, queue,
- item, dsp);
+ item);
else if (matchLeafSequence(item, queue, Directive::OMPD_do_simd))
- genCompositeDoSimd(converter, symTable, semaCtx, eval, loc, queue, item,
- dsp);
+ genCompositeDoSimd(converter, symTable, semaCtx, eval, loc, queue, item);
else if (matchLeafSequence(item, queue, Directive::OMPD_taskloop_simd))
genCompositeTaskloopSimd(converter, symTable, semaCtx, eval, loc, queue,
- item, dsp);
+ item);
else
return false;
@@ -2202,20 +2239,12 @@ static void genOMPDispatch(lower::AbstractConverter &converter,
ConstructQueue::const_iterator item) {
assert(item != queue.end());
- std::optional<DataSharingProcessor> loopDsp;
bool loopLeaf = llvm::omp::getDirectiveAssociation(item->id) ==
llvm::omp::Association::Loop;
if (loopLeaf) {
symTable.pushScope();
- // TODO: Use one DataSharingProcessor for each leaf of a composite
- // construct.
- loopDsp.emplace(converter, semaCtx, item->clauses, eval,
- /*shouldCollectPreDeterminedSymbols=*/true,
- /*useDelayedPrivatization=*/false, &symTable);
- loopDsp->processStep1();
-
if (genOMPCompositeDispatch(converter, symTable, semaCtx, eval, loc, queue,
- item, *loopDsp)) {
+ item)) {
symTable.popScope();
return;
}
@@ -2227,11 +2256,10 @@ static void genOMPDispatch(lower::AbstractConverter &converter,
break;
case llvm::omp::Directive::OMPD_distribute:
genStandaloneDistribute(converter, symTable, semaCtx, eval, loc, queue,
- item, *loopDsp);
+ item);
break;
case llvm::omp::Directive::OMPD_do:
- genStandaloneDo(converter, symTable, semaCtx, eval, loc, queue, item,
- *loopDsp);
+ genStandaloneDo(converter, symTable, semaCtx, eval, loc, queue, item);
break;
case llvm::omp::Directive::OMPD_loop:
TODO(loc, "Unhandled directive " + llvm::omp::getOpenMPDirectiveName(dir));
@@ -2260,8 +2288,7 @@ static void genOMPDispatch(lower::AbstractConverter &converter,
// in genBodyOfOp
break;
case llvm::omp::Directive::OMPD_simd:
- genStandaloneSimd(converter, symTable, semaCtx, eval, loc, queue, item,
- *loopDsp);
+ genStandaloneSimd(converter, symTable, semaCtx, eval, loc, queue, item);
break;
case llvm::omp::Directive::OMPD_single:
genSingleOp(converter, symTable, semaCtx, eval, loc, queue, item);
@@ -2291,8 +2318,7 @@ static void genOMPDispatch(lower::AbstractConverter &converter,
genTaskgroupOp(converter, symTable, semaCtx, eval, loc, queue, item);
break;
case llvm::omp::Directive::OMPD_taskloop:
- genStandaloneTaskloop(converter, symTable, semaCtx, eval, loc, queue, item,
- *loopDsp);
+ genStandaloneTaskloop(converter, symTable, semaCtx, eval, loc, queue, item);
break;
case llvm::omp::Directive::OMPD_taskwait:
genTaskwaitOp(converter, symTable, semaCtx, eval, loc, queue, item);
diff --git a/flang/test/Lower/OpenMP/parallel-reduction3.f90 b/flang/test/Lower/OpenMP/parallel-reduction3.f90
index 441dff34553d4f..591f41cb946602 100644
--- a/flang/test/Lower/OpenMP/parallel-reduction3.f90
+++ b/flang/test/Lower/OpenMP/parallel-reduction3.f90
@@ -69,19 +69,19 @@
! CHECK: %[[VAL_13:.*]] = arith.constant 0 : i32
! CHECK: hlfir.assign %[[VAL_13]] to %[[VAL_12]]#0 : i32, !fir.box<!fir.array<?xi32>>
! CHECK: omp.parallel {
-! CHECK: %[[VAL_14:.*]] = fir.alloca i32 {bindc_name = "i", pinned, {{.*}}}
-! CHECK: %[[VAL_15:.*]]:2 = hlfir.declare %[[VAL_14]] {uniq_name = "_QFsEi"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
-! CHECK: %[[VAL_16:.*]] = fir.alloca !fir.box<!fir.array<?xi32>>
-! CHECK: fir.store %[[VAL_12]]#0 to %[[VAL_16]] : !fir.ref<!fir.box<!fir.array<?xi32>>>
+! CHECK: %[[VAL_14:.*]] = fir.alloca !fir.box<!fir.array<?xi32>>
+! CHECK: fir.store %[[VAL_12]]#0 to %[[VAL_14]] : !fir.ref<!fir.box<!fir.array<?xi32>>>
+! CHECK: %[[VAL_15:.*]] = fir.alloca i32 {bindc_name = "i", pinned, {{.*}}}
+! CHECK: %[[VAL_16:.*]]:2 = hlfir.declare %[[VAL_15]] {uniq_name = "_QFsEi"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
! CHECK: %[[VAL_17:.*]] = arith.constant 1 : i32
! CHECK: %[[VAL_18:.*]] = arith.constant 100 : i32
! CHECK: %[[VAL_19:.*]] = arith.constant 1 : i32
-! CHECK: omp.wsloop reduction(byref @add_reduction_byref_box_Uxi32 %[[VAL_16]] -> %[[VAL_20:.*]] : !fir.ref<!fir.box<!fir.array<?xi32>>>) {
+! CHECK: omp.wsloop reduction(byref @add_reduction_byref_box_Uxi32 %[[VAL_14]] -> %[[VAL_20:.*]] : !fir.ref<!fir.box<!fir.array<?xi32>>>) {
! CHECK-NEXT: omp.loop_nest (%[[VAL_21:.*]]) : i32 = (%[[VAL_17]]) to (%[[VAL_18]]) inclusive step (%[[VAL_19]]) {
! CHECK: %[[VAL_22:.*]]:2 = hlfir.declare %[[VAL_20]] {uniq_name = "_QFsEc"} : (!fir.ref<!fir.box<!fir.array<?xi32>>>) -> (!fir.ref<!fir.box<!fir.array<?xi32>>>, !fir.ref<!fir.box<!fir.array<?xi32>>>)
-! CHECK: fir.store %[[VAL_21]] to %[[VAL_15]]#1 : !fir.ref<i32>
+! CHECK: fir.store %[[VAL_21]] to %[[VAL_16]]#1 : !fir.ref<i32>
! CHECK: %[[VAL_23:.*]] = fir.load %[[VAL_22]]#0 : !fir.ref<!fir.box<!fir.array<?xi32>>>
-! CHECK: %[[VAL_24:.*]] = fir.load %[[VAL_15]]#0 : !fir.ref<i32>
+! CHECK: %[[VAL_24:.*]] = fir.load %[[VAL_16]]#0 : !fir.ref<i32>
! CHECK: %[[VAL_25:.*]] = arith.constant 0 : index
! CHECK: %[[VAL_26:.*]]:3 = fir.box_dims %[[VAL_23]], %[[VAL_25]] : (!fir.box<!fir.array<?xi32>>, index) -> (index, index, index)
! CHECK: %[[VAL_27:.*]] = fir.shape %[[VAL_26]]#1 : (index) -> !fir.shape<1>
diff --git a/flang/test/Lower/OpenMP/wsloop-reduction-array-assumed-shape.f90 b/flang/test/Lower/OpenMP/wsloop-reduction-array-assumed-shape.f90
index c984ab61bedb3b..d881ff8c1a026a 100644
--- a/flang/test/Lower/OpenMP/wsloop-reduction-array-assumed-shape.f90
+++ b/flang/test/Lower/OpenMP/wsloop-reduction-array-assumed-shape.f90
@@ -79,18 +79,18 @@ subroutine reduce(r)
! CHECK: %[[VAL_2:.*]]:2 = hlfir.declare %[[VAL_1]] {uniq_name = "_QFFreduceEi"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
! CHECK: %[[VAL_3:.*]]:2 = hlfir.declare %[[VAL_0]] dummy_scope %{{[0-9]+}} {fortran_attrs = {{.*}}, uniq_name = "_QFFreduceEr"} : (!fir.box<!fir.array<?xf64>>, !fir.dscope) -> (!fir.box<!fir.array<?xf64>>, !fir.box<!fir.array<?xf64>>)
! CHECK: omp.parallel {
-! CHECK: %[[VAL_4:.*]] = fir.alloca i32 {bindc_name = "i", pinned, {{.*}}}
-! CHECK: %[[VAL_5:.*]]:2 = hlfir.declare %[[VAL_4]] {uniq_name = "_QFFreduceEi"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
-! CHECK: %[[VAL_6:.*]] = fir.alloca !fir.box<!fir.array<?xf64>>
-! CHECK: fir.store %[[VAL_3]]#1 to %[[VAL_6]] : !fir.ref<!fir.box<!fir.array<?xf64>>>
+! CHECK: ...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/106066
More information about the llvm-branch-commits
mailing list