Skip to content

Commit d1da5ed

Browse files
NimishMishraNimishMishra
committed
[flang][OpenMP] Fix construct privatization in default clause
1 parent 333aad7 commit d1da5ed

File tree

7 files changed

+102
-24
lines changed

7 files changed

+102
-24
lines changed

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

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

136136
/// Collect the set of symbols with \p flag in \p eval
137-
/// region if \p collectSymbols is true. Likewise, collect the
137+
/// region if \p collectSymbols is true. Otherwise, collect the
138138
/// set of the host symbols with \p flag of the associated symbols in \p eval
139-
/// region if collectHostAssociatedSymbols is true.
139+
/// region if collectHostAssociatedSymbols is true. This allows gathering
140+
/// host association details of symbols particularly in nested directives
141+
/// irrespective of \p flag \p, and can be useful where host
142+
/// association details are needed in flag-agnostic manner.
140143
virtual void collectSymbolSet(
141144
pft::Evaluation &eval,
142145
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
@@ -815,7 +815,7 @@ class FirConverter : public Fortran::lower::AbstractConverter {
815815
bool collectSymbol) {
816816
if (collectSymbol && oriSymbol.test(flag))
817817
symbolSet.insert(&oriSymbol);
818-
if (checkHostAssociatedSymbols)
818+
else if (checkHostAssociatedSymbols)
819819
if (const auto *details{
820820
oriSymbol
821821
.detailsIf<Fortran::semantics::HostAssocDetails>()})

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

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -269,21 +269,39 @@ void DataSharingProcessor::insertLastPrivateCompare(mlir::Operation *op) {
269269
}
270270
}
271271

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

289307
void DataSharingProcessor::collectDefaultSymbols() {
@@ -333,7 +351,6 @@ void DataSharingProcessor::defaultPrivatize(
333351
!sym->GetUltimate().has<Fortran::semantics::DerivedTypeDetails>() &&
334352
!sym->GetUltimate().has<Fortran::semantics::NamelistDetails>() &&
335353
!symbolsInNestedRegions.contains(sym) &&
336-
!symbolsInParentRegions.contains(sym) &&
337354
!privatizedSymbols.contains(sym))
338355
doPrivatize(sym, clauseOps, privateSyms);
339356
}

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ class DataSharingProcessor {
3939
llvm::SetVector<const Fortran::semantics::Symbol *> privatizedSymbols;
4040
llvm::SetVector<const Fortran::semantics::Symbol *> defaultSymbols;
4141
llvm::SetVector<const Fortran::semantics::Symbol *> symbolsInNestedRegions;
42-
llvm::SetVector<const Fortran::semantics::Symbol *> symbolsInParentRegions;
4342
Fortran::lower::AbstractConverter &converter;
4443
fir::FirOpBuilder &firOpBuilder;
4544
omp::List<omp::Clause> clauses;
@@ -49,6 +48,11 @@ class DataSharingProcessor {
4948

5049
bool needBarrier();
5150
void collectSymbols(Fortran::semantics::Symbol::Flag flag);
51+
void collectSymbolsInNestedRegions(
52+
Fortran::lower::pft::Evaluation &eval,
53+
Fortran::semantics::Symbol::Flag flag,
54+
llvm::SetVector<const Fortran::semantics::Symbol *>
55+
&symbolsInNestedRegions);
5256
void collectOmpObjectListSymbol(
5357
const omp::ObjectList &objects,
5458
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: 55 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
@@ -384,6 +388,7 @@ subroutine skipped_default_clause_checks()
384388
!$omp end parallel
385389
end subroutine
386390

391+
387392
!CHECK: func.func @_QPthreadprivate_with_default() {
388393
!CHECK: %[[VAR_I:.*]] = fir.alloca i32 {bindc_name = "i", uniq_name = "_QFthreadprivate_with_defaultEi"}
389394
!CHECK: %[[VAR_I_DECLARE:.*]] = hlfir.declare %[[VAR_I]] {uniq_name = "_QFthreadprivate_with_defaultEi"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
@@ -412,3 +417,49 @@ subroutine threadprivate_with_default
412417
end do
413418
!$omp end parallel do
414419
end subroutine
420+
421+
subroutine nested_constructs
422+
!CHECK: %[[I:.*]] = fir.alloca i32 {bindc_name = "i", uniq_name = "_QFnested_constructsEi"}
423+
!CHECK: %[[I_DECL:.*]]:2 = hlfir.declare %[[I]] {{.*}}
424+
!CHECK: %[[J:.*]] = fir.alloca i32 {bindc_name = "j", uniq_name = "_QFnested_constructsEj"}
425+
!CHECK: %[[J_DECL:.*]]:2 = hlfir.declare %[[J]] {{.*}}
426+
!CHECK: %[[Y:.*]] = fir.alloca i32 {bindc_name = "y", uniq_name = "_QFnested_constructsEy"}
427+
!CHECK: %[[Y_DECL:.*]]:2 = hlfir.declare %[[Y]] {{.*}}
428+
!CHECK: %[[Z:.*]] = fir.alloca i32 {bindc_name = "z", uniq_name = "_QFnested_constructsEz"}
429+
!CHECK: %[[Z_DECL:.*]]:2 = hlfir.declare %[[Z]] {{.*}}
430+
431+
integer :: y, z
432+
!CHECK: omp.parallel {
433+
!CHECK: %[[INNER_J:.*]] = fir.alloca i32 {bindc_name = "j", pinned}
434+
!CHECK: %[[INNER_J_DECL:.*]]:2 = hlfir.declare %[[INNER_J]] {{.*}}
435+
!CHECK: %[[INNER_I:.*]] = fir.alloca i32 {bindc_name = "i", pinned}
436+
!CHECK: %[[INNER_I_DECL:.*]]:2 = hlfir.declare %[[INNER_I]] {{.*}}
437+
!CHECK: %[[INNER_Y:.*]] = fir.alloca i32 {bindc_name = "y", pinned, uniq_name = "_QFnested_constructsEy"}
438+
!CHECK: %[[INNER_Y_DECL:.*]]:2 = hlfir.declare %[[INNER_Y]] {{.*}}
439+
!CHECK: %[[TEMP:.*]] = fir.load %[[Y_DECL]]#0 : !fir.ref<i32>
440+
!CHECK: hlfir.assign %[[TEMP]] to %[[INNER_Y_DECL]]#0 temporary_lhs : i32, !fir.ref<i32>
441+
!CHECK: %[[INNER_Z:.*]] = fir.alloca i32 {bindc_name = "z", pinned, uniq_name = "_QFnested_constructsEz"}
442+
!CHECK: %[[INNER_Z_DECL:.*]]:2 = hlfir.declare %[[INNER_Z]] {{.*}}
443+
!$omp parallel default(private) firstprivate(y)
444+
!CHECK: {{.*}} = fir.do_loop {{.*}} {
445+
do i = 1, 10
446+
!CHECK: %[[CONST_1:.*]] = arith.constant 1 : i32
447+
!CHECK: hlfir.assign %[[CONST_1]] to %[[INNER_Y_DECL]]#0 : i32, !fir.ref<i32>
448+
y = 1
449+
!CHECK: {{.*}} = fir.do_loop {{.*}} {
450+
do j = 1, 10
451+
!CHECK: %[[CONST_20:.*]] = arith.constant 20 : i32
452+
!CHECK: hlfir.assign %[[CONST_20]] to %[[INNER_Z_DECL]]#0 : i32, !fir.ref<i32>
453+
z = 20
454+
!CHECK: omp.parallel {
455+
!CHECK: %[[NESTED_Y:.*]] = fir.alloca i32 {bindc_name = "y", pinned, uniq_name = "_QFnested_constructsEy"}
456+
!CHECK: %[[NESTED_Y_DECL:.*]]:2 = hlfir.declare %[[NESTED_Y]] {{.*}}
457+
!CHECK: %[[CONST_2:.*]] = arith.constant 2 : i32
458+
!CHECK: hlfir.assign %[[CONST_2]] to %[[NESTED_Y_DECL]]#0 : i32, !fir.ref<i32>
459+
!$omp parallel default(private)
460+
y = 2
461+
!$omp end parallel
462+
end do
463+
end do
464+
!$omp end parallel
465+
end subroutine

0 commit comments

Comments
 (0)