Skip to content

Commit 747588d

Browse files
tblahtblah
authored
[flang][Lower][OpenMP] Don't read moldarg for static sized array (#127838)
This should further reduce the number of spurious barriers
1 parent d595fc9 commit 747588d

File tree

5 files changed

+57
-33
lines changed

5 files changed

+57
-33
lines changed

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -508,6 +508,8 @@ void DataSharingProcessor::doPrivatize(const semantics::Symbol *sym,
508508

509509
lower::SymbolBox hsb = converter.lookupOneLevelUpSymbol(*sym);
510510
assert(hsb && "Host symbol box not found");
511+
hlfir::Entity entity{hsb.getAddr()};
512+
bool cannotHaveNonDefaultLowerBounds = !entity.mayHaveNonDefaultLowerBounds();
511513

512514
mlir::Location symLoc = hsb.getAddr().getLoc();
513515
std::string privatizerName = sym->name().ToString() + ".privatizer";
@@ -528,7 +530,6 @@ void DataSharingProcessor::doPrivatize(const semantics::Symbol *sym,
528530
// an alloca for a fir.array type there. Get around this by boxing all
529531
// arrays.
530532
if (mlir::isa<fir::SequenceType>(allocType)) {
531-
hlfir::Entity entity{hsb.getAddr()};
532533
entity = genVariableBox(symLoc, firOpBuilder, entity);
533534
privVal = entity.getBase();
534535
allocType = privVal.getType();
@@ -590,7 +591,7 @@ void DataSharingProcessor::doPrivatize(const semantics::Symbol *sym,
590591
result.getDeallocRegion(),
591592
isFirstPrivate ? DeclOperationKind::FirstPrivate
592593
: DeclOperationKind::Private,
593-
sym);
594+
sym, cannotHaveNonDefaultLowerBounds);
594595
// TODO: currently there are false positives from dead uses of the mold
595596
// arg
596597
if (!result.getInitMoldArg().getUses().empty())

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

Lines changed: 41 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -122,25 +122,40 @@ static void createCleanupRegion(Fortran::lower::AbstractConverter &converter,
122122
typeError();
123123
}
124124

125-
fir::ShapeShiftOp Fortran::lower::omp::getShapeShift(fir::FirOpBuilder &builder,
126-
mlir::Location loc,
127-
mlir::Value box) {
125+
fir::ShapeShiftOp
126+
Fortran::lower::omp::getShapeShift(fir::FirOpBuilder &builder,
127+
mlir::Location loc, mlir::Value box,
128+
bool cannotHaveNonDefaultLowerBounds) {
128129
fir::SequenceType sequenceType = mlir::cast<fir::SequenceType>(
129130
hlfir::getFortranElementOrSequenceType(box.getType()));
130131
const unsigned rank = sequenceType.getDimension();
132+
131133
llvm::SmallVector<mlir::Value> lbAndExtents;
132134
lbAndExtents.reserve(rank * 2);
133-
134135
mlir::Type idxTy = builder.getIndexType();
135-
for (unsigned i = 0; i < rank; ++i) {
136-
// TODO: ideally we want to hoist box reads out of the critical section.
137-
// We could do this by having box dimensions in block arguments like
138-
// OpenACC does
139-
mlir::Value dim = builder.createIntegerConstant(loc, idxTy, i);
140-
auto dimInfo =
141-
builder.create<fir::BoxDimsOp>(loc, idxTy, idxTy, idxTy, box, dim);
142-
lbAndExtents.push_back(dimInfo.getLowerBound());
143-
lbAndExtents.push_back(dimInfo.getExtent());
136+
137+
if (cannotHaveNonDefaultLowerBounds && !sequenceType.hasDynamicExtents()) {
138+
// We don't need fir::BoxDimsOp if all of the extents are statically known
139+
// and we can assume default lower bounds. This helps avoids reads from the
140+
// mold arg.
141+
mlir::Value one = builder.createIntegerConstant(loc, idxTy, 1);
142+
for (int64_t extent : sequenceType.getShape()) {
143+
assert(extent != sequenceType.getUnknownExtent());
144+
mlir::Value extentVal = builder.createIntegerConstant(loc, idxTy, extent);
145+
lbAndExtents.push_back(one);
146+
lbAndExtents.push_back(extentVal);
147+
}
148+
} else {
149+
for (unsigned i = 0; i < rank; ++i) {
150+
// TODO: ideally we want to hoist box reads out of the critical section.
151+
// We could do this by having box dimensions in block arguments like
152+
// OpenACC does
153+
mlir::Value dim = builder.createIntegerConstant(loc, idxTy, i);
154+
auto dimInfo =
155+
builder.create<fir::BoxDimsOp>(loc, idxTy, idxTy, idxTy, box, dim);
156+
lbAndExtents.push_back(dimInfo.getLowerBound());
157+
lbAndExtents.push_back(dimInfo.getExtent());
158+
}
144159
}
145160

146161
auto shapeShiftTy = fir::ShapeShiftType::get(builder.getContext(), rank);
@@ -248,12 +263,13 @@ class PopulateInitAndCleanupRegionsHelper {
248263
mlir::Type argType, mlir::Value scalarInitValue,
249264
mlir::Value allocatedPrivVarArg, mlir::Value moldArg,
250265
mlir::Block *initBlock, mlir::Region &cleanupRegion,
251-
DeclOperationKind kind, const Fortran::semantics::Symbol *sym)
266+
DeclOperationKind kind, const Fortran::semantics::Symbol *sym,
267+
bool cannotHaveLowerBounds)
252268
: converter{converter}, builder{converter.getFirOpBuilder()}, loc{loc},
253269
argType{argType}, scalarInitValue{scalarInitValue},
254270
allocatedPrivVarArg{allocatedPrivVarArg}, moldArg{moldArg},
255271
initBlock{initBlock}, cleanupRegion{cleanupRegion}, kind{kind},
256-
sym{sym} {
272+
sym{sym}, cannotHaveNonDefaultLowerBounds{cannotHaveLowerBounds} {
257273
valType = fir::unwrapRefType(argType);
258274
}
259275

@@ -295,6 +311,10 @@ class PopulateInitAndCleanupRegionsHelper {
295311
/// Any length parameters which have been fetched for the type
296312
mlir::SmallVector<mlir::Value> lenParams;
297313

314+
/// If the source variable being privatized definitely can't have non-default
315+
/// lower bounds then we don't need to generate code to read them.
316+
bool cannotHaveNonDefaultLowerBounds;
317+
298318
void createYield(mlir::Value ret) {
299319
builder.create<mlir::omp::YieldOp>(loc, ret);
300320
}
@@ -432,7 +452,8 @@ void PopulateInitAndCleanupRegionsHelper::initAndCleanupBoxedArray(
432452
// Special case for (possibly allocatable) arrays of polymorphic types
433453
// e.g. !fir.class<!fir.heap<!fir.array<?x!fir.type<>>>>
434454
if (source.isPolymorphic()) {
435-
fir::ShapeShiftOp shape = getShapeShift(builder, loc, source);
455+
fir::ShapeShiftOp shape =
456+
getShapeShift(builder, loc, source, cannotHaveNonDefaultLowerBounds);
436457
mlir::Type arrayType = source.getElementOrSequenceType();
437458
mlir::Value allocatedArray = builder.create<fir::AllocMemOp>(
438459
loc, arrayType, /*typeparams=*/mlir::ValueRange{}, shape.getExtents());
@@ -471,8 +492,8 @@ void PopulateInitAndCleanupRegionsHelper::initAndCleanupBoxedArray(
471492
// Put the temporary inside of a box:
472493
// hlfir::genVariableBox doesn't handle non-default lower bounds
473494
mlir::Value box;
474-
fir::ShapeShiftOp shapeShift =
475-
getShapeShift(builder, loc, getLoadedMoldArg());
495+
fir::ShapeShiftOp shapeShift = getShapeShift(builder, loc, getLoadedMoldArg(),
496+
cannotHaveNonDefaultLowerBounds);
476497
mlir::Type boxType = getLoadedMoldArg().getType();
477498
if (mlir::isa<fir::BaseBoxType>(temp.getType()))
478499
// the box created by the declare form createTempFromMold is missing
@@ -607,10 +628,10 @@ void Fortran::lower::omp::populateByRefInitAndCleanupRegions(
607628
mlir::Type argType, mlir::Value scalarInitValue, mlir::Block *initBlock,
608629
mlir::Value allocatedPrivVarArg, mlir::Value moldArg,
609630
mlir::Region &cleanupRegion, DeclOperationKind kind,
610-
const Fortran::semantics::Symbol *sym) {
631+
const Fortran::semantics::Symbol *sym, bool cannotHaveLowerBounds) {
611632
PopulateInitAndCleanupRegionsHelper helper(
612633
converter, loc, argType, scalarInitValue, allocatedPrivVarArg, moldArg,
613-
initBlock, cleanupRegion, kind, sym);
634+
initBlock, cleanupRegion, kind, sym, cannotHaveLowerBounds);
614635
helper.populateByRefInitAndCleanupRegions();
615636

616637
// Often we load moldArg to check something (e.g. length parameters, shape)

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,11 +55,13 @@ void populateByRefInitAndCleanupRegions(
5555
mlir::Value scalarInitValue, mlir::Block *initBlock,
5656
mlir::Value allocatedPrivVarArg, mlir::Value moldArg,
5757
mlir::Region &cleanupRegion, DeclOperationKind kind,
58-
const Fortran::semantics::Symbol *sym = nullptr);
58+
const Fortran::semantics::Symbol *sym = nullptr,
59+
bool cannotHaveNonDefaultLowerBounds = false);
5960

6061
/// Generate a fir::ShapeShift op describing the provided boxed array.
6162
fir::ShapeShiftOp getShapeShift(fir::FirOpBuilder &builder, mlir::Location loc,
62-
mlir::Value box);
63+
mlir::Value box,
64+
bool cannotHaveNonDefaultLowerBounds = false);
6365

6466
} // namespace omp
6567
} // namespace lower

‎flang/test/Lower/OpenMP/delayed-privatization-array.f90

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -108,15 +108,14 @@ program main
108108
! ONE_DIM_DEFAULT_LB-SAME: @[[PRIVATIZER_SYM:.*]] : [[BOX_TYPE:!fir.box<!fir.array<10xi32>>]] init {
109109

110110
! ONE_DIM_DEFAULT_LB-NEXT: ^bb0(%[[PRIV_ARG:.*]]: [[TYPE:!fir.ref<!fir.box<!fir.array<10xi32>>>]], %[[PRIV_BOX_ALLOC:.*]]: [[TYPE]]):
111-
! ONE_DIM_DEFAULT_LB-NEXT: %[[PRIV_ARG_VAL:.*]] = fir.load %[[PRIV_ARG]]
112111
! ONE_DIM_DEFAULT_LB-NEXT: %[[C10:.*]] = arith.constant 10 : index
113112
! ONE_DIM_DEFAULT_LB-NEXT: %[[SHAPE:.*]] = fir.shape %[[C10]]
114113
! ONE_DIM_DEFAULT_LB-NEXT: %[[ARRAY_ALLOC:.*]] = fir.allocmem !fir.array<10xi32>
115114
! ONE_DIM_DEFAULT_LB-NEXT: %[[TRUE:.*]] = arith.constant true
116115
! ONE_DIM_DEFAULT_LB-NEXT: %[[DECL:.*]]:2 = hlfir.declare %[[ARRAY_ALLOC]](%[[SHAPE]])
117-
! ONE_DIM_DEFAULT_LB-NEXT: %[[C0_0:.*]] = arith.constant 0
118-
! ONE_DIM_DEFAULT_LB-NEXT: %[[DIMS2:.*]]:3 = fir.box_dims %[[PRIV_ARG_VAL]], %[[C0_0]]
119-
! ONE_DIM_DEFAULT_LB-NEXT: %[[SHAPE_SHIFT:.*]] = fir.shape_shift %[[DIMS2]]#0, %[[DIMS2]]#1
116+
! ONE_DIM_DEFAULT_LB-NEXT: %[[ONE:.*]] = arith.constant 1 : index
117+
! ONE_DIM_DEFAULT_LB-NEXT: %[[TEN:.*]] = arith.constant 10 : index
118+
! ONE_DIM_DEFAULT_LB-NEXT: %[[SHAPE_SHIFT:.*]] = fir.shape_shift %[[ONE]], %[[TEN]]
120119
! ONE_DIM_DEFAULT_LB-NEXT: %[[EMBOX:.*]] = fir.embox %[[DECL]]#0(%[[SHAPE_SHIFT]])
121120
! ONE_DIM_DEFAULT_LB-NEXT: fir.store %[[EMBOX]] to %[[PRIV_BOX_ALLOC]]
122121
! ONE_DIM_DEFAULT_LB-NEXT: omp.yield(%[[PRIV_BOX_ALLOC]] : [[TYPE]])

‎flang/test/Lower/OpenMP/different_vars_lastprivate_barrier.f90

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,29 @@
11
! RUN: %flang_fc1 -fopenmp -mmlir --openmp-enable-delayed-privatization-staging=true -emit-hlfir %s -o - | FileCheck %s
22

3-
subroutine first_and_lastprivate
3+
subroutine first_and_lastprivate(var)
44
integer i
5-
integer, dimension(3) :: var
5+
integer, dimension(:) :: var
66

77
!$omp parallel do lastprivate(i) private(var)
88
do i=1,1
99
end do
1010
!$omp end parallel do
1111
end subroutine
1212

13-
! CHECK: omp.private {type = private} @[[VAR_PRIVATIZER:.*Evar_private_box_3xi32]] : [[BOX_TYPE:!fir\.box<!fir\.array<3xi32>>]] init {
13+
! CHECK: omp.private {type = private} @[[VAR_PRIVATIZER:.*Evar_private_box_Uxi32]] : [[BOX_TYPE:!fir\.box<!fir\.array<\?xi32>>]] init {
1414
! CHECK-NEXT: ^bb0(%[[ORIG_REF:.*]]: {{.*}}, %[[PRIV_REF:.*]]: {{.*}}):
1515
! CHECK: %[[ORIG_VAL:.*]] = fir.load %[[ORIG_REF]]
16+
! CHECK: %[[BOX_DIMS_0:.*]]:3 = fir.box_dims %[[ORIG_VAL]], %{{.*}} : ([[BOX_TYPE]], index) -> (index, index, index)
1617
! CHECK: %[[BOX_DIMS:.*]]:3 = fir.box_dims %[[ORIG_VAL]], %{{.*}} : ([[BOX_TYPE]], index) -> (index, index, index)
1718
! CHECK: %[[SHAPE_SHIFT:.*]] = fir.shape_shift %[[BOX_DIMS]]#0, %[[BOX_DIMS]]#1
18-
! CHECK: %[[EMBOX:.*]] = fir.embox %{{.*}}(%[[SHAPE_SHIFT]]) : {{.*}} -> [[BOX_TYPE]]
19+
! CHECK: %[[EMBOX:.*]] = fir.rebox %{{.*}}(%[[SHAPE_SHIFT]]) : {{.*}} -> [[BOX_TYPE]]
1920
! CHECK: fir.store %[[EMBOX]] to %[[PRIV_REF]]
2021
! CHECK: omp.yield(%[[PRIV_REF]] : !fir.ref<[[BOX_TYPE]]>)
2122
! CHECK: }
2223

2324
! CHECK: omp.private {type = private} @[[I_PRIVATIZER:.*Ei_private_i32]] : i32
2425

25-
! CHECK: func.func @{{.*}}first_and_lastprivate()
26+
! CHECK: func.func @{{.*}}first_and_lastprivate({{.*}})
2627
! CHECK: %[[ORIG_I_DECL:.*]]:2 = hlfir.declare {{.*}} {uniq_name = "{{.*}}Ei"}
2728
! CHECK: omp.parallel {
2829
! CHECK-NOT: omp.barrier

0 commit comments

Comments
 (0)