Skip to content

Commit bd30838

Browse files
[flang][acc] Improve acc lowering around fir.box and arrays (#125600)
The current implementation of OpenACC lowering includes explicit expansion of following cases: - Creation of `acc.bounds` operations for all arrays, including those whose dimensions are captured in the type (eg `!fir.array<100xf32>`) - Expansion of box types by only putting the box's address in the data clause. The address was extracted with a `fir.box_addr` operation and the bounds were filled with `fir.box_dims` operation. However, with the creation of the new type interface `MappableType`, the idea is that specific type-based semantics can now be used. This also really simplifies representation in the IR. Consider the following example: ``` subroutine sub(arr) real :: arr(:) !$acc enter data copyin(arr) end subroutine ``` Before the current PR, the relevant acc dialect IR looked like: ``` func.func @_QPsub(%arg0: !fir.box<!fir.array<?xf32>> {fir.bindc_name = "arr"}) { ... %1:2 = hlfir.declare %arg0 dummy_scope %0 {uniq_name = "_QFsubEarr"} : (!fir.box<!fir.array<?xf32>>, !fir.dscope) -> (!fir.box<!fir.array<?xf32>>, !fir.box<!fir.array<?xf32>>) %c1 = arith.constant 1 : index %c0 = arith.constant 0 : index %2:3 = fir.box_dims %1#0, %c0 : (!fir.box<!fir.array<?xf32>>, index) -> (index, index, index) %c0_0 = arith.constant 0 : index %3 = arith.subi %2#1, %c1 : index %4 = acc.bounds lowerbound(%c0_0 : index) upperbound(%3 : index) extent(%2#1 : index) stride(%2#2 : index) startIdx(%c1 : index) {strideInBytes = true} %5 = fir.box_addr %1#0 : (!fir.box<!fir.array<?xf32>>) -> !fir.ref<!fir.array<?xf32>> %6 = acc.copyin varPtr(%5 : !fir.ref<!fir.array<?xf32>>) bounds(%4) -> !fir.ref<!fir.array<?xf32>> {name = "arr", structured = false} acc.enter_data dataOperands(%6 : !fir.ref<!fir.array<?xf32>>) ``` After the current change, it looks like: ``` func.func @_QPsub(%arg0: !fir.box<!fir.array<?xf32>> {fir.bindc_name = "arr"}) { ... %1:2 = hlfir.declare %arg0 dummy_scope %0 {uniq_name = "_QFsubEarr"} : (!fir.box<!fir.array<?xf32>>, !fir.dscope) -> (!fir.box<!fir.array<?xf32>>, !fir.box<!fir.array<?xf32>>) %2 = acc.copyin var(%1#0 : !fir.box<!fir.array<?xf32>>) -> !fir.box<!fir.array<?xf32>> {name = "arr", structured = false} acc.enter_data dataOperands(%2 : !fir.box<!fir.array<?xf32>>) ``` Restoring the old behavior can be done with following command line options: `--openacc-unwrap-fir-box=true --openacc-generate-default-bounds=true`
1 parent 389d135 commit bd30838

30 files changed

+4400
-1086
lines changed

flang/include/flang/Lower/DirectivesCommon.h

Lines changed: 30 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -584,10 +584,11 @@ void createEmptyRegionBlocks(
584584
inline fir::factory::AddrAndBoundsInfo
585585
getDataOperandBaseAddr(Fortran::lower::AbstractConverter &converter,
586586
fir::FirOpBuilder &builder,
587-
Fortran::lower::SymbolRef sym, mlir::Location loc) {
587+
Fortran::lower::SymbolRef sym, mlir::Location loc,
588+
bool unwrapFirBox = true) {
588589
return fir::factory::getDataOperandBaseAddr(
589590
builder, converter.getSymbolAddress(sym),
590-
Fortran::semantics::IsOptional(sym), loc);
591+
Fortran::semantics::IsOptional(sym), loc, unwrapFirBox);
591592
}
592593

593594
namespace detail {
@@ -880,13 +881,15 @@ fir::factory::AddrAndBoundsInfo gatherDataOperandAddrAndBounds(
880881
Fortran::semantics::SymbolRef symbol,
881882
const Fortran::semantics::MaybeExpr &maybeDesignator,
882883
mlir::Location operandLocation, std::stringstream &asFortran,
883-
llvm::SmallVector<mlir::Value> &bounds, bool treatIndexAsSection = false) {
884+
llvm::SmallVector<mlir::Value> &bounds, bool treatIndexAsSection = false,
885+
bool unwrapFirBox = true, bool genDefaultBounds = true) {
884886
using namespace Fortran;
885887

886888
fir::factory::AddrAndBoundsInfo info;
887889

888890
if (!maybeDesignator) {
889-
info = getDataOperandBaseAddr(converter, builder, symbol, operandLocation);
891+
info = getDataOperandBaseAddr(converter, builder, symbol, operandLocation,
892+
unwrapFirBox);
890893
asFortran << symbol->name().ToString();
891894
return info;
892895
}
@@ -930,7 +933,8 @@ fir::factory::AddrAndBoundsInfo gatherDataOperandAddrAndBounds(
930933
const semantics::Symbol &sym = arrayRef->GetLastSymbol();
931934
dataExvIsAssumedSize =
932935
Fortran::semantics::IsAssumedSizeArray(sym.GetUltimate());
933-
info = getDataOperandBaseAddr(converter, builder, sym, operandLocation);
936+
info = getDataOperandBaseAddr(converter, builder, sym, operandLocation,
937+
unwrapFirBox);
934938
dataExv = converter.getSymbolExtendedValue(sym);
935939
asFortran << sym.name().ToString();
936940
}
@@ -947,7 +951,8 @@ fir::factory::AddrAndBoundsInfo gatherDataOperandAddrAndBounds(
947951
converter.genExprAddr(operandLocation, designator, stmtCtx);
948952
info.addr = fir::getBase(compExv);
949953
info.rawInput = info.addr;
950-
if (mlir::isa<fir::SequenceType>(fir::unwrapRefType(info.addr.getType())))
954+
if (genDefaultBounds &&
955+
mlir::isa<fir::SequenceType>(fir::unwrapRefType(info.addr.getType())))
951956
bounds = fir::factory::genBaseBoundsOps<BoundsOp, BoundsType>(
952957
builder, operandLocation, compExv,
953958
/*isAssumedSize=*/false);
@@ -958,14 +963,17 @@ fir::factory::AddrAndBoundsInfo gatherDataOperandAddrAndBounds(
958963
operandLocation, builder.getI1Type(), info.rawInput);
959964
}
960965

961-
if (auto loadOp =
962-
mlir::dyn_cast_or_null<fir::LoadOp>(info.addr.getDefiningOp())) {
963-
if (fir::isAllocatableType(loadOp.getType()) ||
964-
fir::isPointerType(loadOp.getType())) {
965-
info.boxType = info.addr.getType();
966-
info.addr = builder.create<fir::BoxAddrOp>(operandLocation, info.addr);
966+
if (unwrapFirBox) {
967+
if (auto loadOp =
968+
mlir::dyn_cast_or_null<fir::LoadOp>(info.addr.getDefiningOp())) {
969+
if (fir::isAllocatableType(loadOp.getType()) ||
970+
fir::isPointerType(loadOp.getType())) {
971+
info.boxType = info.addr.getType();
972+
info.addr =
973+
builder.create<fir::BoxAddrOp>(operandLocation, info.addr);
974+
}
975+
info.rawInput = info.addr;
967976
}
968-
info.rawInput = info.addr;
969977
}
970978

971979
// If the component is an allocatable or pointer the result of
@@ -977,8 +985,9 @@ fir::factory::AddrAndBoundsInfo gatherDataOperandAddrAndBounds(
977985
info.addr = boxAddrOp.getVal();
978986
info.boxType = info.addr.getType();
979987
info.rawInput = info.addr;
980-
bounds = fir::factory::genBoundsOpsFromBox<BoundsOp, BoundsType>(
981-
builder, operandLocation, compExv, info);
988+
if (genDefaultBounds)
989+
bounds = fir::factory::genBoundsOpsFromBox<BoundsOp, BoundsType>(
990+
builder, operandLocation, compExv, info);
982991
}
983992
} else {
984993
if (detail::getRef<evaluate::ArrayRef>(designator)) {
@@ -990,17 +999,18 @@ fir::factory::AddrAndBoundsInfo gatherDataOperandAddrAndBounds(
990999
} else if (auto symRef = detail::getRef<semantics::SymbolRef>(designator)) {
9911000
// Scalar or full array.
9921001
fir::ExtendedValue dataExv = converter.getSymbolExtendedValue(*symRef);
993-
info =
994-
getDataOperandBaseAddr(converter, builder, *symRef, operandLocation);
995-
if (mlir::isa<fir::BaseBoxType>(
996-
fir::unwrapRefType(info.addr.getType()))) {
1002+
info = getDataOperandBaseAddr(converter, builder, *symRef,
1003+
operandLocation, unwrapFirBox);
1004+
if (genDefaultBounds && mlir::isa<fir::BaseBoxType>(
1005+
fir::unwrapRefType(info.addr.getType()))) {
9971006
info.boxType = fir::unwrapRefType(info.addr.getType());
9981007
bounds = fir::factory::genBoundsOpsFromBox<BoundsOp, BoundsType>(
9991008
builder, operandLocation, dataExv, info);
10001009
}
10011010
bool dataExvIsAssumedSize =
10021011
Fortran::semantics::IsAssumedSizeArray(symRef->get().GetUltimate());
1003-
if (mlir::isa<fir::SequenceType>(fir::unwrapRefType(info.addr.getType())))
1012+
if (genDefaultBounds &&
1013+
mlir::isa<fir::SequenceType>(fir::unwrapRefType(info.addr.getType())))
10041014
bounds = fir::factory::genBaseBoundsOps<BoundsOp, BoundsType>(
10051015
builder, operandLocation, dataExv, dataExvIsAssumedSize);
10061016
asFortran << symRef->get().name().ToString();

flang/include/flang/Optimizer/Builder/DirectivesCommon.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,8 @@ struct AddrAndBoundsInfo {
5454
inline AddrAndBoundsInfo getDataOperandBaseAddr(fir::FirOpBuilder &builder,
5555
mlir::Value symAddr,
5656
bool isOptional,
57-
mlir::Location loc) {
57+
mlir::Location loc,
58+
bool unwrapFirBox = true) {
5859
mlir::Value rawInput = symAddr;
5960
if (auto declareOp =
6061
mlir::dyn_cast_or_null<hlfir::DeclareOp>(symAddr.getDefiningOp())) {
@@ -80,7 +81,8 @@ inline AddrAndBoundsInfo getDataOperandBaseAddr(fir::FirOpBuilder &builder,
8081
// all address/dimension retrievals. For Fortran optional though, leave
8182
// the load generation for later so it can be done in the appropriate
8283
// if branches.
83-
if (mlir::isa<fir::ReferenceType>(symAddr.getType()) && !isOptional) {
84+
if (unwrapFirBox && mlir::isa<fir::ReferenceType>(symAddr.getType()) &&
85+
!isOptional) {
8486
mlir::Value addr = builder.create<fir::LoadOp>(loc, symAddr);
8587
return AddrAndBoundsInfo(addr, rawInput, isPresent, boxTy);
8688
}

0 commit comments

Comments
 (0)