[flang-commits] [flang] [flang][OpenMP][Lower] fix statement context cleanup insertion point (PR #133891)
via flang-commits
flang-commits at lists.llvm.org
Mon Apr 7 09:32:08 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-flang-fir-hlfir
Author: Tom Eccles (tblah)
<details>
<summary>Changes</summary>
The statement context is used for lowering clauses for openmp operations using generalised helpers from flang lowering. The statement context stores closures which generate code for cleaning up temporary values generated by the lowering helper. These closures are run when the statement construct is destroyed. Keeping the statement context local to the clause or operation being lowered without any special handling was not correct because any cleanup code would be generated at the insertion point when that statement context went out of scope (which would in general be inside of the newly created container operation). It would be better to generate the cleanup code after the newly created operation (clause processing is synchronous even for deferred tasks).
Currently supported clauses are mostly populated with simple scalar values that require no cleanup. Even the simple array sections added by #<!-- -->132994 needed no cleanup because indexing the right values of the array did not create any temporaries. Supporting array sections with vector indexing will generate hlfir.destroy operations for cleanup. This patch fixes where those will be created. Those hlfir.destroy operations don't generate any FIR (or LLVM) code, but the issue still exists theoretically.
I wasn't able to find any clauses which have any cleanup to use to test this PR. It is probably NFC for the current lowering. This will be tested in [the PR adding vector subscripting of array sections](https://github.com/llvm/llvm-project/pull/133892).
---
Patch is 31.26 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/133891.diff
2 Files Affected:
- (modified) flang/lib/Lower/OpenMP/OpenMP.cpp (+164-159)
- (added) flang/test/Lower/OpenMP/clause-cleanup.f90 (+17)
``````````diff
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index ab90b4609e855..0968df4f88e28 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -1763,7 +1763,6 @@ static void genTaskClauses(lower::AbstractConverter &converter,
cp.processPriority(stmtCtx, clauseOps);
cp.processUntied(clauseOps);
cp.processDetach(clauseOps);
- // TODO Support delayed privatization.
cp.processTODO<clause::Affinity, clause::InReduction>(
loc, llvm::omp::Directive::OMPD_task);
@@ -1914,12 +1913,11 @@ static mlir::omp::LoopNestOp genLoopNestOp(
queue, item, clauseOps);
}
-static void genLoopOp(lower::AbstractConverter &converter,
- lower::SymMap &symTable,
- semantics::SemanticsContext &semaCtx,
- lower::pft::Evaluation &eval, mlir::Location loc,
- const ConstructQueue &queue,
- ConstructQueue::const_iterator item) {
+static mlir::omp::LoopOp
+genLoopOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
+ semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
+ mlir::Location loc, const ConstructQueue &queue,
+ ConstructQueue::const_iterator item) {
mlir::omp::LoopOperands loopClauseOps;
llvm::SmallVector<const semantics::Symbol *> loopReductionSyms;
genLoopClauses(converter, semaCtx, item->clauses, loc, loopClauseOps,
@@ -1946,14 +1944,15 @@ static void genLoopOp(lower::AbstractConverter &converter,
genLoopNestOp(converter, symTable, semaCtx, eval, loc, queue, item,
loopNestClauseOps, iv, {{loopOp, loopArgs}},
llvm::omp::Directive::OMPD_loop, dsp);
+ return loopOp;
}
static mlir::omp::MaskedOp
genMaskedOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
+ lower::StatementContext &stmtCtx,
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
mlir::Location loc, const ConstructQueue &queue,
ConstructQueue::const_iterator item) {
- lower::StatementContext stmtCtx;
mlir::omp::MaskedOperands clauseOps;
genMaskedClauses(converter, semaCtx, stmtCtx, item->clauses, loc, clauseOps);
@@ -2157,13 +2156,13 @@ genSectionsOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
return sectionsOp;
}
-static void genScopeOp(lower::AbstractConverter &converter,
- lower::SymMap &symTable,
- semantics::SemanticsContext &semaCtx,
- lower::pft::Evaluation &eval, mlir::Location loc,
- const ConstructQueue &queue,
- ConstructQueue::const_iterator item) {
+static mlir::Operation *
+genScopeOp(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, "Scope construct");
+ return nullptr;
}
static mlir::omp::SingleOp
@@ -2183,11 +2182,11 @@ genSingleOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
static mlir::omp::TargetOp
genTargetOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
+ lower::StatementContext &stmtCtx,
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
mlir::Location loc, const ConstructQueue &queue,
ConstructQueue::const_iterator item) {
fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
- lower::StatementContext stmtCtx;
bool isTargetDevice =
llvm::cast<mlir::omp::OffloadModuleInterface>(*converter.getModuleOp())
.getIsTargetDevice();
@@ -2366,13 +2365,11 @@ genTargetOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
return targetOp;
}
-static mlir::omp::TargetDataOp
-genTargetDataOp(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;
+static mlir::omp::TargetDataOp genTargetDataOp(
+ lower::AbstractConverter &converter, lower::SymMap &symTable,
+ lower::StatementContext &stmtCtx, semantics::SemanticsContext &semaCtx,
+ lower::pft::Evaluation &eval, mlir::Location loc,
+ const ConstructQueue &queue, ConstructQueue::const_iterator item) {
mlir::omp::TargetDataOperands clauseOps;
llvm::SmallVector<const semantics::Symbol *> useDeviceAddrSyms,
useDevicePtrSyms;
@@ -2402,10 +2399,10 @@ genTargetDataOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
template <typename OpTy>
static OpTy genTargetEnterExitUpdateDataOp(
lower::AbstractConverter &converter, lower::SymMap &symTable,
- semantics::SemanticsContext &semaCtx, mlir::Location loc,
- const ConstructQueue &queue, ConstructQueue::const_iterator item) {
+ lower::StatementContext &stmtCtx, semantics::SemanticsContext &semaCtx,
+ mlir::Location loc, const ConstructQueue &queue,
+ ConstructQueue::const_iterator item) {
fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
- lower::StatementContext stmtCtx;
// GCC 9.3.0 emits a (probably) bogus warning about an unused variable.
[[maybe_unused]] llvm::omp::Directive directive;
@@ -2428,10 +2425,10 @@ static OpTy genTargetEnterExitUpdateDataOp(
static mlir::omp::TaskOp
genTaskOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
+ lower::StatementContext &stmtCtx,
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
mlir::Location loc, const ConstructQueue &queue,
ConstructQueue::const_iterator item) {
- lower::StatementContext stmtCtx;
mlir::omp::TaskOperands clauseOps;
genTaskClauses(converter, semaCtx, symTable, stmtCtx, item->clauses, loc,
clauseOps);
@@ -2498,13 +2495,11 @@ genTaskyieldOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
return converter.getFirOpBuilder().create<mlir::omp::TaskyieldOp>(loc);
}
-static mlir::omp::WorkshareOp
-genWorkshareOp(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;
+static mlir::omp::WorkshareOp genWorkshareOp(
+ lower::AbstractConverter &converter, lower::SymMap &symTable,
+ lower::StatementContext &stmtCtx, semantics::SemanticsContext &semaCtx,
+ lower::pft::Evaluation &eval, mlir::Location loc,
+ const ConstructQueue &queue, ConstructQueue::const_iterator item) {
mlir::omp::WorkshareOperands clauseOps;
genWorkshareClauses(converter, semaCtx, stmtCtx, item->clauses, loc,
clauseOps);
@@ -2518,11 +2513,10 @@ genWorkshareOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
static mlir::omp::TeamsOp
genTeamsOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
+ lower::StatementContext &stmtCtx,
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
mlir::Location loc, const ConstructQueue &queue,
ConstructQueue::const_iterator item) {
- lower::StatementContext stmtCtx;
-
mlir::omp::TeamsOperands clauseOps;
llvm::SmallVector<const semantics::Symbol *> reductionSyms;
genTeamsClauses(converter, semaCtx, stmtCtx, item->clauses, loc, clauseOps,
@@ -2546,15 +2540,11 @@ 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) {
- lower::StatementContext stmtCtx;
-
+static mlir::omp::DistributeOp genStandaloneDistribute(
+ lower::AbstractConverter &converter, lower::SymMap &symTable,
+ lower::StatementContext &stmtCtx, semantics::SemanticsContext &semaCtx,
+ lower::pft::Evaluation &eval, mlir::Location loc,
+ const ConstructQueue &queue, ConstructQueue::const_iterator item) {
mlir::omp::DistributeOperands distributeClauseOps;
genDistributeClauses(converter, semaCtx, stmtCtx, item->clauses, loc,
distributeClauseOps);
@@ -2578,16 +2568,14 @@ static void genStandaloneDistribute(lower::AbstractConverter &converter,
genLoopNestOp(converter, symTable, semaCtx, eval, loc, queue, item,
loopNestClauseOps, iv, {{distributeOp, distributeArgs}},
llvm::omp::Directive::OMPD_distribute, dsp);
+ return distributeOp;
}
-static void genStandaloneDo(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;
-
+static mlir::omp::WsloopOp genStandaloneDo(
+ lower::AbstractConverter &converter, lower::SymMap &symTable,
+ lower::StatementContext &stmtCtx, semantics::SemanticsContext &semaCtx,
+ lower::pft::Evaluation &eval, mlir::Location loc,
+ const ConstructQueue &queue, ConstructQueue::const_iterator item) {
mlir::omp::WsloopOperands wsloopClauseOps;
llvm::SmallVector<const semantics::Symbol *> wsloopReductionSyms;
genWsloopClauses(converter, semaCtx, stmtCtx, item->clauses, loc,
@@ -2614,17 +2602,14 @@ static void genStandaloneDo(lower::AbstractConverter &converter,
genLoopNestOp(converter, symTable, semaCtx, eval, loc, queue, item,
loopNestClauseOps, iv, {{wsloopOp, wsloopArgs}},
llvm::omp::Directive::OMPD_do, dsp);
+ return wsloopOp;
}
-static void genStandaloneParallel(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;
-
+static mlir::omp::ParallelOp genStandaloneParallel(
+ lower::AbstractConverter &converter, lower::SymMap &symTable,
+ lower::StatementContext &stmtCtx, semantics::SemanticsContext &semaCtx,
+ lower::pft::Evaluation &eval, mlir::Location loc,
+ const ConstructQueue &queue, ConstructQueue::const_iterator item) {
mlir::omp::ParallelOperands parallelClauseOps;
llvm::SmallVector<const semantics::Symbol *> parallelReductionSyms;
genParallelClauses(converter, semaCtx, stmtCtx, item->clauses, loc,
@@ -2644,17 +2629,17 @@ static void genStandaloneParallel(lower::AbstractConverter &converter,
parallelArgs.priv.vars = parallelClauseOps.privateVars;
parallelArgs.reduction.syms = parallelReductionSyms;
parallelArgs.reduction.vars = parallelClauseOps.reductionVars;
- genParallelOp(converter, symTable, semaCtx, eval, loc, queue, item,
- parallelClauseOps, parallelArgs,
- enableDelayedPrivatization ? &dsp.value() : nullptr);
+ return genParallelOp(converter, symTable, semaCtx, eval, loc, queue, item,
+ parallelClauseOps, parallelArgs,
+ enableDelayedPrivatization ? &dsp.value() : nullptr);
}
-static void genStandaloneSimd(lower::AbstractConverter &converter,
- lower::SymMap &symTable,
- semantics::SemanticsContext &semaCtx,
- lower::pft::Evaluation &eval, mlir::Location loc,
- const ConstructQueue &queue,
- ConstructQueue::const_iterator item) {
+static mlir::omp::SimdOp
+genStandaloneSimd(lower::AbstractConverter &converter, lower::SymMap &symTable,
+ semantics::SemanticsContext &semaCtx,
+ lower::pft::Evaluation &eval, mlir::Location loc,
+ const ConstructQueue &queue,
+ ConstructQueue::const_iterator item) {
mlir::omp::SimdOperands simdClauseOps;
llvm::SmallVector<const semantics::Symbol *> simdReductionSyms;
genSimdClauses(converter, semaCtx, item->clauses, loc, simdClauseOps,
@@ -2681,29 +2666,27 @@ static void genStandaloneSimd(lower::AbstractConverter &converter,
genLoopNestOp(converter, symTable, semaCtx, eval, loc, queue, item,
loopNestClauseOps, iv, {{simdOp, simdArgs}},
llvm::omp::Directive::OMPD_simd, dsp);
+ return simdOp;
}
-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) {
+static mlir::omp::TaskloopOp 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");
+ return nullptr;
}
//===----------------------------------------------------------------------===//
// Code generation functions for composite constructs
//===----------------------------------------------------------------------===//
-static void genCompositeDistributeParallelDo(
+static mlir::omp::DistributeOp genCompositeDistributeParallelDo(
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;
-
+ lower::StatementContext &stmtCtx, semantics::SemanticsContext &semaCtx,
+ lower::pft::Evaluation &eval, mlir::Location loc,
+ const ConstructQueue &queue, ConstructQueue::const_iterator item) {
assert(std::distance(item, queue.end()) == 3 && "Invalid leaf constructs");
ConstructQueue::const_iterator distributeItem = item;
ConstructQueue::const_iterator parallelItem = std::next(distributeItem);
@@ -2762,15 +2745,14 @@ static void genCompositeDistributeParallelDo(
loopNestClauseOps, iv,
{{distributeOp, distributeArgs}, {wsloopOp, wsloopArgs}},
llvm::omp::Directive::OMPD_distribute_parallel_do, dsp);
+ return distributeOp;
}
-static void genCompositeDistributeParallelDoSimd(
+static mlir::omp::DistributeOp genCompositeDistributeParallelDoSimd(
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;
-
+ lower::StatementContext &stmtCtx, semantics::SemanticsContext &semaCtx,
+ lower::pft::Evaluation &eval, mlir::Location loc,
+ const ConstructQueue &queue, ConstructQueue::const_iterator item) {
assert(std::distance(item, queue.end()) == 4 && "Invalid leaf constructs");
ConstructQueue::const_iterator distributeItem = item;
ConstructQueue::const_iterator parallelItem = std::next(distributeItem);
@@ -2854,17 +2836,14 @@ static void genCompositeDistributeParallelDoSimd(
{simdOp, simdArgs}},
llvm::omp::Directive::OMPD_distribute_parallel_do_simd,
simdItemDSP);
+ return distributeOp;
}
-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;
-
+static mlir::omp::DistributeOp genCompositeDistributeSimd(
+ lower::AbstractConverter &converter, lower::SymMap &symTable,
+ lower::StatementContext &stmtCtx, 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");
ConstructQueue::const_iterator distributeItem = item;
ConstructQueue::const_iterator simdItem = std::next(distributeItem);
@@ -2911,16 +2890,14 @@ static void genCompositeDistributeSimd(lower::AbstractConverter &converter,
loopNestClauseOps, iv,
{{distributeOp, distributeArgs}, {simdOp, simdArgs}},
llvm::omp::Directive::OMPD_distribute_simd, dsp);
+ return distributeOp;
}
-static void genCompositeDoSimd(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;
-
+static mlir::omp::WsloopOp genCompositeDoSimd(
+ lower::AbstractConverter &converter, lower::SymMap &symTable,
+ lower::StatementContext &stmtCtx, 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");
ConstructQueue::const_iterator doItem = item;
ConstructQueue::const_iterator simdItem = std::next(doItem);
@@ -2970,30 +2947,29 @@ static void genCompositeDoSimd(lower::AbstractConverter &converter,
loopNestClauseOps, iv,
{{wsloopOp, wsloopArgs}, {simdOp, simdArgs}},
llvm::omp::Directive::OMPD_do_simd, dsp);
+ return wsloopOp;
}
-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) {
+static mlir::omp::TaskloopOp genCompositeTaskloopSimd(
+ lower::AbstractConverter &converter, lower::SymMap &symTable,
+ lower::StatementContext &stmtCtx, 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");
+ return nullptr;
}
//===----------------------------------------------------------------------===//
// Dispatch
//===--------------------------------------------------...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/133891
More information about the flang-commits
mailing list