[flang-commits] [flang] [Flang][OpenMP] NFC: Use ConstructQueue::const_iterator (PR #102612)

via flang-commits flang-commits at lists.llvm.org
Fri Aug 9 06:04:24 PDT 2024 

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-flang-fir-hlfir

Author: Sergio Afonso (skatrak)

<details>
<summary>Changes</summary>

This patch replaces `ConstructQueue::iterator` arguments with `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.

---

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:

- (modified) flang/lib/Lower/OpenMP/Decomposer.cpp (+1-1) 
- (modified) flang/lib/Lower/OpenMP/Decomposer.h (+1-1) 
- (modified) flang/lib/Lower/OpenMP/OpenMP.cpp (+58-52) 


``````````diff
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 &currentLocation, const ConstructQueue &queue,
-    ConstructQueue::iterator item) {
+    ConstructQueue::const_iterator item) {
   fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
   mlir::Region &region = 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 &currentLocation,
-                  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 &currentLocation, 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 &sectionBlocks) {
   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]

``````````

</details>


https://github.com/llvm/llvm-project/pull/102612

More information about the flang-commits mailing list