[flang-commits] [flang] [Flang][Lower] NFC: Introduce SymMapScope helper class (PR #107866)
Sergio Afonso via flang-commits
flang-commits at lists.llvm.org
Mon Sep 9 07:26:18 PDT 2024
https://github.com/skatrak created https://github.com/llvm/llvm-project/pull/107866
This patch creates a simple RAII wrapper class for `SymMap` to make it easier to use and prevent a missing matching `popScope()` for a `pushScope()` call on simple use cases.
Some push-pop pairs are replaced with instances of the new class by this patch.
>From 594b207aee8582f7fa3097f02ebdf9a91f3b267f Mon Sep 17 00:00:00 2001
From: Sergio Afonso <safonsof at amd.com>
Date: Mon, 9 Sep 2024 14:43:44 +0100
Subject: [PATCH] [Flang][Lower] NFC: Introduce SymMapScope helper class
This patch creates a simple RAII wrapper class for `SymMap` to make it easier
to use and prevent a missing matching `popScope()` for a `pushScope()` call on
simple use cases.
Some push-pop pairs are replaced with instances of the new class by this patch.
---
flang/include/flang/Lower/SymbolMap.h | 10 ++++++++++
flang/lib/Lower/Bridge.cpp | 9 +++------
flang/lib/Lower/OpenMP/DataSharingProcessor.cpp | 15 ++++-----------
flang/lib/Lower/OpenMP/OpenMP.cpp | 6 ++----
4 files changed, 19 insertions(+), 21 deletions(-)
diff --git a/flang/include/flang/Lower/SymbolMap.h b/flang/include/flang/Lower/SymbolMap.h
index a55e4b133fe0a8..c03f9afd40801c 100644
--- a/flang/include/flang/Lower/SymbolMap.h
+++ b/flang/include/flang/Lower/SymbolMap.h
@@ -334,6 +334,16 @@ class SymMap {
llvm::SmallVector<std::pair<AcDoVar, mlir::Value>> impliedDoStack;
};
+/// RAII wrapper for SymMap.
+class SymMapScope {
+public:
+ explicit SymMapScope(SymMap &map) : map(map) { map.pushScope(); }
+ ~SymMapScope() { map.popScope(); }
+
+private:
+ SymMap ↦
+};
+
} // namespace Fortran::lower
#endif // FORTRAN_LOWER_SYMBOLMAP_H
diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index 7cdecb788425a0..f78e952e4782c7 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -2597,14 +2597,12 @@ class FirConverter : public Fortran::lower::AbstractConverter {
stmt.t)
.value();
if (lowerToHighLevelFIR()) {
- mlir::OpBuilder::InsertPoint insertPt = builder->saveInsertionPoint();
- localSymbols.pushScope();
+ mlir::OpBuilder::InsertionGuard guard(*builder);
+ Fortran::lower::SymMapScope scope(localSymbols);
genForallNest(concurrentHeader);
genFIR(std::get<Fortran::parser::UnlabeledStatement<
Fortran::parser::ForallAssignmentStmt>>(stmt.t)
.statement);
- localSymbols.popScope();
- builder->restoreInsertionPoint(insertPt);
return;
}
prepareExplicitSpace(stmt);
@@ -2824,7 +2822,7 @@ class FirConverter : public Fortran::lower::AbstractConverter {
}
void genFIR(const Fortran::parser::CUFKernelDoConstruct &kernel) {
- localSymbols.pushScope();
+ Fortran::lower::SymMapScope scope(localSymbols);
const Fortran::parser::CUFKernelDoConstruct::Directive &dir =
std::get<Fortran::parser::CUFKernelDoConstruct::Directive>(kernel.t);
@@ -3015,7 +3013,6 @@ class FirConverter : public Fortran::lower::AbstractConverter {
builder->create<fir::FirEndOp>(loc);
builder->setInsertionPointAfter(op);
- localSymbols.popScope();
}
void genFIR(const Fortran::parser::OpenMPConstruct &omp) {
diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
index 592c77289f9f29..5f4138e0f63e73 100644
--- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
@@ -93,8 +93,7 @@ void DataSharingProcessor::insertDeallocs() {
fir::ExtendedValue symExV = converter.getSymbolExtendedValue(*sym);
mlir::omp::PrivateClauseOp privatizer = symToPrivatizer.at(sym);
- symTable->pushScope();
-
+ lower::SymMapScope scope(*symTable);
mlir::OpBuilder::InsertionGuard guard(firOpBuilder);
mlir::Region &deallocRegion = privatizer.getDeallocRegion();
@@ -108,8 +107,6 @@ void DataSharingProcessor::insertDeallocs() {
converter.createHostAssociateVarCloneDealloc(*sym);
firOpBuilder.create<mlir::omp::YieldOp>(hsb.getAddr().getLoc());
-
- symTable->popScope();
}
}
@@ -488,8 +485,7 @@ void DataSharingProcessor::doPrivatize(const semantics::Symbol *sym,
isFirstPrivate ? mlir::omp::DataSharingClauseType::FirstPrivate
: mlir::omp::DataSharingClauseType::Private);
fir::ExtendedValue symExV = converter.getSymbolExtendedValue(*sym);
-
- symTable->pushScope();
+ lower::SymMapScope outerScope(*symTable);
// Populate the `alloc` region.
{
@@ -507,7 +503,7 @@ void DataSharingProcessor::doPrivatize(const semantics::Symbol *sym,
.first;
symTable->addSymbol(*sym, localExV);
- symTable->pushScope();
+ lower::SymMapScope innerScope(*symTable);
cloneSymbol(sym);
mlir::Value cloneAddr = symTable->shallowLookupSymbol(*sym).getAddr();
mlir::Type cloneType = cloneAddr.getType();
@@ -523,7 +519,6 @@ void DataSharingProcessor::doPrivatize(const semantics::Symbol *sym,
firOpBuilder.create<mlir::omp::YieldOp>(hsb.getAddr().getLoc(),
yieldedValue);
- symTable->popScope();
}
// Populate the `copy` region if this is a `firstprivate`.
@@ -548,7 +543,7 @@ void DataSharingProcessor::doPrivatize(const semantics::Symbol *sym,
};
addSymbol(0, true);
- symTable->pushScope();
+ lower::SymMapScope innerScope(*symTable);
addSymbol(1);
auto ip = firOpBuilder.saveInsertionPoint();
@@ -557,10 +552,8 @@ void DataSharingProcessor::doPrivatize(const semantics::Symbol *sym,
firOpBuilder.create<mlir::omp::YieldOp>(
hsb.getAddr().getLoc(),
symTable->shallowLookupSymbol(*sym).getAddr());
- symTable->popScope();
}
- symTable->popScope();
return result;
}();
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 8b77f1ac6b4ff2..233aacb40354d4 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -1546,7 +1546,7 @@ genSectionsOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
auto &builder = converter.getFirOpBuilder();
// Insert privatizations before SECTIONS
- symTable.pushScope();
+ lower::SymMapScope scope(symTable);
DataSharingProcessor dsp(converter, semaCtx, item->clauses, eval,
lower::omp::isLastItemInQueue(item, queue));
dsp.processStep1();
@@ -1643,7 +1643,6 @@ genSectionsOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
if (clauseOps.nowait && !lastprivates.empty())
builder.create<mlir::omp::BarrierOp>(loc);
- symTable.popScope();
return sectionsOp;
}
@@ -2900,9 +2899,8 @@ void Fortran::lower::genOpenMPConstruct(lower::AbstractConverter &converter,
semantics::SemanticsContext &semaCtx,
lower::pft::Evaluation &eval,
const parser::OpenMPConstruct &omp) {
- symTable.pushScope();
+ lower::SymMapScope scope(symTable);
genOMP(converter, symTable, semaCtx, eval, omp);
- symTable.popScope();
}
void Fortran::lower::genOpenMPDeclarativeConstruct(
More information about the flang-commits
mailing list