Skip to content

Commit 4d52dde

Browse files
committed
Implement lupori's derive type initialization patch
For reference see both #120295 and #121808 This changes the barrier logic. See discussion here: https://github.com/llvm/llvm-project/pull/120295/files#r1910663473
1 parent 0015bb3 commit 4d52dde

File tree

7 files changed

+134
-31
lines changed

7 files changed

+134
-31
lines changed

flang/lib/Lower/OpenMP/DataSharingProcessor.cpp

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ void DataSharingProcessor::cloneSymbol(const semantics::Symbol *sym) {
168168

169169
if (needInitClone()) {
170170
Fortran::lower::initializeCloneAtRuntime(converter, *sym, symTable);
171-
callsInitClone = true;
171+
mightHaveReadMoldArg = true;
172172
}
173173
}
174174

@@ -220,7 +220,8 @@ bool DataSharingProcessor::needBarrier() {
220220
// Emit implicit barrier for linear clause. Maybe on somewhere else.
221221
for (const semantics::Symbol *sym : allPrivatizedSymbols) {
222222
if (sym->test(semantics::Symbol::Flag::OmpLastPrivate) &&
223-
(sym->test(semantics::Symbol::Flag::OmpFirstPrivate) || callsInitClone))
223+
(sym->test(semantics::Symbol::Flag::OmpFirstPrivate) ||
224+
mightHaveReadMoldArg))
224225
return true;
225226
}
226227
return false;
@@ -578,7 +579,14 @@ void DataSharingProcessor::doPrivatize(const semantics::Symbol *sym,
578579
populateByRefInitAndCleanupRegions(
579580
firOpBuilder, symLoc, argType, /*scalarInitValue=*/nullptr, initBlock,
580581
result.getInitPrivateArg(), result.getInitMoldArg(),
581-
result.getDeallocRegion(), /*isPrivate=*/true, sym);
582+
result.getDeallocRegion(),
583+
isFirstPrivate ? DeclOperationKind::FirstPrivate
584+
: DeclOperationKind::Private,
585+
sym);
586+
// TODO: currently there are false positives from dead uses of the mold
587+
// arg
588+
if (!result.getInitMoldArg().getUses().empty())
589+
mightHaveReadMoldArg = true;
582590
}
583591

584592
// Populate the `copy` region if this is a `firstprivate`.

flang/lib/Lower/OpenMP/DataSharingProcessor.h

Lines changed: 1 addition & 1 deletion
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-
bool callsInitClone = false;
89+
bool mightHaveReadMoldArg = false;
9090
lower::SymMap &symTable;
9191
OMPConstructSymbolVisitor visitor;
9292

flang/lib/Lower/OpenMP/PrivateReductionUtils.cpp

Lines changed: 74 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,58 @@ fir::ShapeShiftOp Fortran::lower::omp::getShapeShift(fir::FirOpBuilder &builder,
122122
return shapeShift;
123123
}
124124

125+
// Initialize box newBox using moldBox. These should both have the same type and
126+
// be boxes containing derived types e.g.
127+
// fir.box<!fir.type<>>
128+
// fir.box<!fir.heap<!fir.type<>>
129+
// fir.box<!fir.heap<!fir.array<fir.type<>>>
130+
// fir.class<...<!fir.type<>>>
131+
// If the type doesn't match , this does nothing
132+
static void initializeIfDerivedTypeBox(fir::FirOpBuilder &builder,
133+
mlir::Location loc, mlir::Value newBox,
134+
mlir::Value moldBox, bool hasInitializer,
135+
bool isFirstPrivate) {
136+
fir::BoxType boxTy = mlir::dyn_cast<fir::BoxType>(newBox.getType());
137+
fir::ClassType classTy = mlir::dyn_cast<fir::ClassType>(newBox.getType());
138+
if (!boxTy && !classTy)
139+
return;
140+
141+
// remove pointer and array types in the middle
142+
mlir::Type eleTy;
143+
if (boxTy)
144+
eleTy = boxTy.getElementType();
145+
if (classTy)
146+
eleTy = classTy.getEleTy();
147+
mlir::Type derivedTy = fir::unwrapRefType(eleTy);
148+
if (auto array = mlir::dyn_cast<fir::SequenceType>(derivedTy))
149+
derivedTy = array.getElementType();
150+
151+
if (!fir::isa_derived(derivedTy))
152+
return;
153+
assert(moldBox.getType() == newBox.getType());
154+
155+
if (hasInitializer)
156+
fir::runtime::genDerivedTypeInitialize(builder, loc, newBox);
157+
158+
if (hlfir::mayHaveAllocatableComponent(derivedTy) && !isFirstPrivate)
159+
fir::runtime::genDerivedTypeInitializeClone(builder, loc, newBox, moldBox);
160+
}
161+
162+
static bool
163+
isDerivedTypeNeedingInitialization(const Fortran::semantics::Symbol &sym) {
164+
// Fortran::lower::hasDefaultInitialization returns false for ALLOCATABLE, so
165+
// re-implement here.
166+
// ignorePointer=true because either the pointer points to the same target as
167+
// the original variable, or it is uninitialized.
168+
if (const Fortran::semantics::DeclTypeSpec *declTypeSpec = sym.GetType())
169+
if (const Fortran::semantics::DerivedTypeSpec *derivedTypeSpec =
170+
declTypeSpec->AsDerived())
171+
if (derivedTypeSpec->HasDefaultInitialization(
172+
/*ignoreAllocatable=*/false, /*ignorePointer=*/true))
173+
return true;
174+
return false;
175+
}
176+
125177
static mlir::Value generateZeroShapeForRank(fir::FirOpBuilder &builder,
126178
mlir::Location loc,
127179
mlir::Value moldArg) {
@@ -145,19 +197,18 @@ void Fortran::lower::omp::populateByRefInitAndCleanupRegions(
145197
fir::FirOpBuilder &builder, mlir::Location loc, mlir::Type argType,
146198
mlir::Value scalarInitValue, mlir::Block *initBlock,
147199
mlir::Value allocatedPrivVarArg, mlir::Value moldArg,
148-
mlir::Region &cleanupRegion, bool isPrivate,
200+
mlir::Region &cleanupRegion, DeclOperationKind kind,
149201
const Fortran::semantics::Symbol *sym) {
150202
mlir::Type ty = fir::unwrapRefType(argType);
151203
builder.setInsertionPointToEnd(initBlock);
152204
auto yield = [&](mlir::Value ret) {
153205
builder.create<mlir::omp::YieldOp>(loc, ret);
154206
};
155207

156-
if (isPrivate)
208+
if (isPrivatization(kind))
157209
assert(sym && "Symbol information is needed to privatize derived types");
158210
bool needsInitialization =
159-
sym ? Fortran::lower::hasDefaultInitialization(sym->GetUltimate())
160-
: false;
211+
sym ? isDerivedTypeNeedingInitialization(sym->GetUltimate()) : false;
161212

162213
if (fir::isa_trivial(ty)) {
163214
builder.setInsertionPointToEnd(initBlock);
@@ -210,7 +261,8 @@ void Fortran::lower::omp::populateByRefInitAndCleanupRegions(
210261

211262
// The initial state of a private pointer is undefined so we don't need to
212263
// match the mold argument (OpenMP 5.2 end of page 106).
213-
if (isPrivate && mlir::isa<fir::PointerType>(boxTy.getEleTy())) {
264+
if (isPrivatization(kind) &&
265+
mlir::isa<fir::PointerType>(boxTy.getEleTy())) {
214266
// we need a shape with the right rank so that the embox op is lowered
215267
// to an llvm struct of the right type. This returns nullptr if the types
216268
// aren't right.
@@ -242,7 +294,7 @@ void Fortran::lower::omp::populateByRefInitAndCleanupRegions(
242294
TODO(loc, "Reduction/Privatization of non-allocatable trivial or "
243295
"character typed box");
244296

245-
if ((isDerived || isChar) && (!isPrivate || scalarInitValue))
297+
if ((isDerived || isChar) && (isReduction(kind) || scalarInitValue))
246298
TODO(loc, "Reduction of an unsupported boxed type");
247299

248300
fir::IfOp ifUnallocated{nullptr};
@@ -259,8 +311,9 @@ void Fortran::lower::omp::populateByRefInitAndCleanupRegions(
259311
mlir::Value box = builder.create<fir::EmboxOp>(
260312
loc, ty, valAlloc, /*shape=*/mlir::Value{}, /*slice=*/mlir::Value{},
261313
lenParams);
262-
if (needsInitialization)
263-
fir::runtime::genDerivedTypeInitialize(builder, loc, box);
314+
initializeIfDerivedTypeBox(
315+
builder, loc, box, moldArg, needsInitialization,
316+
/*isFirstPrivate=*/kind == DeclOperationKind::FirstPrivate);
264317
fir::StoreOp lastOp = builder.create<fir::StoreOp>(loc, box, boxAlloca);
265318

266319
createCleanupRegion(builder, loc, argType, cleanupRegion, sym);
@@ -335,8 +388,10 @@ void Fortran::lower::omp::populateByRefInitAndCleanupRegions(
335388

336389
if (scalarInitValue)
337390
builder.create<hlfir::AssignOp>(loc, scalarInitValue, box);
338-
if (needsInitialization)
339-
fir::runtime::genDerivedTypeInitialize(builder, loc, box);
391+
392+
initializeIfDerivedTypeBox(builder, loc, box, moldArg, needsInitialization,
393+
/*isFirstPrivate=*/kind ==
394+
DeclOperationKind::FirstPrivate);
340395

341396
builder.create<fir::StoreOp>(loc, box, boxAlloca);
342397
if (ifUnallocated)
@@ -371,13 +426,15 @@ void Fortran::lower::omp::populateByRefInitAndCleanupRegions(
371426
}
372427

373428
if (fir::isa_derived(ty)) {
374-
if (needsInitialization) {
375-
builder.setInsertionPointToStart(initBlock);
376-
mlir::Type boxedTy = fir::BoxType::get(ty);
377-
mlir::Value box =
378-
builder.create<fir::EmboxOp>(loc, boxedTy, allocatedPrivVarArg);
379-
fir::runtime::genDerivedTypeInitialize(builder, loc, box);
380-
}
429+
builder.setInsertionPointToStart(initBlock);
430+
mlir::Type boxedTy = fir::BoxType::get(ty);
431+
mlir::Value newBox =
432+
builder.create<fir::EmboxOp>(loc, boxedTy, allocatedPrivVarArg);
433+
mlir::Value moldBox = builder.create<fir::EmboxOp>(loc, boxedTy, moldArg);
434+
initializeIfDerivedTypeBox(
435+
builder, loc, newBox, moldBox, needsInitialization,
436+
/*isFirstPrivate=*/kind == DeclOperationKind::FirstPrivate);
437+
381438
if (sym && hasFinalization(*sym))
382439
createCleanupRegion(builder, loc, argType, cleanupRegion, sym);
383440

flang/lib/Lower/OpenMP/PrivateReductionUtils.h

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,15 @@ namespace Fortran {
3535
namespace lower {
3636
namespace omp {
3737

38+
enum class DeclOperationKind { Private, FirstPrivate, Reduction };
39+
inline bool isPrivatization(DeclOperationKind kind) {
40+
return (kind == DeclOperationKind::FirstPrivate) ||
41+
(kind == DeclOperationKind::Private);
42+
}
43+
inline bool isReduction(DeclOperationKind kind) {
44+
return kind == DeclOperationKind::Reduction;
45+
}
46+
3847
/// Generate init and cleanup regions suitable for reduction or privatizer
3948
/// declarations. `scalarInitValue` may be nullptr if there is no default
4049
/// initialization (for privatization). If this is for a privatizer, set
@@ -43,7 +52,7 @@ void populateByRefInitAndCleanupRegions(
4352
fir::FirOpBuilder &builder, mlir::Location loc, mlir::Type argType,
4453
mlir::Value scalarInitValue, mlir::Block *initBlock,
4554
mlir::Value allocatedPrivVarArg, mlir::Value moldArg,
46-
mlir::Region &cleanupRegion, bool isPrivate = false,
55+
mlir::Region &cleanupRegion, DeclOperationKind kind,
4756
const Fortran::semantics::Symbol *sym = nullptr);
4857

4958
/// Generate a fir::ShapeShift op describing the provided boxed array.

flang/lib/Lower/OpenMP/ReductionProcessor.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -442,7 +442,8 @@ static void createReductionAllocAndInitRegions(
442442
populateByRefInitAndCleanupRegions(builder, loc, type, initValue, initBlock,
443443
reductionDecl.getInitializerAllocArg(),
444444
reductionDecl.getInitializerMoldArg(),
445-
reductionDecl.getCleanupRegion());
445+
reductionDecl.getCleanupRegion(),
446+
DeclOperationKind::Reduction);
446447
}
447448

448449
if (fir::isa_trivial(ty)) {

flang/test/Lower/OpenMP/derived-type-allocatable.f90

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,32 +13,42 @@ module m1
1313

1414
contains
1515

16+
!CHECK-LABEL: omp.private {type = private} @_QMm1Ftest_class_allocatable
17+
!CHECK: fir.call @_FortranAInitialize
18+
!CHECK: omp.yield
19+
20+
!CHECK-LABEL: omp.private {type = private} @_QMm1Ftest_allocatable
21+
!CHECK: fir.call @_FortranAInitialize
22+
!CHECK: omp.yield
23+
1624
!CHECK-LABEL: omp.private {type = private} @_QMm1Ftest_pointer
1725
!CHECK-NOT: fir.call @_FortranAInitializeClone
1826
!CHECK: omp.yield
1927

2028
!CHECK-LABEL: omp.private {type = private} @_QMm1Ftest_nested
2129
!CHECK: fir.call @_FortranAInitializeClone
22-
!CHECK-NEXT: omp.yield
30+
!CHECK: omp.yield
2331

2432
!CHECK-LABEL: omp.private {type = private} @_QMm1Ftest_array_of_allocs
2533
!CHECK: fir.call @_FortranAInitializeClone
26-
!CHECK-NEXT: omp.yield
34+
!CHECK: omp.yield
2735
!CHECK: } dealloc {
2836
!CHECK: fir.call @_FortranAAllocatableDeallocate
2937
!CHECK: omp.yield
3038

3139
!CHECK-LABEL: omp.private {type = firstprivate} @_QMm1Ftest_array
40+
!CHECK: fir.call @_FortranAInitialize(
3241
!CHECK-NOT: fir.call @_FortranAInitializeClone
3342
!CHECK: omp.yield
3443

3544
!CHECK-LABEL: omp.private {type = private} @_QMm1Ftest_array
45+
!CHECK: fir.call @_FortranAInitialize(
3646
!CHECK: fir.call @_FortranAInitializeClone
37-
!CHECK-NEXT: omp.yield
47+
!CHECK: omp.yield
3848

3949
!CHECK-LABEL: omp.private {type = private} @_QMm1Ftest_scalar
4050
!CHECK: fir.call @_FortranAInitializeClone
41-
!CHECK-NEXT: omp.yield
51+
!CHECK: omp.yield
4252

4353
subroutine test_scalar()
4454
type(x) :: v
@@ -105,4 +115,24 @@ subroutine test_pointer()
105115
!$omp parallel private(ptr)
106116
!$omp end parallel
107117
end subroutine
118+
119+
subroutine test_allocatable()
120+
type needs_init
121+
integer :: i = 1
122+
end type
123+
type(needs_init), allocatable :: a
124+
125+
!$omp parallel private(a)
126+
!$omp end parallel
127+
end subroutine
128+
129+
subroutine test_class_allocatable()
130+
type needs_init
131+
integer :: i = 1
132+
end type
133+
class(needs_init), allocatable :: a
134+
135+
!$omp parallel private(a)
136+
!$omp end parallel
137+
end subroutine
108138
end module

flang/test/Lower/OpenMP/private-derived-type.f90

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,14 @@ subroutine s4
1515
!$omp end parallel
1616
end subroutine s4
1717

18-
! CHECK: omp.private {type = private} @[[DERIVED_PRIV:.*]] : !fir.ref<!fir.type<{{.*}}y3{x:!fir.box<!fir.heap<i32>>}>> alloc {
19-
! CHECK: %[[VAL_23:.*]] = fir.alloca !fir.type<_QFs4Ty3{x:!fir.box<!fir.heap<i32>>}> {bindc_name = "v", pinned, uniq_name = "_QFs4Ev"}
20-
! CHECK: %[[VAL_25:.*]] = fir.embox %[[VAL_23]] : (!fir.ref<!fir.type<_QFs4Ty3{x:!fir.box<!fir.heap<i32>>}>>) -> !fir.box<!fir.type<_QFs4Ty3{x:!fir.box<!fir.heap<i32>>}>>
18+
! CHECK: omp.private {type = private} @[[DERIVED_PRIV:.*]] : !fir.type<{{.*}}y3{x:!fir.box<!fir.heap<i32>>}> init {
19+
! CHECK: %[[VAL_25:.*]] = fir.embox %[[VAL_23:.*]] : (!fir.ref<!fir.type<_QFs4Ty3{x:!fir.box<!fir.heap<i32>>}>>) -> !fir.box<!fir.type<_QFs4Ty3{x:!fir.box<!fir.heap<i32>>}>>
2120
! CHECK: %[[VAL_26:.*]] = fir.address_of
2221
! CHECK: %[[VAL_27:.*]] = arith.constant 8 : i32
2322
! CHECK: %[[VAL_28:.*]] = fir.convert %[[VAL_25]] : (!fir.box<!fir.type<_QFs4Ty3{x:!fir.box<!fir.heap<i32>>}>>) -> !fir.box<none>
2423
! CHECK: %[[VAL_29:.*]] = fir.convert %[[VAL_26]] : (!fir.ref<!fir.char<1,{{.*}}>>) -> !fir.ref<i8>
2524
! Check we do call FortranAInitialize on the derived type
2625
! CHECK: fir.call @_FortranAInitialize(%[[VAL_28]], %[[VAL_29]], %[[VAL_27]]) fastmath<contract> : (!fir.box<none>, !fir.ref<i8>, i32) -> ()
27-
! CHECK: %[[VAL_24:.*]]:2 = hlfir.declare %[[VAL_23]] {uniq_name = "_QFs4Ev"} : (!fir.ref<!fir.type<_QFs4Ty3{x:!fir.box<!fir.heap<i32>>}>>) -> (!fir.ref<!fir.type<_QFs4Ty3{x:!fir.box<!fir.heap<i32>>}>>, !fir.ref<!fir.type<_QFs4Ty3{x:!fir.box<!fir.heap<i32>>}>>)
2826
! CHECK: }
2927

3028
! CHECK-LABEL: func.func @_QPs4() {

0 commit comments

Comments
 (0)