[Flang][OpenMP] NFC: Use ConstructQueue::const_iterator #102612
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This patch replaces `ConstructQueue::iterator` arguments for `ConstructQueue::const_iterator` where it's used as a pointer to an element inside of a `const ConstructQueue &` passed along with it. Since these functions don't intend to modify the list or any elements in it, keeping constness consistent between both makes it simpler to work with.
llvmbot
added
flang
|
@llvm/pr-subscribers-flang-fir-hlfir Author: Sergio Afonso (skatrak) ChangesThis patch replaces Since these functions don't intend to modify the list or any elements in it, keeping constness consistent between both makes it simpler to work with. Patch is 21.70 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/102612.diff 3 Files Affected:
diff --git a/flang/lib/Lower/OpenMP/Decomposer.cpp b/flang/lib/Lower/OpenMP/Decomposer.cpp
index 66e4028c7a287f..dfd85897469e28 100644
--- a/flang/lib/Lower/OpenMP/Decomposer.cpp
+++ b/flang/lib/Lower/OpenMP/Decomposer.cpp
@@ -124,7 +124,7 @@ ConstructQueue buildConstructQueue(
return constructs;
}
-bool isLastItemInQueue(ConstructQueue::iterator item,
+bool isLastItemInQueue(ConstructQueue::const_iterator item,
const ConstructQueue &queue) {
return std::next(item) == queue.end();
}
diff --git a/flang/lib/Lower/OpenMP/Decomposer.h b/flang/lib/Lower/OpenMP/Decomposer.h
index a7851d8534e541..e85956ffe1a231 100644
--- a/flang/lib/Lower/OpenMP/Decomposer.h
+++ b/flang/lib/Lower/OpenMP/Decomposer.h
@@ -47,7 +47,7 @@ ConstructQueue buildConstructQueue(mlir::ModuleOp modOp,
llvm::omp::Directive compound,
const List<Clause> &clauses);
-bool isLastItemInQueue(ConstructQueue::iterator item,
+bool isLastItemInQueue(ConstructQueue::const_iterator item,
const ConstructQueue &queue);
} // namespace Fortran::lower::omp
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index bbde77c14f36a1..14723360f0ee1c 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -50,7 +50,7 @@ static void genOMPDispatch(lower::AbstractConverter &converter,
semantics::SemanticsContext &semaCtx,
lower::pft::Evaluation &eval, mlir::Location loc,
const ConstructQueue &queue,
- ConstructQueue::iterator item);
+ ConstructQueue::const_iterator item);
static lower::pft::Evaluation *
getCollapsedLoopEval(lower::pft::Evaluation &eval, int collapseValue) {
@@ -548,7 +548,7 @@ struct OpWithBodyGenInfo {
/// \param [in] item - item in the queue to generate body for.
static void createBodyOfOp(mlir::Operation &op, const OpWithBodyGenInfo &info,
const ConstructQueue &queue,
- ConstructQueue::iterator item) {
+ ConstructQueue::const_iterator item) {
fir::FirOpBuilder &firOpBuilder = info.converter.getFirOpBuilder();
auto insertMarker = [](fir::FirOpBuilder &builder) {
@@ -600,7 +600,8 @@ static void createBodyOfOp(mlir::Operation &op, const OpWithBodyGenInfo &info,
}
}
- if (ConstructQueue::iterator next = std::next(item); next != queue.end()) {
+ if (ConstructQueue::const_iterator next = std::next(item);
+ next != queue.end()) {
genOMPDispatch(info.converter, info.symTable, info.semaCtx, info.eval,
info.loc, queue, next);
} else {
@@ -698,7 +699,7 @@ static void genBodyOfTargetDataOp(
llvm::ArrayRef<mlir::Location> useDeviceLocs,
llvm::ArrayRef<const semantics::Symbol *> useDeviceSymbols,
const mlir::Location ¤tLocation, const ConstructQueue &queue,
- ConstructQueue::iterator item) {
+ ConstructQueue::const_iterator item) {
fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
mlir::Region ®ion = dataOp.getRegion();
@@ -751,7 +752,8 @@ static void genBodyOfTargetDataOp(
// Set the insertion point after the marker.
firOpBuilder.setInsertionPointAfter(undefMarker.getDefiningOp());
- if (ConstructQueue::iterator next = std::next(item); next != queue.end()) {
+ if (ConstructQueue::const_iterator next = std::next(item);
+ next != queue.end()) {
genOMPDispatch(converter, symTable, semaCtx, eval, currentLocation, queue,
next);
} else {
@@ -788,16 +790,15 @@ static void genIntermediateCommonBlockAccessors(
// This functions creates a block for the body of the targetOp's region. It adds
// all the symbols present in mapSymbols as block arguments to this block.
-static void
-genBodyOfTargetOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
- semantics::SemanticsContext &semaCtx,
- lower::pft::Evaluation &eval, mlir::omp::TargetOp &targetOp,
- llvm::ArrayRef<const semantics::Symbol *> mapSyms,
- llvm::ArrayRef<mlir::Location> mapSymLocs,
- llvm::ArrayRef<mlir::Type> mapSymTypes,
- DataSharingProcessor &dsp,
- const mlir::Location ¤tLocation,
- const ConstructQueue &queue, ConstructQueue::iterator item) {
+static void genBodyOfTargetOp(
+ lower::AbstractConverter &converter, lower::SymMap &symTable,
+ semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
+ mlir::omp::TargetOp &targetOp,
+ llvm::ArrayRef<const semantics::Symbol *> mapSyms,
+ llvm::ArrayRef<mlir::Location> mapSymLocs,
+ llvm::ArrayRef<mlir::Type> mapSymTypes, DataSharingProcessor &dsp,
+ const mlir::Location ¤tLocation, const ConstructQueue &queue,
+ ConstructQueue::const_iterator item) {
assert(mapSymTypes.size() == mapSymLocs.size());
fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
@@ -975,7 +976,8 @@ genBodyOfTargetOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
genIntermediateCommonBlockAccessors(converter, currentLocation, region,
mapSyms);
- if (ConstructQueue::iterator next = std::next(item); next != queue.end()) {
+ if (ConstructQueue::const_iterator next = std::next(item);
+ next != queue.end()) {
genOMPDispatch(converter, symTable, semaCtx, eval, currentLocation, queue,
next);
} else {
@@ -988,7 +990,7 @@ genBodyOfTargetOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
template <typename OpTy, typename... Args>
static OpTy genOpWithBody(const OpWithBodyGenInfo &info,
const ConstructQueue &queue,
- ConstructQueue::iterator item, Args &&...args) {
+ ConstructQueue::const_iterator item, Args &&...args) {
auto op = info.converter.getFirOpBuilder().create<OpTy>(
info.loc, std::forward<Args>(args)...);
createBodyOfOp(*op, info, queue, item);
@@ -1312,7 +1314,7 @@ static mlir::omp::BarrierOp
genBarrierOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
mlir::Location loc, const ConstructQueue &queue,
- ConstructQueue::iterator item) {
+ ConstructQueue::const_iterator item) {
return converter.getFirOpBuilder().create<mlir::omp::BarrierOp>(loc);
}
@@ -1320,7 +1322,7 @@ static mlir::omp::CriticalOp
genCriticalOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx,
lower::pft::Evaluation &eval, mlir::Location loc,
- const ConstructQueue &queue, ConstructQueue::iterator item,
+ const ConstructQueue &queue, ConstructQueue::const_iterator item,
const std::optional<parser::Name> &name) {
fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
mlir::FlatSymbolRefAttr nameAttr;
@@ -1351,7 +1353,7 @@ static mlir::omp::FlushOp
genFlushOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
mlir::Location loc, const ObjectList &objects,
- const ConstructQueue &queue, ConstructQueue::iterator item) {
+ const ConstructQueue &queue, ConstructQueue::const_iterator item) {
llvm::SmallVector<mlir::Value> operandRange;
genFlushClauses(converter, semaCtx, objects, item->clauses, loc,
operandRange);
@@ -1364,7 +1366,7 @@ static mlir::omp::LoopNestOp
genLoopNestOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx,
lower::pft::Evaluation &eval, mlir::Location loc,
- const ConstructQueue &queue, ConstructQueue::iterator item,
+ const ConstructQueue &queue, ConstructQueue::const_iterator item,
mlir::omp::LoopNestOperands &clauseOps,
llvm::ArrayRef<const semantics::Symbol *> iv,
llvm::ArrayRef<const semantics::Symbol *> wrapperSyms,
@@ -1391,7 +1393,7 @@ static mlir::omp::MaskedOp
genMaskedOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
mlir::Location loc, const ConstructQueue &queue,
- ConstructQueue::iterator item) {
+ ConstructQueue::const_iterator item) {
lower::StatementContext stmtCtx;
mlir::omp::MaskedOperands clauseOps;
genMaskedClauses(converter, semaCtx, stmtCtx, item->clauses, loc, clauseOps);
@@ -1406,7 +1408,7 @@ static mlir::omp::MasterOp
genMasterOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
mlir::Location loc, const ConstructQueue &queue,
- ConstructQueue::iterator item) {
+ ConstructQueue::const_iterator item) {
return genOpWithBody<mlir::omp::MasterOp>(
OpWithBodyGenInfo(converter, symTable, semaCtx, loc, eval,
llvm::omp::Directive::OMPD_master),
@@ -1417,7 +1419,7 @@ static mlir::omp::OrderedOp
genOrderedOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
mlir::Location loc, const ConstructQueue &queue,
- ConstructQueue::iterator item) {
+ ConstructQueue::const_iterator item) {
TODO(loc, "OMPD_ordered");
return nullptr;
}
@@ -1426,7 +1428,8 @@ static mlir::omp::OrderedRegionOp
genOrderedRegionOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx,
lower::pft::Evaluation &eval, mlir::Location loc,
- const ConstructQueue &queue, ConstructQueue::iterator item) {
+ const ConstructQueue &queue,
+ ConstructQueue::const_iterator item) {
mlir::omp::OrderedRegionOperands clauseOps;
genOrderedRegionClauses(converter, semaCtx, item->clauses, loc, clauseOps);
@@ -1440,7 +1443,7 @@ 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,
+ const ConstructQueue &queue, ConstructQueue::const_iterator item,
mlir::omp::ParallelOperands &clauseOps,
llvm::ArrayRef<const semantics::Symbol *> reductionSyms,
llvm::ArrayRef<mlir::Type> reductionTypes) {
@@ -1528,7 +1531,7 @@ static mlir::omp::SectionsOp
genSectionsOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx,
lower::pft::Evaluation &eval, mlir::Location loc,
- const ConstructQueue &queue, ConstructQueue::iterator item,
+ const ConstructQueue &queue, ConstructQueue::const_iterator item,
const parser::OmpSectionBlocks §ionBlocks) {
llvm::SmallVector<mlir::Type> reductionTypes;
llvm::SmallVector<const semantics::Symbol *> reductionSyms;
@@ -1644,7 +1647,7 @@ static mlir::omp::SingleOp
genSingleOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
mlir::Location loc, const ConstructQueue &queue,
- ConstructQueue::iterator item) {
+ ConstructQueue::const_iterator item) {
mlir::omp::SingleOperands clauseOps;
genSingleClauses(converter, semaCtx, item->clauses, loc, clauseOps);
@@ -1659,7 +1662,7 @@ static mlir::omp::TargetOp
genTargetOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
mlir::Location loc, const ConstructQueue &queue,
- ConstructQueue::iterator item) {
+ ConstructQueue::const_iterator item) {
fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
lower::StatementContext stmtCtx;
@@ -1793,7 +1796,8 @@ 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::iterator item) {
+ const ConstructQueue &queue,
+ ConstructQueue::const_iterator item) {
lower::StatementContext stmtCtx;
mlir::omp::TargetDataOperands clauseOps;
llvm::SmallVector<mlir::Type> useDeviceTypes;
@@ -1812,12 +1816,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::iterator item) {
+static OpTy genTargetEnterExitUpdateDataOp(
+ lower::AbstractConverter &converter, lower::SymMap &symTable,
+ semantics::SemanticsContext &semaCtx, mlir::Location loc,
+ const ConstructQueue &queue, ConstructQueue::const_iterator item) {
fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
lower::StatementContext stmtCtx;
@@ -1844,7 +1846,7 @@ static mlir::omp::TaskOp
genTaskOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
mlir::Location loc, const ConstructQueue &queue,
- ConstructQueue::iterator item) {
+ ConstructQueue::const_iterator item) {
lower::StatementContext stmtCtx;
mlir::omp::TaskOperands clauseOps;
genTaskClauses(converter, semaCtx, stmtCtx, item->clauses, loc, clauseOps);
@@ -1860,7 +1862,8 @@ static mlir::omp::TaskgroupOp
genTaskgroupOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx,
lower::pft::Evaluation &eval, mlir::Location loc,
- const ConstructQueue &queue, ConstructQueue::iterator item) {
+ const ConstructQueue &queue,
+ ConstructQueue::const_iterator item) {
mlir::omp::TaskgroupOperands clauseOps;
genTaskgroupClauses(converter, semaCtx, item->clauses, loc, clauseOps);
@@ -1875,7 +1878,8 @@ static mlir::omp::TaskwaitOp
genTaskwaitOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx,
lower::pft::Evaluation &eval, mlir::Location loc,
- const ConstructQueue &queue, ConstructQueue::iterator item) {
+ const ConstructQueue &queue,
+ ConstructQueue::const_iterator item) {
mlir::omp::TaskwaitOperands clauseOps;
genTaskwaitClauses(converter, semaCtx, item->clauses, loc, clauseOps);
return converter.getFirOpBuilder().create<mlir::omp::TaskwaitOp>(loc,
@@ -1886,7 +1890,8 @@ static mlir::omp::TaskyieldOp
genTaskyieldOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx,
lower::pft::Evaluation &eval, mlir::Location loc,
- const ConstructQueue &queue, ConstructQueue::iterator item) {
+ const ConstructQueue &queue,
+ ConstructQueue::const_iterator item) {
return converter.getFirOpBuilder().create<mlir::omp::TaskyieldOp>(loc);
}
@@ -1894,7 +1899,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) {
+ ConstructQueue::const_iterator item) {
lower::StatementContext stmtCtx;
mlir::omp::TeamsOperands clauseOps;
genTeamsClauses(converter, semaCtx, stmtCtx, item->clauses, loc, clauseOps);
@@ -1915,7 +1920,7 @@ static void genStandaloneDistribute(
lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
mlir::Location loc, const ConstructQueue &queue,
- ConstructQueue::iterator item, DataSharingProcessor &dsp) {
+ ConstructQueue::const_iterator item, DataSharingProcessor &dsp) {
lower::StatementContext stmtCtx;
mlir::omp::DistributeOperands distributeClauseOps;
@@ -1942,7 +1947,7 @@ static void genStandaloneDo(lower::AbstractConverter &converter,
semantics::SemanticsContext &semaCtx,
lower::pft::Evaluation &eval, mlir::Location loc,
const ConstructQueue &queue,
- ConstructQueue::iterator item,
+ ConstructQueue::const_iterator item,
DataSharingProcessor &dsp) {
lower::StatementContext stmtCtx;
@@ -1973,7 +1978,7 @@ static void genStandaloneParallel(lower::AbstractConverter &converter,
lower::pft::Evaluation &eval,
mlir::Location loc,
const ConstructQueue &queue,
- ConstructQueue::iterator item) {
+ ConstructQueue::const_iterator item) {
lower::StatementContext stmtCtx;
mlir::omp::ParallelOperands clauseOps;
@@ -1991,7 +1996,7 @@ static void genStandaloneSimd(lower::AbstractConverter &converter,
semantics::SemanticsContext &semaCtx,
lower::pft::Evaluation &eval, mlir::Location loc,
const ConstructQueue &queue,
- ConstructQueue::iterator item,
+ ConstructQueue::const_iterator item,
DataSharingProcessor &dsp) {
mlir::omp::SimdOperands simdClauseOps;
genSimdClauses(converter, semaCtx, item->clauses, loc, simdClauseOps);
@@ -2015,7 +2020,7 @@ static void genStandaloneTaskloop(
lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
mlir::Location loc, const ConstructQueue &queue,
- ConstructQueue::iterator item, DataSharingProcessor &dsp) {
+ ConstructQueue::const_iterator item, DataSharingProcessor &dsp) {
TODO(loc, "Taskloop construct");
}
@@ -2027,7 +2032,8 @@ static void genCompositeDistributeParallelDo(
lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
mlir::Location loc, const ConstructQueue &queue,
- ConstructQueue::iterator item, DataSharingProcessor &dsp) {
+ ConstructQueue::const_iterator item, DataSharingProcessor &dsp) {
+ assert(std::distance(item, queue.end()) == 3 && "Invalid leaf constructs");
TODO(loc, "Composite DISTRIBUTE PARALLEL DO");
}
@@ -2035,7 +2041,7 @@ static void genCompositeDistributeParallelDoSimd(
lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
mlir::Location loc, const ConstructQueue &queue,
- ConstructQueue::iterator item, DataSharingProcessor &dsp) {
+ ConstructQueue::const_iterator item, DataSharingProcessor &dsp) {
TODO(loc, "Composite DISTRIBUTE PARALLEL DO SIMD");
}
@@ -2043,7 +2049,7 @@ static void genCompositeDistributeSimd(
lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx, lower::pft::Ev...
[truncated]
|
skatrak
added a commit
that referenced
this pull request
Aug 20, 2024
After decomposition of OpenMP compound constructs and assignment of applicable clauses to each leaf construct, composite constructs are then combined again into a single element in the construct queue. This helped later lowering stages easily identify composite constructs. However, as a result of the re-composition stage, the same list of clauses is used to produce all MLIR operations corresponding to each leaf of the original composite construct. This undoes existing logic introducing implicit clauses and deciding to which leaf construct(s) each clause applies. This patch removes construct re-composition logic and updates Flang lowering to be able to identify composite constructs from a list of leaf constructs. As a result, the right set of clauses is produced for each operation representing a leaf of a composite construct. PR stack: - #102612 - #102613
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch replaces
ConstructQueue::iteratorarguments withConstructQueue::const_iteratorwhere it's used as a pointer to an element inside of aconst ConstructQueue &passed along with it.Since these functions don't intend to modify the list or any elements in it, keeping constness consistent between both makes it simpler to work with.
PR stack: