Skip to content

Commit db9856b

Browse files
luporlluporl
authored
[flang][OpenMP][NFC] Turn symTable into a reference (#119435)
Convert `DataSharingProcessor::symTable` from pointer to reference. This avoids accidental null pointer dereferences and makes it possible to use `symTable` when delayed privatization is disabled.
1 parent ccfcc91 commit db9856b

File tree

3 files changed

+29
-29
lines changed

3 files changed

+29
-29
lines changed

‎flang/lib/Lower/OpenMP/DataSharingProcessor.cpp‎

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ DataSharingProcessor::DataSharingProcessor(
3838
lower::AbstractConverter &converter, semantics::SemanticsContext &semaCtx,
3939
const List<Clause> &clauses, lower::pft::Evaluation &eval,
4040
bool shouldCollectPreDeterminedSymbols, bool useDelayedPrivatization,
41-
lower::SymMap *symTable)
41+
lower::SymMap &symTable)
4242
: converter(converter), semaCtx(semaCtx),
4343
firOpBuilder(converter.getFirOpBuilder()), clauses(clauses), eval(eval),
4444
shouldCollectPreDeterminedSymbols(shouldCollectPreDeterminedSymbols),
@@ -93,7 +93,7 @@ void DataSharingProcessor::insertDeallocs() {
9393
fir::ExtendedValue symExV = converter.getSymbolExtendedValue(*sym);
9494
mlir::omp::PrivateClauseOp privatizer = symToPrivatizer.at(sym);
9595

96-
lower::SymMapScope scope(*symTable);
96+
lower::SymMapScope scope(symTable);
9797
mlir::OpBuilder::InsertionGuard guard(firOpBuilder);
9898

9999
mlir::Region &deallocRegion = privatizer.getDeallocRegion();
@@ -102,8 +102,8 @@ void DataSharingProcessor::insertDeallocs() {
102102
&deallocRegion, /*insertPt=*/{}, symType, symLoc);
103103

104104
firOpBuilder.setInsertionPointToEnd(deallocEntryBlock);
105-
symTable->addSymbol(*sym,
106-
fir::substBase(symExV, deallocRegion.getArgument(0)));
105+
symTable.addSymbol(*sym,
106+
fir::substBase(symExV, deallocRegion.getArgument(0)));
107107

108108
converter.createHostAssociateVarCloneDealloc(*sym);
109109
firOpBuilder.create<mlir::omp::YieldOp>(hsb.getAddr().getLoc());
@@ -474,7 +474,7 @@ void DataSharingProcessor::doPrivatize(const semantics::Symbol *sym,
474474
isFirstPrivate ? mlir::omp::DataSharingClauseType::FirstPrivate
475475
: mlir::omp::DataSharingClauseType::Private);
476476
fir::ExtendedValue symExV = converter.getSymbolExtendedValue(*sym);
477-
lower::SymMapScope outerScope(*symTable);
477+
lower::SymMapScope outerScope(symTable);
478478

479479
// Populate the `alloc` region.
480480
{
@@ -491,10 +491,10 @@ void DataSharingProcessor::doPrivatize(const semantics::Symbol *sym,
491491
evaluate::IsSimplyContiguous(*sym, converter.getFoldingContext()))
492492
.first;
493493

494-
symTable->addSymbol(*sym, localExV);
495-
lower::SymMapScope innerScope(*symTable);
494+
symTable.addSymbol(*sym, localExV);
495+
lower::SymMapScope innerScope(symTable);
496496
cloneSymbol(sym);
497-
mlir::Value cloneAddr = symTable->shallowLookupSymbol(*sym).getAddr();
497+
mlir::Value cloneAddr = symTable.shallowLookupSymbol(*sym).getAddr();
498498
mlir::Type cloneType = cloneAddr.getType();
499499

500500
// A `convert` op is required for variables that are storage associated
@@ -522,25 +522,24 @@ void DataSharingProcessor::doPrivatize(const semantics::Symbol *sym,
522522
auto addSymbol = [&](unsigned argIdx, bool force = false) {
523523
symExV.match(
524524
[&](const fir::MutableBoxValue &box) {
525-
symTable->addSymbol(
525+
symTable.addSymbol(
526526
*sym, fir::substBase(box, copyRegion.getArgument(argIdx)),
527527
force);
528528
},
529529
[&](const auto &box) {
530-
symTable->addSymbol(*sym, copyRegion.getArgument(argIdx), force);
530+
symTable.addSymbol(*sym, copyRegion.getArgument(argIdx), force);
531531
});
532532
};
533533

534534
addSymbol(0, true);
535-
lower::SymMapScope innerScope(*symTable);
535+
lower::SymMapScope innerScope(symTable);
536536
addSymbol(1);
537537

538538
auto ip = firOpBuilder.saveInsertionPoint();
539539
copyFirstPrivateSymbol(sym, &ip);
540540

541541
firOpBuilder.create<mlir::omp::YieldOp>(
542-
hsb.getAddr().getLoc(),
543-
symTable->shallowLookupSymbol(*sym).getAddr());
542+
hsb.getAddr().getLoc(), symTable.shallowLookupSymbol(*sym).getAddr());
544543
}
545544

546545
return result;

‎flang/lib/Lower/OpenMP/DataSharingProcessor.h‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ class DataSharingProcessor {
8686
lower::pft::Evaluation &eval;
8787
bool shouldCollectPreDeterminedSymbols;
8888
bool useDelayedPrivatization;
89-
lower::SymMap *symTable;
89+
lower::SymMap &symTable;
9090
OMPConstructSymbolVisitor visitor;
9191

9292
bool needBarrier();
@@ -122,8 +122,7 @@ class DataSharingProcessor {
122122
const List<Clause> &clauses,
123123
lower::pft::Evaluation &eval,
124124
bool shouldCollectPreDeterminedSymbols,
125-
bool useDelayedPrivatization = false,
126-
lower::SymMap *symTable = nullptr);
125+
bool useDelayedPrivatization, lower::SymMap &symTable);
127126

128127
// Privatisation is split into two steps.
129128
// Step1 performs cloning of all privatisation clauses and copying for

‎flang/lib/Lower/OpenMP/OpenMP.cpp‎

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -718,7 +718,8 @@ static void createBodyOfOp(mlir::Operation &op, const OpWithBodyGenInfo &info,
718718
std::optional<DataSharingProcessor> tempDsp;
719719
if (privatize && !info.dsp) {
720720
tempDsp.emplace(info.converter, info.semaCtx, *info.clauses, info.eval,
721-
Fortran::lower::omp::isLastItemInQueue(item, queue));
721+
Fortran::lower::omp::isLastItemInQueue(item, queue),
722+
/*useDelayedPrivatization=*/false, info.symTable);
722723
tempDsp->processStep1();
723724
}
724725

@@ -1423,7 +1424,7 @@ static void genLoopOp(lower::AbstractConverter &converter,
14231424

14241425
DataSharingProcessor dsp(converter, semaCtx, item->clauses, eval,
14251426
/*shouldCollectPreDeterminedSymbols=*/true,
1426-
/*useDelayedPrivatization=*/true, &symTable);
1427+
/*useDelayedPrivatization=*/true, symTable);
14271428
dsp.processStep1(&loopClauseOps);
14281429

14291430
mlir::omp::LoopNestOperands loopNestClauseOps;
@@ -1544,7 +1545,8 @@ genSectionsOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
15441545
// Insert privatizations before SECTIONS
15451546
lower::SymMapScope scope(symTable);
15461547
DataSharingProcessor dsp(converter, semaCtx, item->clauses, eval,
1547-
lower::omp::isLastItemInQueue(item, queue));
1548+
lower::omp::isLastItemInQueue(item, queue),
1549+
/*useDelayedPrivatization=*/false, symTable);
15481550
dsp.processStep1();
15491551

15501552
List<Clause> nonDsaClauses;
@@ -1695,7 +1697,7 @@ genTargetOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
16951697
DataSharingProcessor dsp(converter, semaCtx, item->clauses, eval,
16961698
/*shouldCollectPreDeterminedSymbols=*/
16971699
lower::omp::isLastItemInQueue(item, queue),
1698-
/*useDelayedPrivatization=*/true, &symTable);
1700+
/*useDelayedPrivatization=*/true, symTable);
16991701
dsp.processStep1(&clauseOps);
17001702

17011703
// 5.8.1 Implicit Data-Mapping Attribute Rules
@@ -1896,7 +1898,7 @@ genTaskOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
18961898

18971899
DataSharingProcessor dsp(converter, semaCtx, item->clauses, eval,
18981900
lower::omp::isLastItemInQueue(item, queue),
1899-
/*useDelayedPrivatization=*/true, &symTable);
1901+
/*useDelayedPrivatization=*/true, symTable);
19001902
dsp.processStep1(&clauseOps);
19011903

19021904
EntryBlockArgs taskArgs;
@@ -2011,7 +2013,7 @@ static void genStandaloneDistribute(lower::AbstractConverter &converter,
20112013

20122014
DataSharingProcessor dsp(converter, semaCtx, item->clauses, eval,
20132015
/*shouldCollectPreDeterminedSymbols=*/true,
2014-
enableDelayedPrivatizationStaging, &symTable);
2016+
enableDelayedPrivatizationStaging, symTable);
20152017
dsp.processStep1(&distributeClauseOps);
20162018

20172019
mlir::omp::LoopNestOperands loopNestClauseOps;
@@ -2045,7 +2047,7 @@ static void genStandaloneDo(lower::AbstractConverter &converter,
20452047

20462048
DataSharingProcessor dsp(converter, semaCtx, item->clauses, eval,
20472049
/*shouldCollectPreDeterminedSymbols=*/true,
2048-
enableDelayedPrivatizationStaging, &symTable);
2050+
enableDelayedPrivatizationStaging, symTable);
20492051
dsp.processStep1(&wsloopClauseOps);
20502052

20512053
mlir::omp::LoopNestOperands loopNestClauseOps;
@@ -2084,7 +2086,7 @@ static void genStandaloneParallel(lower::AbstractConverter &converter,
20842086
if (enableDelayedPrivatization) {
20852087
dsp.emplace(converter, semaCtx, item->clauses, eval,
20862088
lower::omp::isLastItemInQueue(item, queue),
2087-
/*useDelayedPrivatization=*/true, &symTable);
2089+
/*useDelayedPrivatization=*/true, symTable);
20882090
dsp->processStep1(&parallelClauseOps);
20892091
}
20902092

@@ -2113,7 +2115,7 @@ static void genStandaloneSimd(lower::AbstractConverter &converter,
21132115
// TODO: Support delayed privatization.
21142116
DataSharingProcessor dsp(converter, semaCtx, item->clauses, eval,
21152117
/*shouldCollectPreDeterminedSymbols=*/true,
2116-
/*useDelayedPrivatization=*/false, &symTable);
2118+
/*useDelayedPrivatization=*/false, symTable);
21172119
dsp.processStep1();
21182120

21192121
mlir::omp::LoopNestOperands loopNestClauseOps;
@@ -2167,7 +2169,7 @@ static void genCompositeDistributeParallelDo(
21672169

21682170
DataSharingProcessor dsp(converter, semaCtx, doItem->clauses, eval,
21692171
/*shouldCollectPreDeterminedSymbols=*/true,
2170-
/*useDelayedPrivatization=*/true, &symTable);
2172+
/*useDelayedPrivatization=*/true, symTable);
21712173
dsp.processStep1(&parallelClauseOps);
21722174

21732175
EntryBlockArgs parallelArgs;
@@ -2235,7 +2237,7 @@ static void genCompositeDistributeParallelDoSimd(
22352237

22362238
DataSharingProcessor dsp(converter, semaCtx, simdItem->clauses, eval,
22372239
/*shouldCollectPreDeterminedSymbols=*/true,
2238-
/*useDelayedPrivatization=*/true, &symTable);
2240+
/*useDelayedPrivatization=*/true, symTable);
22392241
dsp.processStep1(&parallelClauseOps);
22402242

22412243
EntryBlockArgs parallelArgs;
@@ -2323,7 +2325,7 @@ static void genCompositeDistributeSimd(lower::AbstractConverter &converter,
23232325
// TODO: Support delayed privatization.
23242326
DataSharingProcessor dsp(converter, semaCtx, simdItem->clauses, eval,
23252327
/*shouldCollectPreDeterminedSymbols=*/true,
2326-
/*useDelayedPrivatization=*/false, &symTable);
2328+
/*useDelayedPrivatization=*/false, symTable);
23272329
dsp.processStep1();
23282330

23292331
// Pass the innermost leaf construct's clauses because that's where COLLAPSE
@@ -2380,7 +2382,7 @@ static void genCompositeDoSimd(lower::AbstractConverter &converter,
23802382
// TODO: Support delayed privatization.
23812383
DataSharingProcessor dsp(converter, semaCtx, simdItem->clauses, eval,
23822384
/*shouldCollectPreDeterminedSymbols=*/true,
2383-
/*useDelayedPrivatization=*/false, &symTable);
2385+
/*useDelayedPrivatization=*/false, symTable);
23842386
dsp.processStep1();
23852387

23862388
// Pass the innermost leaf construct's clauses because that's where COLLAPSE

0 commit comments

Comments
 (0)