Skip to content

Commit 096f795

Browse files
committed
[flang][OpenMP] Fix privatization when critical is present
When a critical construct is present inside another construct where privatizations may occur, such as a parallel construct, some privatizations are skipped if the corresponding symbols are defined inside the critical section only (see the example below). This happens because, while critical constructs have a "body", they don't have a separate scope (which makes sense, since no privatizations can occur in them). Because of this, in semantics phase, it's not possible to insert a new host association symbol, but instead the symbol from the enclosing context is used directly. This makes symbol collection in DataSharingProcessor consider the new symbol to be defined by the critical construct, instead of by the enclosing one, which causes the privatization to be skipped. Example: ``` !$omp parallel default(firstprivate) !$omp critical i = 200 !$omp end critical !$omp end parallel ``` This patch fixes this by identifying constructs where privatizations may not happen and skipping them during the collection of nested symbols. Currently, this seems to happen only with critical constructs, but others can be easily added to the skip list, if needed. Fixes llvm#75767
1 parent 0977504 commit 096f795

File tree

2 files changed

+34
-1
lines changed

2 files changed

+34
-1
lines changed

flang/lib/Lower/OpenMP/DataSharingProcessor.cpp

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,15 @@ void DataSharingProcessor::insertLastPrivateCompare(mlir::Operation *op) {
262262
}
263263
}
264264

265+
static bool mayHavePrivatizations(const lower::pft::Evaluation &eval) {
266+
if (const parser::OpenMPConstruct *construct =
267+
eval.getIf<parser::OpenMPConstruct>()) {
268+
if (std::holds_alternative<parser::OpenMPCriticalConstruct>(construct->u))
269+
return false;
270+
}
271+
return true;
272+
}
273+
265274
static const parser::CharBlock *
266275
getSource(const semantics::SemanticsContext &semaCtx,
267276
const lower::pft::Evaluation &eval) {
@@ -305,7 +314,7 @@ void DataSharingProcessor::collectSymbolsInNestedRegions(
305314
llvm::SetVector<const semantics::Symbol *> &symbolsInNestedRegions) {
306315
for (lower::pft::Evaluation &nestedEval : eval.getNestedEvaluations()) {
307316
if (nestedEval.hasNestedEvaluations()) {
308-
if (nestedEval.isConstruct())
317+
if (nestedEval.isConstruct() || !mayHavePrivatizations(nestedEval))
309318
// Recursively look for OpenMP constructs within `nestedEval`'s region
310319
collectSymbolsInNestedRegions(nestedEval, flag, symbolsInNestedRegions);
311320
else {

flang/test/Lower/OpenMP/critical.f90

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,3 +51,27 @@ subroutine predetermined_privatization()
5151
end do
5252
!$omp end parallel do
5353
end
54+
55+
! https://github.com/llvm/llvm-project/issues/75767
56+
!CHECK-LABEL: func @_QPnested_privatization(
57+
subroutine nested_privatization()
58+
integer :: i
59+
60+
!CHECK: %[[I:.*]] = fir.alloca i32 {bindc_name = "i", uniq_name = "_QFnested_privatizationEi"}
61+
!CHECK: %[[I_DECL:.*]]:2 = hlfir.declare %[[I]] {uniq_name = "_QFnested_privatizationEi"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
62+
!CHECK: omp.parallel {
63+
!CHECK: %[[PRIV_I:.*]] = fir.alloca i32 {bindc_name = "i", pinned, uniq_name = "_QFnested_privatizationEi"}
64+
!CHECK: %[[PRIV_I_DECL:.*]]:2 = hlfir.declare %[[PRIV_I]] {uniq_name = "_QFnested_privatizationEi"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
65+
!CHECK: %[[TEMP:.*]] = fir.load %[[I_DECL]]#0 : !fir.ref<i32>
66+
!CHECK: hlfir.assign %[[TEMP]] to %[[PRIV_I_DECL]]#0 temporary_lhs : i32, !fir.ref<i32>
67+
!$omp parallel default(firstprivate)
68+
!CHECK: omp.critical {
69+
!$omp critical
70+
!CHECK: %[[C200:.*]] = arith.constant 200 : i32
71+
!CHECK: hlfir.assign %[[C200]] to %[[PRIV_I_DECL]]#0 : i32, !fir.ref<i32>
72+
i = 200
73+
!CHECK: }
74+
!$omp end critical
75+
!CHECK: }
76+
!$omp end parallel
77+
end subroutine

0 commit comments

Comments
 (0)