Skip to content

Commit 9153b0a

Browse files
author
nimishra
committed
[flang][OpenMP] Fix construct privatization in default clause
Change-Id: I4ce23a31336daf2fd54df925649bc0f85f246227
1 parent 5b544b5 commit 9153b0a

File tree

7 files changed

+101
-24
lines changed

7 files changed

+101
-24
lines changed

‎flang/include/flang/Lower/AbstractConverter.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,9 +130,12 @@ class AbstractConverter {
130130
virtual bool isPresentShallowLookup(Fortran::semantics::Symbol &sym) = 0;
131131

132132
/// Collect the set of symbols with \p flag in \p eval
133-
/// region if \p collectSymbols is true. Likewise, collect the
133+
/// region if \p collectSymbols is true. Otherwise, collect the
134134
/// set of the host symbols with \p flag of the associated symbols in \p eval
135-
/// region if collectHostAssociatedSymbols is true.
135+
/// region if collectHostAssociatedSymbols is true. This allows gathering
136+
/// host association details of symbols particularly in nested directives
137+
/// irrespective of \p flag \p, and can be useful where host
138+
/// association details are needed in flag-agnostic manner.
136139
virtual void collectSymbolSet(
137140
pft::Evaluation &eval,
138141
llvm::SetVector<const Fortran::semantics::Symbol *> &symbolSet,

‎flang/lib/Lower/Bridge.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -814,7 +814,7 @@ class FirConverter : public Fortran::lower::AbstractConverter {
814814
bool collectSymbol) {
815815
if (collectSymbol && oriSymbol.test(flag))
816816
symbolSet.insert(&oriSymbol);
817-
if (checkHostAssociatedSymbols)
817+
else if (checkHostAssociatedSymbols)
818818
if (const auto *details{
819819
oriSymbol
820820
.detailsIf<Fortran::semantics::HostAssocDetails>()})

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

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -268,21 +268,39 @@ void DataSharingProcessor::insertLastPrivateCompare(mlir::Operation *op) {
268268
firOpBuilder.restoreInsertionPoint(localInsPt);
269269
}
270270

271+
void DataSharingProcessor::collectSymbolsInNestedRegions(
272+
Fortran::lower::pft::Evaluation &eval,
273+
Fortran::semantics::Symbol::Flag flag,
274+
llvm::SetVector<const Fortran::semantics::Symbol *>
275+
&symbolsInNestedRegions) {
276+
for (Fortran::lower::pft::Evaluation &nestedEval :
277+
eval.getNestedEvaluations()) {
278+
if (nestedEval.hasNestedEvaluations()) {
279+
if (nestedEval.isConstruct())
280+
// Recursively look for OpenMP constructs within `nestedEval`'s region
281+
collectSymbolsInNestedRegions(nestedEval, flag, symbolsInNestedRegions);
282+
else
283+
converter.collectSymbolSet(nestedEval, symbolsInNestedRegions, flag,
284+
/*collectSymbols=*/true,
285+
/*collectHostAssociatedSymbols=*/false);
286+
}
287+
}
288+
}
289+
290+
// Collect symbols to be default privatized in two steps.
291+
// In step 1, collect all symbols in `eval` that match `flag`
292+
// into `defaultSymbols`.
293+
// In step 2, for nested constructs (if any), if and only if
294+
// the nested construct is an OpenMP construct, collect those nested
295+
// symbols skipping host assicauted symbols into `symbolsInNestedRegions`.
296+
// Finally, in current context, lower all symbols in the set
297+
// `defaultSymbols` - `symbolsInNestedRegions`.
271298
void DataSharingProcessor::collectSymbols(
272299
Fortran::semantics::Symbol::Flag flag) {
273300
converter.collectSymbolSet(eval, defaultSymbols, flag,
274301
/*collectSymbols=*/true,
275302
/*collectHostAssociatedSymbols=*/true);
276-
for (Fortran::lower::pft::Evaluation &e : eval.getNestedEvaluations()) {
277-
if (e.hasNestedEvaluations())
278-
converter.collectSymbolSet(e, symbolsInNestedRegions, flag,
279-
/*collectSymbols=*/true,
280-
/*collectHostAssociatedSymbols=*/false);
281-
else
282-
converter.collectSymbolSet(e, symbolsInParentRegions, flag,
283-
/*collectSymbols=*/false,
284-
/*collectHostAssociatedSymbols=*/true);
285-
}
303+
collectSymbolsInNestedRegions(eval, flag, symbolsInNestedRegions);
286304
}
287305

288306
void DataSharingProcessor::collectDefaultSymbols() {
@@ -327,7 +345,6 @@ void DataSharingProcessor::defaultPrivatize() {
327345
!sym->GetUltimate().has<Fortran::semantics::DerivedTypeDetails>() &&
328346
!sym->GetUltimate().has<Fortran::semantics::NamelistDetails>() &&
329347
!symbolsInNestedRegions.contains(sym) &&
330-
!symbolsInParentRegions.contains(sym) &&
331348
!privatizedSymbols.contains(sym))
332349
doPrivatize(sym);
333350
}

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@ class DataSharingProcessor {
5050
llvm::SetVector<const Fortran::semantics::Symbol *> privatizedSymbols;
5151
llvm::SetVector<const Fortran::semantics::Symbol *> defaultSymbols;
5252
llvm::SetVector<const Fortran::semantics::Symbol *> symbolsInNestedRegions;
53-
llvm::SetVector<const Fortran::semantics::Symbol *> symbolsInParentRegions;
5453
Fortran::lower::AbstractConverter &converter;
5554
fir::FirOpBuilder &firOpBuilder;
5655
omp::List<omp::Clause> clauses;
@@ -61,6 +60,11 @@ class DataSharingProcessor {
6160

6261
bool needBarrier();
6362
void collectSymbols(Fortran::semantics::Symbol::Flag flag);
63+
void collectSymbolsInNestedRegions(
64+
Fortran::lower::pft::Evaluation &eval,
65+
Fortran::semantics::Symbol::Flag flag,
66+
llvm::SetVector<const Fortran::semantics::Symbol *>
67+
&symbolsInNestedRegions);
6468
void collectOmpObjectListSymbol(
6569
const omp::ObjectList &objects,
6670
llvm::SetVector<const Fortran::semantics::Symbol *> &symbolSet);

‎flang/test/Lower/OpenMP/FIR/default-clause.f90

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,6 @@ subroutine nested_default_clause_tests
193193
!CHECK: %[[PRIVATE_X:.*]] = fir.alloca i32 {bindc_name = "x", pinned, uniq_name = "_QFnested_default_clause_testsEx"}
194194
!CHECK: %[[PRIVATE_Y:.*]] = fir.alloca i32 {bindc_name = "y", pinned, uniq_name = "_QFnested_default_clause_testsEy"}
195195
!CHECK: %[[PRIVATE_Z:.*]] = fir.alloca i32 {bindc_name = "z", pinned, uniq_name = "_QFnested_default_clause_testsEz"}
196-
!CHECK: %[[PRIVATE_W:.*]] = fir.alloca i32 {bindc_name = "w", pinned, uniq_name = "_QFnested_default_clause_testsEw"}
197196
!CHECK: omp.parallel {
198197
!CHECK: %[[PRIVATE_INNER_X:.*]] = fir.alloca i32 {bindc_name = "x", pinned, uniq_name = "_QFnested_default_clause_testsEx"}
199198
!CHECK: %[[temp:.*]] = fir.load %[[PRIVATE_X]] : !fir.ref<i32>
@@ -206,10 +205,12 @@ subroutine nested_default_clause_tests
206205
!CHECK: omp.terminator
207206
!CHECK: }
208207
!CHECK: omp.parallel {
208+
!CHECK: %[[PRIVATE_INNER_Z:.*]] = fir.alloca i32 {bindc_name = "z", pinned, uniq_name =
209+
!"_QFnested_default_clause_testsEz"}
209210
!CHECK: %[[PRIVATE_INNER_W:.*]] = fir.alloca i32 {bindc_name = "w", pinned, uniq_name = "_QFnested_default_clause_testsEw"}
210211
!CHECK: %[[PRIVATE_INNER_X:.*]] = fir.alloca i32 {bindc_name = "x", pinned, uniq_name = "_QFnested_default_clause_testsEx"}
211212
!CHECK: %[[temp_1:.*]] = fir.load %[[PRIVATE_INNER_X]] : !fir.ref<i32>
212-
!CHECK: %[[temp_2:.*]] = fir.load %[[PRIVATE_Z]] : !fir.ref<i32>
213+
!CHECK: %[[temp_2:.*]] = fir.load %[[PRIVATE_INNER_Z]] : !fir.ref<i32>
213214
!CHECK: %[[result:.*]] = arith.addi %{{.*}}, %{{.*}} : i32
214215
!CHECK: fir.store %[[result]] to %[[PRIVATE_INNER_W]] : !fir.ref<i32>
215216
!CHECK: omp.terminator

‎flang/test/Lower/OpenMP/default-clause-byref.f90

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -226,8 +226,6 @@ subroutine nested_default_clause_tests
226226
!CHECK: %[[PRIVATE_Y_DECL:.*]]:2 = hlfir.declare %[[PRIVATE_Y]] {uniq_name = "_QFnested_default_clause_testsEy"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
227227
!CHECK: %[[PRIVATE_Z:.*]] = fir.alloca i32 {bindc_name = "z", pinned, uniq_name = "_QFnested_default_clause_testsEz"}
228228
!CHECK: %[[PRIVATE_Z_DECL:.*]]:2 = hlfir.declare %[[PRIVATE_Z]] {uniq_name = "_QFnested_default_clause_testsEz"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
229-
!CHECK: %[[PRIVATE_W:.*]] = fir.alloca i32 {bindc_name = "w", pinned, uniq_name = "_QFnested_default_clause_testsEw"}
230-
!CHECK: %[[PRIVATE_W_DECL:.*]]:2 = hlfir.declare %[[PRIVATE_W]] {uniq_name = "_QFnested_default_clause_testsEw"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
231229
!CHECK: omp.parallel {
232230
!CHECK: %[[PRIVATE_INNER_X:.*]] = fir.alloca i32 {bindc_name = "x", pinned, uniq_name = "_QFnested_default_clause_testsEx"}
233231
!CHECK: %[[PRIVATE_INNER_X_DECL:.*]]:2 = hlfir.declare %[[PRIVATE_INNER_X]] {uniq_name = "_QFnested_default_clause_testsEx"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
@@ -242,12 +240,16 @@ subroutine nested_default_clause_tests
242240
!CHECK: omp.terminator
243241
!CHECK: }
244242
!CHECK: omp.parallel {
243+
!CHECK: %[[PRIVATE_INNER_Z:.*]] = fir.alloca i32 {bindc_name = "z", pinned, uniq_name =
244+
!"_QFnested_default_clause_testsEz"}
245+
!CHECK: %[[PRIVATE_INNER_Z_DECL:.*]]:2 = hlfir.declare %[[PRIVATE_INNER_Z]] {uniq_name =
246+
!"_QFnested_default_clause_testsEz"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
245247
!CHECK: %[[PRIVATE_INNER_W:.*]] = fir.alloca i32 {bindc_name = "w", pinned, uniq_name = "_QFnested_default_clause_testsEw"}
246248
!CHECK: %[[PRIVATE_INNER_W_DECL:.*]]:2 = hlfir.declare %[[PRIVATE_INNER_W]] {uniq_name = "_QFnested_default_clause_testsEw"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
247249
!CHECK: %[[PRIVATE_INNER_X:.*]] = fir.alloca i32 {bindc_name = "x", pinned, uniq_name = "_QFnested_default_clause_testsEx"}
248250
!CHECK: %[[PRIVATE_INNER_X_DECL:.*]]:2 = hlfir.declare %[[PRIVATE_INNER_X]] {uniq_name = "_QFnested_default_clause_testsEx"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
249251
!CHECK: %[[TEMP_1:.*]] = fir.load %[[PRIVATE_INNER_X_DECL]]#0 : !fir.ref<i32>
250-
!CHECK: %[[TEMP_2:.*]] = fir.load %[[PRIVATE_Z_DECL]]#0 : !fir.ref<i32>
252+
!CHECK: %[[TEMP_2:.*]] = fir.load %[[PRIVATE_INNER_Z_DECL]]#0 : !fir.ref<i32>
251253
!CHECK: %[[RESULT:.*]] = arith.addi %{{.*}}, %{{.*}} : i32
252254
!CHECK: hlfir.assign %[[RESULT]] to %[[PRIVATE_INNER_W_DECL]]#0 : i32, !fir.ref<i32>
253255
!CHECK: omp.terminator

‎flang/test/Lower/OpenMP/default-clause.f90

Lines changed: 54 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ program default_clause_lowering
149149
end program default_clause_lowering
150150

151151
subroutine nested_default_clause_tests
152-
integer :: x, y, z, w, k, a
152+
integer :: x, y, z, w, k
153153
!CHECK: %[[K:.*]] = fir.alloca i32 {bindc_name = "k", uniq_name = "_QFnested_default_clause_testsEk"}
154154
!CHECK: %[[K_DECL:.*]]:2 = hlfir.declare %[[K]] {uniq_name = "_QFnested_default_clause_testsEk"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
155155
!CHECK: %[[W:.*]] = fir.alloca i32 {bindc_name = "w", uniq_name = "_QFnested_default_clause_testsEw"}
@@ -221,13 +221,13 @@ subroutine nested_default_clause_tests
221221

222222

223223
!CHECK: omp.parallel {
224+
!CHECK: %[[PRIVATE_X:.*]] = fir.alloca i32 {bindc_name = "x", pinned, uniq_name =
225+
!"_QFnested_default_clause_testsEx"}
224226
!CHECK: %[[PRIVATE_X_DECL:.*]]:2 = hlfir.declare %[[PRIVATE_X]] {uniq_name = "_QFnested_default_clause_testsEx"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
225227
!CHECK: %[[PRIVATE_Y:.*]] = fir.alloca i32 {bindc_name = "y", pinned, uniq_name = "_QFnested_default_clause_testsEy"}
226228
!CHECK: %[[PRIVATE_Y_DECL:.*]]:2 = hlfir.declare %[[PRIVATE_Y]] {uniq_name = "_QFnested_default_clause_testsEy"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
227229
!CHECK: %[[PRIVATE_Z:.*]] = fir.alloca i32 {bindc_name = "z", pinned, uniq_name = "_QFnested_default_clause_testsEz"}
228230
!CHECK: %[[PRIVATE_Z_DECL:.*]]:2 = hlfir.declare %[[PRIVATE_Z]] {uniq_name = "_QFnested_default_clause_testsEz"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
229-
!CHECK: %[[PRIVATE_W:.*]] = fir.alloca i32 {bindc_name = "w", pinned, uniq_name = "_QFnested_default_clause_testsEw"}
230-
!CHECK: %[[PRIVATE_W_DECL:.*]]:2 = hlfir.declare %[[PRIVATE_W]] {uniq_name = "_QFnested_default_clause_testsEw"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
231231
!CHECK: omp.parallel {
232232
!CHECK: %[[PRIVATE_INNER_X:.*]] = fir.alloca i32 {bindc_name = "x", pinned, uniq_name = "_QFnested_default_clause_testsEx"}
233233
!CHECK: %[[PRIVATE_INNER_X_DECL:.*]]:2 = hlfir.declare %[[PRIVATE_INNER_X]] {uniq_name = "_QFnested_default_clause_testsEx"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
@@ -242,12 +242,16 @@ subroutine nested_default_clause_tests
242242
!CHECK: omp.terminator
243243
!CHECK: }
244244
!CHECK: omp.parallel {
245+
!CHECK: %[[PRIVATE_INNER_Z:.*]] = fir.alloca i32 {bindc_name = "z", pinned, uniq_name =
246+
!"_QFnested_default_clause_testsEz"}
247+
!CHECK: %[[PRIVATE_INNER_Z_DECL:.*]]:2 = hlfir.declare %[[PRIVATE_INNER_Z]] {uniq_name =
248+
!"_QFnested_default_clause_testsEz"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
245249
!CHECK: %[[PRIVATE_INNER_W:.*]] = fir.alloca i32 {bindc_name = "w", pinned, uniq_name = "_QFnested_default_clause_testsEw"}
246250
!CHECK: %[[PRIVATE_INNER_W_DECL:.*]]:2 = hlfir.declare %[[PRIVATE_INNER_W]] {uniq_name = "_QFnested_default_clause_testsEw"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
247251
!CHECK: %[[PRIVATE_INNER_X:.*]] = fir.alloca i32 {bindc_name = "x", pinned, uniq_name = "_QFnested_default_clause_testsEx"}
248252
!CHECK: %[[PRIVATE_INNER_X_DECL:.*]]:2 = hlfir.declare %[[PRIVATE_INNER_X]] {uniq_name = "_QFnested_default_clause_testsEx"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
249253
!CHECK: %[[TEMP_1:.*]] = fir.load %[[PRIVATE_INNER_X_DECL]]#0 : !fir.ref<i32>
250-
!CHECK: %[[TEMP_2:.*]] = fir.load %[[PRIVATE_Z_DECL]]#0 : !fir.ref<i32>
254+
!CHECK: %[[TEMP_2:.*]] = fir.load %[[PRIVATE_INNER_Z_DECL]]#0 : !fir.ref<i32>
251255
!CHECK: %[[RESULT:.*]] = arith.addi %{{.*}}, %{{.*}} : i32
252256
!CHECK: hlfir.assign %[[RESULT]] to %[[PRIVATE_INNER_W_DECL]]#0 : i32, !fir.ref<i32>
253257
!CHECK: omp.terminator
@@ -383,3 +387,49 @@ subroutine skipped_default_clause_checks()
383387
iii=it(11)
384388
!$omp end parallel
385389
end subroutine
390+
391+
subroutine nested_constructs
392+
!CHECK: %[[I:.*]] = fir.alloca i32 {bindc_name = "i", uniq_name = "_QFnested_constructsEi"}
393+
!CHECK: %[[I_DECL:.*]]:2 = hlfir.declare %[[I]] {{.*}}
394+
!CHECK: %[[J:.*]] = fir.alloca i32 {bindc_name = "j", uniq_name = "_QFnested_constructsEj"}
395+
!CHECK: %[[J_DECL:.*]]:2 = hlfir.declare %[[J]] {{.*}}
396+
!CHECK: %[[Y:.*]] = fir.alloca i32 {bindc_name = "y", uniq_name = "_QFnested_constructsEy"}
397+
!CHECK: %[[Y_DECL:.*]]:2 = hlfir.declare %[[Y]] {{.*}}
398+
!CHECK: %[[Z:.*]] = fir.alloca i32 {bindc_name = "z", uniq_name = "_QFnested_constructsEz"}
399+
!CHECK: %[[Z_DECL:.*]]:2 = hlfir.declare %[[Z]] {{.*}}
400+
401+
integer :: y, z
402+
!CHECK: omp.parallel {
403+
!CHECK: %[[INNER_J:.*]] = fir.alloca i32 {bindc_name = "j", pinned}
404+
!CHECK: %[[INNER_J_DECL:.*]]:2 = hlfir.declare %[[INNER_J]] {{.*}}
405+
!CHECK: %[[INNER_I:.*]] = fir.alloca i32 {bindc_name = "i", pinned}
406+
!CHECK: %[[INNER_I_DECL:.*]]:2 = hlfir.declare %[[INNER_I]] {{.*}}
407+
!CHECK: %[[INNER_Y:.*]] = fir.alloca i32 {bindc_name = "y", pinned, uniq_name = "_QFnested_constructsEy"}
408+
!CHECK: %[[INNER_Y_DECL:.*]]:2 = hlfir.declare %[[INNER_Y]] {{.*}}
409+
!CHECK: %[[TEMP:.*]] = fir.load %[[Y_DECL]]#0 : !fir.ref<i32>
410+
!CHECK: hlfir.assign %[[TEMP]] to %[[INNER_Y_DECL]]#0 temporary_lhs : i32, !fir.ref<i32>
411+
!CHECK: %[[INNER_Z:.*]] = fir.alloca i32 {bindc_name = "z", pinned, uniq_name = "_QFnested_constructsEz"}
412+
!CHECK: %[[INNER_Z_DECL:.*]]:2 = hlfir.declare %[[INNER_Z]] {{.*}}
413+
!$omp parallel default(private) firstprivate(y)
414+
!CHECK: {{.*}} = fir.do_loop {{.*}} {
415+
do i = 1, 10
416+
!CHECK: %[[CONST_1:.*]] = arith.constant 1 : i32
417+
!CHECK: hlfir.assign %[[CONST_1]] to %[[INNER_Y_DECL]]#0 : i32, !fir.ref<i32>
418+
y = 1
419+
!CHECK: {{.*}} = fir.do_loop {{.*}} {
420+
do j = 1, 10
421+
!CHECK: %[[CONST_20:.*]] = arith.constant 20 : i32
422+
!CHECK: hlfir.assign %[[CONST_20]] to %[[INNER_Z_DECL]]#0 : i32, !fir.ref<i32>
423+
z = 20
424+
!CHECK: omp.parallel {
425+
!CHECK: %[[NESTED_Y:.*]] = fir.alloca i32 {bindc_name = "y", pinned, uniq_name = "_QFnested_constructsEy"}
426+
!CHECK: %[[NESTED_Y_DECL:.*]]:2 = hlfir.declare %[[NESTED_Y]] {{.*}}
427+
!CHECK: %[[CONST_2:.*]] = arith.constant 2 : i32
428+
!CHECK: hlfir.assign %[[CONST_2]] to %[[NESTED_Y_DECL]]#0 : i32, !fir.ref<i32>
429+
!$omp parallel default(private)
430+
y = 2
431+
!$omp end parallel
432+
end do
433+
end do
434+
!$omp end parallel
435+
end subroutine

0 commit comments

Comments
 (0)