Skip to content

Commit 34fcdd0

Browse files
author
Razvan Lupusoru
committed
[flang][acc] Use non-boxed value for assumed-size in acc data clause
Assumed-size arrays end up looking as follows at the HLFIR level: ``` func.func @_QPsub(%arg0: !fir.ref<!fir.array<?xf64>> {fir.bindc_name = "arr"}) { ... %1 = fir.shape %c-1 : (index) -> !fir.shape<1> %2:2 = hlfir.declare %arg0(%1) dummy_scope %0 {uniq_name = "_QFsubEarr"} : (!fir.ref<!fir.array<?xf64>>, !fir.shape<1>, !fir.dscope) -> (!fir.box<!fir.array<?xf64>>, !fir.ref<!fir.array<?xf64>>) ``` The declare operation produces an entity with Fortran properties (wrapped via a box) or the raw data pointer. The current acc lowering uses the box value. During ConvertHLFIRtoFIR, this leads to a forced materialization of descriptor even though the descriptor itself does not hold useful extent information (it holds -1). Other operations such as those that index the array access the raw pointer directly and thus do not force materialization of descriptor. Since there is nothing useful in descriptor and since at end of the day the acc dialect can accept a pointer to the data, make it consistent. This is useful property because without any acc data clauses, an assumed-size array becomes live-in via raw pointer typically. Thus, the materialization of descriptor is not something acc lowering forces.
1 parent e697c99 commit 34fcdd0

File tree

2 files changed

+51
-9
lines changed

2 files changed

+51
-9
lines changed

flang/lib/Lower/OpenACC.cpp

Lines changed: 50 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -369,6 +369,55 @@ getSymbolFromAccObject(const Fortran::parser::AccObject &accObject) {
369369
llvm::report_fatal_error("Could not find symbol");
370370
}
371371

372+
static mlir::Value getBaseAddr(Fortran::semantics::Symbol &symbol,
373+
const fir::factory::AddrAndBoundsInfo &info) {
374+
if (Fortran::semantics::IsAssumedSizeArray(symbol)) {
375+
// Assumed-size arrays in FIR are represented as:
376+
// func.func @func(%arg0: !fir.ref<!fir.array<?xf64>> {fir.bindc_name = "arr"}) {
377+
// %arr:2 = hlfir.declare %arg0(%shape) ... -> (!fir.box<!fir.array<?xf64>>, !fir.ref<!fir.array<?xf64>>)
378+
// The `rawInput` refers to the #1 output of the `hlfir.declare` operation.
379+
// This is preferred since the Fortran variable properties does not contain
380+
// any useful size information.
381+
return info.rawInput;
382+
}
383+
384+
if (Fortran::semantics::IsOptional(symbol)) {
385+
// When there is an optional argument for which there is a possibility
386+
// to create a descriptor, pick the rawInput instead. This is done to
387+
// avoid materializing the descriptor which leads to following pattern
388+
// generated at the FIR level which adds an extra indirection that makes
389+
// recovering original variable not evident.
390+
// This is the pattern we want to avoid to be generated:
391+
// %1 = fir.declare %arg0 ... {fortran_attrs = #fir.var_attrs<optional>, uniq_name = "_QFsub1Eassumedshapeoptarr"} : (!fir.box<!fir.array<?xf32>>, !fir.dscope) -> !fir.box<!fir.array<?xf32>>
392+
// %2 = fir.is_present %1 : (!fir.box<!fir.array<?xf32>>) -> i1
393+
// %3 = fir.if %2 -> (!fir.box<!fir.array<?xf32>>) {
394+
// %5 = fir.rebox %1 : (!fir.box<!fir.array<?xf32>>) -> !fir.box<!fir.array<?xf32>>
395+
// fir.result %5 : !fir.box<!fir.array<?xf32>>
396+
// } else {
397+
// %5 = fir.absent !fir.box<!fir.array<?xf32>>
398+
// fir.result %5 : !fir.box<!fir.array<?xf32>>
399+
// }
400+
// %4 = acc.copyin var(%3 : !fir.box<!fir.array<?xf32>>) ...
401+
//
402+
// Instead by picking the rawInput we get the following pattern:
403+
// %1 = fir.declare %arg0 ... {fortran_attrs = #fir.var_attrs<optional>, uniq_name = "_QFsub1Eassumedshapeoptarr"} : (!fir.box<!fir.array<?xf32>>, !fir.dscope) -> !fir.box<!fir.array<?xf32>>
404+
// %2 = acc.copyin var(%2 : !fir.box<!fir.array<?xf32>>) ...
405+
if (fir::unwrapRefType(info.addr.getType()) !=
406+
fir::unwrapRefType(info.rawInput.getType())) {
407+
return info.rawInput;
408+
}
409+
}
410+
411+
// The `addr` field refers to the address of the Fortran entity, but with the
412+
// ssa value that when lowered to FIR will include the tied Fortran variable
413+
// properties. Additionally, in cases where `unwrapFirBox` is requested,
414+
// it refers to the address of the data (either result of fir.box_addr or
415+
// result of `fir.if` in case of optional).
416+
// Therefore, use the processed address in all cases by default unless it was
417+
// deemed through the earlier checks in this routine that it is not useful.
418+
return info.addr;
419+
}
420+
372421
template <typename Op>
373422
static void
374423
genDataOperandOperations(const Fortran::parser::AccObjectList &objectList,
@@ -399,13 +448,7 @@ genDataOperandOperations(const Fortran::parser::AccObjectList &objectList,
399448
/*genDefaultBounds=*/generateDefaultBounds);
400449
LLVM_DEBUG(llvm::dbgs() << __func__ << "\n"; info.dump(llvm::dbgs()));
401450

402-
// If the input value is optional and is not a descriptor, we use the
403-
// rawInput directly.
404-
mlir::Value baseAddr = ((fir::unwrapRefType(info.addr.getType()) !=
405-
fir::unwrapRefType(info.rawInput.getType())) &&
406-
info.isPresent)
407-
? info.rawInput
408-
: info.addr;
451+
mlir::Value baseAddr = getBaseAddr(symbol, info);
409452
Op op = createDataEntryOp<Op>(
410453
builder, operandLocation, baseAddr, asFortran, bounds, structured,
411454
implicit, dataClause, baseAddr.getType(), async, asyncDeviceTypes,

flang/test/Lower/OpenACC/acc-bounds.f90

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,7 @@ subroutine acc_undefined_extent(a)
9292
! CHECK: %[[DIMS0:.*]]:3 = fir.box_dims %[[DECL_ARG0]]#0, %c0{{.*}} : (!fir.box<!fir.array<?xf32>>, index) -> (index, index, index)
9393
! CHECK: %[[UB:.*]] = arith.subi %[[DIMS0]]#1, %c1{{.*}} : index
9494
! CHECK: %[[BOUND:.*]] = acc.bounds lowerbound(%c0{{.*}} : index) upperbound(%[[UB]] : index) extent(%[[DIMS0]]#1 : index) stride(%[[DIMS0]]#2 : index) startIdx(%c1{{.*}} : index) {strideInBytes = true}
95-
! CHECK: %[[ADDR:.*]] = fir.box_addr %[[DECL_ARG0]]#0 : (!fir.box<!fir.array<?xf32>>) -> !fir.ref<!fir.array<?xf32>>
96-
! CHECK: %[[PRESENT:.*]] = acc.present varPtr(%[[ADDR]] : !fir.ref<!fir.array<?xf32>>) bounds(%[[BOUND]]) -> !fir.ref<!fir.array<?xf32>> {name = "a"}
95+
! CHECK: %[[PRESENT:.*]] = acc.present varPtr(%[[DECL_ARG0]]#1 : !fir.ref<!fir.array<?xf32>>) bounds(%[[BOUND]]) -> !fir.ref<!fir.array<?xf32>> {name = "a"}
9796
! CHECK: acc.kernels dataOperands(%[[PRESENT]] : !fir.ref<!fir.array<?xf32>>)
9897

9998
subroutine acc_multi_strides(a)

0 commit comments

Comments
 (0)