Skip to content

Commit 433ca3e

Browse files
authored
[Flang][Lower] Introduce SymMapScope helper class (NFC) (llvm#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.
1 parent 516f08b commit 433ca3e

File tree

4 files changed

+19
-21
lines changed

4 files changed

+19
-21
lines changed

flang/include/flang/Lower/SymbolMap.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,16 @@ class SymMap {
334334
llvm::SmallVector<std::pair<AcDoVar, mlir::Value>> impliedDoStack;
335335
};
336336

337+
/// RAII wrapper for SymMap.
338+
class SymMapScope {
339+
public:
340+
explicit SymMapScope(SymMap &map) : map(map) { map.pushScope(); }
341+
~SymMapScope() { map.popScope(); }
342+
343+
private:
344+
SymMap &map;
345+
};
346+
337347
} // namespace Fortran::lower
338348

339349
#endif // FORTRAN_LOWER_SYMBOLMAP_H

flang/lib/Lower/Bridge.cpp

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2597,14 +2597,12 @@ class FirConverter : public Fortran::lower::AbstractConverter {
25972597
stmt.t)
25982598
.value();
25992599
if (lowerToHighLevelFIR()) {
2600-
mlir::OpBuilder::InsertPoint insertPt = builder->saveInsertionPoint();
2601-
localSymbols.pushScope();
2600+
mlir::OpBuilder::InsertionGuard guard(*builder);
2601+
Fortran::lower::SymMapScope scope(localSymbols);
26022602
genForallNest(concurrentHeader);
26032603
genFIR(std::get<Fortran::parser::UnlabeledStatement<
26042604
Fortran::parser::ForallAssignmentStmt>>(stmt.t)
26052605
.statement);
2606-
localSymbols.popScope();
2607-
builder->restoreInsertionPoint(insertPt);
26082606
return;
26092607
}
26102608
prepareExplicitSpace(stmt);
@@ -2824,7 +2822,7 @@ class FirConverter : public Fortran::lower::AbstractConverter {
28242822
}
28252823

28262824
void genFIR(const Fortran::parser::CUFKernelDoConstruct &kernel) {
2827-
localSymbols.pushScope();
2825+
Fortran::lower::SymMapScope scope(localSymbols);
28282826
const Fortran::parser::CUFKernelDoConstruct::Directive &dir =
28292827
std::get<Fortran::parser::CUFKernelDoConstruct::Directive>(kernel.t);
28302828

@@ -3015,7 +3013,6 @@ class FirConverter : public Fortran::lower::AbstractConverter {
30153013

30163014
builder->create<fir::FirEndOp>(loc);
30173015
builder->setInsertionPointAfter(op);
3018-
localSymbols.popScope();
30193016
}
30203017

30213018
void genFIR(const Fortran::parser::OpenMPConstruct &omp) {

flang/lib/Lower/OpenMP/DataSharingProcessor.cpp

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,7 @@ void DataSharingProcessor::insertDeallocs() {
9393
fir::ExtendedValue symExV = converter.getSymbolExtendedValue(*sym);
9494
mlir::omp::PrivateClauseOp privatizer = symToPrivatizer.at(sym);
9595

96-
symTable->pushScope();
97-
96+
lower::SymMapScope scope(*symTable);
9897
mlir::OpBuilder::InsertionGuard guard(firOpBuilder);
9998

10099
mlir::Region &deallocRegion = privatizer.getDeallocRegion();
@@ -108,8 +107,6 @@ void DataSharingProcessor::insertDeallocs() {
108107

109108
converter.createHostAssociateVarCloneDealloc(*sym);
110109
firOpBuilder.create<mlir::omp::YieldOp>(hsb.getAddr().getLoc());
111-
112-
symTable->popScope();
113110
}
114111
}
115112

@@ -488,8 +485,7 @@ void DataSharingProcessor::doPrivatize(const semantics::Symbol *sym,
488485
isFirstPrivate ? mlir::omp::DataSharingClauseType::FirstPrivate
489486
: mlir::omp::DataSharingClauseType::Private);
490487
fir::ExtendedValue symExV = converter.getSymbolExtendedValue(*sym);
491-
492-
symTable->pushScope();
488+
lower::SymMapScope outerScope(*symTable);
493489

494490
// Populate the `alloc` region.
495491
{
@@ -507,7 +503,7 @@ void DataSharingProcessor::doPrivatize(const semantics::Symbol *sym,
507503
.first;
508504

509505
symTable->addSymbol(*sym, localExV);
510-
symTable->pushScope();
506+
lower::SymMapScope innerScope(*symTable);
511507
cloneSymbol(sym);
512508
mlir::Value cloneAddr = symTable->shallowLookupSymbol(*sym).getAddr();
513509
mlir::Type cloneType = cloneAddr.getType();
@@ -523,7 +519,6 @@ void DataSharingProcessor::doPrivatize(const semantics::Symbol *sym,
523519

524520
firOpBuilder.create<mlir::omp::YieldOp>(hsb.getAddr().getLoc(),
525521
yieldedValue);
526-
symTable->popScope();
527522
}
528523

529524
// Populate the `copy` region if this is a `firstprivate`.
@@ -548,7 +543,7 @@ void DataSharingProcessor::doPrivatize(const semantics::Symbol *sym,
548543
};
549544

550545
addSymbol(0, true);
551-
symTable->pushScope();
546+
lower::SymMapScope innerScope(*symTable);
552547
addSymbol(1);
553548

554549
auto ip = firOpBuilder.saveInsertionPoint();
@@ -557,10 +552,8 @@ void DataSharingProcessor::doPrivatize(const semantics::Symbol *sym,
557552
firOpBuilder.create<mlir::omp::YieldOp>(
558553
hsb.getAddr().getLoc(),
559554
symTable->shallowLookupSymbol(*sym).getAddr());
560-
symTable->popScope();
561555
}
562556

563-
symTable->popScope();
564557
return result;
565558
}();
566559

flang/lib/Lower/OpenMP/OpenMP.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1546,7 +1546,7 @@ genSectionsOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
15461546
auto &builder = converter.getFirOpBuilder();
15471547

15481548
// Insert privatizations before SECTIONS
1549-
symTable.pushScope();
1549+
lower::SymMapScope scope(symTable);
15501550
DataSharingProcessor dsp(converter, semaCtx, item->clauses, eval,
15511551
lower::omp::isLastItemInQueue(item, queue));
15521552
dsp.processStep1();
@@ -1643,7 +1643,6 @@ genSectionsOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
16431643
if (clauseOps.nowait && !lastprivates.empty())
16441644
builder.create<mlir::omp::BarrierOp>(loc);
16451645

1646-
symTable.popScope();
16471646
return sectionsOp;
16481647
}
16491648

@@ -2900,9 +2899,8 @@ void Fortran::lower::genOpenMPConstruct(lower::AbstractConverter &converter,
29002899
semantics::SemanticsContext &semaCtx,
29012900
lower::pft::Evaluation &eval,
29022901
const parser::OpenMPConstruct &omp) {
2903-
symTable.pushScope();
2902+
lower::SymMapScope scope(symTable);
29042903
genOMP(converter, symTable, semaCtx, eval, omp);
2905-
symTable.popScope();
29062904
}
29072905

29082906
void Fortran::lower::genOpenMPDeclarativeConstruct(

0 commit comments

Comments
 (0)