Skip to content

Commit 3b12905

Browse files
NimishMishraNimishMishra
committed
[flang][OpenMP] Fix construct privatization in default clause
1 parent 599adf3 commit 3b12905

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
@@ -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
firOpBuilder.restoreInsertionPoint(localInsPt);
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() {
@@ -329,7 +347,6 @@ void DataSharingProcessor::defaultPrivatize() {
329347
!sym->GetUltimate().has<Fortran::semantics::DerivedTypeDetails>() &&
330348
!sym->GetUltimate().has<Fortran::semantics::NamelistDetails>() &&
331349
!symbolsInNestedRegions.contains(sym) &&
332-
!symbolsInParentRegions.contains(sym) &&
333350
!privatizedSymbols.contains(sym))
334351
doPrivatize(sym);
335352
}

‎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)