[flang-commits] [flang] [Flang][OpenMP] Remove unused OpWithBodyGenInfo attributes (PR #97572)
Sergio Afonso via flang-commits
flang-commits at lists.llvm.org
Thu Jul 4 07:41:14 PDT 2024
https://github.com/skatrak updated https://github.com/llvm/llvm-project/pull/97572
>From 7439c9a0fb95afe90f44dbd2fb504b3324d4e454 Mon Sep 17 00:00:00 2001
From: Sergio Afonso <safonsof at amd.com>
Date: Wed, 3 Jul 2024 13:33:14 +0100
Subject: [PATCH] [Flang][OpenMP] Remove unused OpWithBodyGenInfo attributes
This patch removes the `outerCombined`, `reductionSymbols` and `reductionTypes`
attributes from the `OpWithBodyGenInfo` structure and their uses, as they never
impact the lowering process or its output.
The `outerCombined` variable is always set to `false`, so in practice it
doesn't represent what its name indicates. Furthermore, initializing it
correctly can result in privatization not being performed in cases where it
should (at least tests doing this together with composite construct support
pointed me in that direction). It seems to be tied to the early privatization
approach, where a redundant alloca could possibly be avoided in certain cases.
With the transition to delayed privatization, it seems like it won't serve that
purpose anymore, since the decision of what and where privatization-related
allocations are inserted will be postponed to the MLIR to LLVM IR translation
stage. Since this feature is already currently not being used, its potential
benefit appears to be minor and it won't make sense to do once the delayed
privatization approach is rolled out, I propose removing it.
The `reductionSymbols` and `reductionTypes` variables are set in certain cases
but never used. Unless there's a plan where these will be needed, in which case
it would be a better alternative to document it, I believe we should also
remove them.
---
flang/lib/Lower/OpenMP/OpenMP.cpp | 64 +++++++------------------------
1 file changed, 14 insertions(+), 50 deletions(-)
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 5b2cf7930e4a3..e1efe0d1bd2d4 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -504,11 +504,6 @@ struct OpWithBodyGenInfo {
: converter(converter), symTable(symTable), semaCtx(semaCtx), loc(loc),
eval(eval), dir(dir) {}
- OpWithBodyGenInfo &setOuterCombined(bool value) {
- outerCombined = value;
- return *this;
- }
-
OpWithBodyGenInfo &setClauses(const List<Clause> *value) {
clauses = value;
return *this;
@@ -519,14 +514,6 @@ struct OpWithBodyGenInfo {
return *this;
}
- OpWithBodyGenInfo &
- setReductions(llvm::SmallVectorImpl<const semantics::Symbol *> *value1,
- llvm::SmallVectorImpl<mlir::Type> *value2) {
- reductionSymbols = value1;
- reductionTypes = value2;
- return *this;
- }
-
OpWithBodyGenInfo &setGenRegionEntryCb(GenOMPRegionEntryCBFn value) {
genRegionEntryCB = value;
return *this;
@@ -544,16 +531,10 @@ struct OpWithBodyGenInfo {
lower::pft::Evaluation &eval;
/// [in] leaf directive for which to generate the op body.
llvm::omp::Directive dir;
- /// [in] is this an outer operation - prevents privatization.
- bool outerCombined = false;
/// [in] list of clauses to process.
const List<Clause> *clauses = nullptr;
/// [in] if provided, processes the construct's data-sharing attributes.
DataSharingProcessor *dsp = nullptr;
- /// [in] if provided, list of reduction symbols
- llvm::SmallVectorImpl<const semantics::Symbol *> *reductionSymbols = nullptr;
- /// [in] if provided, list of reduction types
- llvm::SmallVectorImpl<mlir::Type> *reductionTypes = nullptr;
/// [in] if provided, emits the op's region entry. Otherwise, an emtpy block
/// is created in the region.
GenOMPRegionEntryCBFn genRegionEntryCB = nullptr;
@@ -591,9 +572,8 @@ static void createBodyOfOp(mlir::Operation &op, const OpWithBodyGenInfo &info,
// Mark the earliest insertion point.
mlir::Operation *marker = insertMarker(firOpBuilder);
- // If it is an unstructured region and is not the outer region of a combined
- // construct, create empty blocks for all evaluations.
- if (info.eval.lowerAsUnstructured() && !info.outerCombined)
+ // If it is an unstructured region, create empty blocks for all evaluations.
+ if (info.eval.lowerAsUnstructured())
lower::createEmptyRegionBlocks<mlir::omp::TerminatorOp, mlir::omp::YieldOp>(
firOpBuilder, info.eval.getNestedEvaluations());
@@ -601,16 +581,14 @@ static void createBodyOfOp(mlir::Operation &op, const OpWithBodyGenInfo &info,
// code will use the right symbols.
bool isLoop = llvm::omp::getDirectiveAssociation(info.dir) ==
llvm::omp::Association::Loop;
- bool privatize = info.clauses && !info.outerCombined;
+ bool privatize = info.clauses;
firOpBuilder.setInsertionPoint(marker);
std::optional<DataSharingProcessor> tempDsp;
- if (privatize) {
- if (!info.dsp) {
- tempDsp.emplace(info.converter, info.semaCtx, *info.clauses, info.eval,
- Fortran::lower::omp::isLastItemInQueue(item, queue));
- tempDsp->processStep1();
- }
+ if (privatize && !info.dsp) {
+ tempDsp.emplace(info.converter, info.semaCtx, *info.clauses, info.eval,
+ Fortran::lower::omp::isLastItemInQueue(item, queue));
+ tempDsp->processStep1();
}
if (info.dir == llvm::omp::Directive::OMPD_parallel) {
@@ -1078,8 +1056,7 @@ genOrderedRegionClauses(lower::AbstractConverter &converter,
static void genParallelClauses(
lower::AbstractConverter &converter, semantics::SemanticsContext &semaCtx,
lower::StatementContext &stmtCtx, const List<Clause> &clauses,
- mlir::Location loc, bool processReduction,
- mlir::omp::ParallelClauseOps &clauseOps,
+ mlir::Location loc, mlir::omp::ParallelClauseOps &clauseOps,
llvm::SmallVectorImpl<mlir::Type> &reductionTypes,
llvm::SmallVectorImpl<const semantics::Symbol *> &reductionSyms) {
ClauseProcessor cp(converter, semaCtx, clauses);
@@ -1088,10 +1065,7 @@ static void genParallelClauses(
cp.processIf(llvm::omp::Directive::OMPD_parallel, clauseOps);
cp.processNumThreads(stmtCtx, clauseOps);
cp.processProcBind(clauseOps);
-
- if (processReduction) {
- cp.processReduction(loc, clauseOps, &reductionTypes, &reductionSyms);
- }
+ cp.processReduction(loc, clauseOps, &reductionTypes, &reductionSyms);
}
static void genSectionsClauses(lower::AbstractConverter &converter,
@@ -1440,15 +1414,13 @@ static mlir::omp::ParallelOp
genParallelOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx,
lower::pft::Evaluation &eval, mlir::Location loc,
- const ConstructQueue &queue, ConstructQueue::iterator item,
- bool outerCombined = false) {
+ const ConstructQueue &queue, ConstructQueue::iterator item) {
fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
lower::StatementContext stmtCtx;
mlir::omp::ParallelClauseOps clauseOps;
llvm::SmallVector<mlir::Type> reductionTypes;
llvm::SmallVector<const semantics::Symbol *> reductionSyms;
- genParallelClauses(converter, semaCtx, stmtCtx, item->clauses, loc,
- /*processReduction=*/!outerCombined, clauseOps,
+ genParallelClauses(converter, semaCtx, stmtCtx, item->clauses, loc, clauseOps,
reductionTypes, reductionSyms);
auto reductionCallback = [&](mlir::Operation *op) {
@@ -1459,22 +1431,17 @@ genParallelOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
OpWithBodyGenInfo genInfo =
OpWithBodyGenInfo(converter, symTable, semaCtx, loc, eval,
llvm::omp::Directive::OMPD_parallel)
- .setOuterCombined(outerCombined)
.setClauses(&item->clauses)
- .setReductions(&reductionSyms, &reductionTypes)
.setGenRegionEntryCb(reductionCallback);
if (!enableDelayedPrivatization)
return genOpWithBody<mlir::omp::ParallelOp>(genInfo, queue, item,
clauseOps);
- bool privatize = !outerCombined;
DataSharingProcessor dsp(converter, semaCtx, item->clauses, eval,
lower::omp::isLastItemInQueue(item, queue),
/*useDelayedPrivatization=*/true, &symTable);
-
- if (privatize)
- dsp.processStep1(&clauseOps);
+ dsp.processStep1(&clauseOps);
auto genRegionEntryCB = [&](mlir::Operation *op) {
auto parallelOp = llvm::cast<mlir::omp::ParallelOp>(op);
@@ -1921,7 +1888,7 @@ static mlir::omp::TeamsOp
genTeamsOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
mlir::Location loc, const ConstructQueue &queue,
- ConstructQueue::iterator item, bool outerCombined = false) {
+ ConstructQueue::iterator item) {
lower::StatementContext stmtCtx;
mlir::omp::TeamsClauseOps clauseOps;
genTeamsClauses(converter, semaCtx, stmtCtx, item->clauses, loc, clauseOps);
@@ -1929,7 +1896,6 @@ genTeamsOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
return genOpWithBody<mlir::omp::TeamsOp>(
OpWithBodyGenInfo(converter, symTable, semaCtx, loc, eval,
llvm::omp::Directive::OMPD_teams)
- .setOuterCombined(outerCombined)
.setClauses(&item->clauses),
queue, item, clauseOps);
}
@@ -1982,7 +1948,6 @@ genWsloopOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
*nestedEval, llvm::omp::Directive::OMPD_do)
.setClauses(&item->clauses)
.setDataSharingProcessor(&dsp)
- .setReductions(&reductionSyms, &reductionTypes)
.setGenRegionEntryCb(ivCallback),
queue, item);
symTable.popScope();
@@ -2084,8 +2049,7 @@ static void genOMPDispatch(lower::AbstractConverter &converter,
genOrderedRegionOp(converter, symTable, semaCtx, eval, loc, queue, item);
break;
case llvm::omp::Directive::OMPD_parallel:
- genParallelOp(converter, symTable, semaCtx, eval, loc, queue, item,
- /*outerCombined=*/false);
+ genParallelOp(converter, symTable, semaCtx, eval, loc, queue, item);
break;
case llvm::omp::Directive::OMPD_section:
genSectionOp(converter, symTable, semaCtx, eval, loc, queue, item);
More information about the flang-commits
mailing list