[flang-commits] [flang] [Flang][OpenMP] NFC: Simplify handling of insertion points (PR #89221)

[Sergio Afonso via flang-commits]

https://github.com/skatrak created https://github.com/llvm/llvm-project/pull/89221

This patch replaces some `saveInsertionPoint`, `restoreInsertionPoint` call pairs for an `InsertionGuard` instance where it makes sense within Flang OpenMP lowering to make further modifications less error-prone.

>From b3c2bc4dc2e422281ac5a252938a423564f81da0 Mon Sep 17 00:00:00 2001
From: Sergio Afonso <safonsof at amd.com>
Date: Thu, 18 Apr 2024 12:51:15 +0100
Subject: [PATCH] [Flang][OpenMP] NFC: Simplify handling of insertion points

This patch replaces some `saveInsertionPoint`, `restoreInsertionPoint` call
pairs for an `InsertionGuard` instance where it makes sense within Flang OpenMP
lowering to make further modifications less error-prone.
---
 flang/lib/Lower/OpenMP/DataSharingProcessor.cpp | 11 ++++-------
 flang/lib/Lower/OpenMP/OpenMP.cpp               |  4 +---
 flang/lib/Lower/OpenMP/ReductionProcessor.cpp   |  3 +--
 3 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
index 5a42e6a6aa4175..8bb2f83282b556 100644
--- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
@@ -136,7 +136,7 @@ void DataSharingProcessor::insertBarrier() {
 
 void DataSharingProcessor::insertLastPrivateCompare(mlir::Operation *op) {
   bool cmpCreated = false;
-  mlir::OpBuilder::InsertPoint localInsPt = firOpBuilder.saveInsertionPoint();
+  mlir::OpBuilder::InsertionGuard guard(firOpBuilder);
   for (const omp::Clause &clause : clauses) {
     if (clause.id != llvm::omp::OMPC_lastprivate)
       continue;
@@ -203,12 +203,11 @@ void DataSharingProcessor::insertLastPrivateCompare(mlir::Operation *op) {
           // Lastprivate operation is inserted at the end
           // of the lexically last section in the sections
           // construct
-          mlir::OpBuilder::InsertPoint unstructuredSectionsIP =
-              firOpBuilder.saveInsertionPoint();
+          mlir::OpBuilder::InsertionGuard unstructuredSectionsGuard(
+              firOpBuilder);
           mlir::Operation *lastOper = op->getRegion(0).back().getTerminator();
           firOpBuilder.setInsertionPoint(lastOper);
           lastPrivIP = firOpBuilder.saveInsertionPoint();
-          firOpBuilder.restoreInsertionPoint(unstructuredSectionsIP);
         }
       }
     } else if (mlir::isa<mlir::omp::WsloopOp>(op)) {
@@ -268,7 +267,6 @@ void DataSharingProcessor::insertLastPrivateCompare(mlir::Operation *op) {
            "simd/worksharing-loop");
     }
   }
-  firOpBuilder.restoreInsertionPoint(localInsPt);
 }
 
 void DataSharingProcessor::collectSymbols(
@@ -372,7 +370,7 @@ void DataSharingProcessor::doPrivatize(
                 uniquePrivatizerName))
       return existingPrivatizer;
 
-    auto ip = firOpBuilder.saveInsertionPoint();
+    mlir::OpBuilder::InsertionGuard guard(firOpBuilder);
     firOpBuilder.setInsertionPoint(&moduleOp.getBodyRegion().front(),
                                    moduleOp.getBodyRegion().front().begin());
     auto result = firOpBuilder.create<mlir::omp::PrivateClauseOp>(
@@ -424,7 +422,6 @@ void DataSharingProcessor::doPrivatize(
     }
 
     symTable->popScope();
-    firOpBuilder.restoreInsertionPoint(ip);
     return result;
   }();
 
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index bb38082b245ef5..ceefd6090a0ee9 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -133,7 +133,7 @@ static void threadPrivatizeVars(Fortran::lower::AbstractConverter &converter,
                                 Fortran::lower::pft::Evaluation &eval) {
   fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
   mlir::Location currentLocation = converter.getCurrentLocation();
-  mlir::OpBuilder::InsertPoint insPt = firOpBuilder.saveInsertionPoint();
+  mlir::OpBuilder::InsertionGuard guard(firOpBuilder);
   firOpBuilder.setInsertionPointToStart(firOpBuilder.getAllocaBlock());
 
   // If the symbol corresponds to the original ThreadprivateOp, use the symbol
@@ -196,8 +196,6 @@ static void threadPrivatizeVars(Fortran::lower::AbstractConverter &converter,
         getExtendedValue(sexv, symThreadprivateValue);
     converter.bindSymbol(*sym, symThreadprivateExv);
   }
-
-  firOpBuilder.restoreInsertionPoint(insPt);
 }
 
 static mlir::Operation *
diff --git a/flang/lib/Lower/OpenMP/ReductionProcessor.cpp b/flang/lib/Lower/OpenMP/ReductionProcessor.cpp
index f42386fe2736dd..6a91ee4b32f639 100644
--- a/flang/lib/Lower/OpenMP/ReductionProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/ReductionProcessor.cpp
@@ -486,9 +486,8 @@ createReductionInitRegion(fir::FirOpBuilder &builder, mlir::Location loc,
     assert(cstNeedsDealloc.has_value() &&
            "createTempFromMold decides this statically");
     if (cstNeedsDealloc.has_value() && *cstNeedsDealloc != false) {
-      auto insPt = builder.saveInsertionPoint();
+      mlir::OpBuilder::InsertionGuard guard(builder);
       createReductionCleanupRegion(builder, loc, reductionDecl);
-      builder.restoreInsertionPoint(insPt);
     }
 
     // Put the temporary inside of a box: