Skip to content

Commit e748dc5

Browse files
committed
[flang][OpenMP][HLFIR] Support vector subscripted array sections for DEPEND
The OpenMP runtime needs the base address of the array section to identify the dependency. If we just put the vector subscript through the usual HLFIR expression lowering, that would generate a new contiguous array representing the values of the elements in the array which was sectioned. We cannot use addresses from this array because these addresses would not match dependencies on the original array. For example ``` integer :: array(1024) integer :: indices(2) indices(1) = 1 indices(2) = 100 !$omp task depend(out: array(1:512)) !$omp end task !$omp task depend(in: array(indices)) !$omp end task ``` This requires taking the lowering path previously only used for ordered assignments to get the address of the elements in the original array which were indexed. This is done using `hlfir.elemental_addr`. e.g. ``` array(indices) = 2 ``` `hlfir.elemental_addr` is awkward to use because it (by design) doesn't return something like `!hlfir.expr<>` (like `hlfir.elemental`) and so it can't have a generic lowering: each place it is used has to carefully inline the contents of the operation and extract the needed address. For this reason, `hlfir.elemental_addr` was not previously allowed outside of these ordered assignments. In this commit I relax that restriction so that I can use `hlfir.elemental_addr` to lower the OpenMP DEPEND clause. The disadvantage of doing so is that there is no generic lowering supporting uses of `hlfir.elemental_addr` in arbitrary contexts. I think it is okay to have this gap in the dialect verifier because HLFIR is not a dialect shared with users outside of flang. One alternative solution would have been to provide my own more limited re-implementation of `HlfirDesignatorBuilder` which skipped `hlfir::elemental_addr`, instead inlining its body directly at the current insertion point applying indices only for the first element. This would have been difficult to maintain because designation in Fortran is complex.
1 parent 4c09ae0 commit e748dc5

File tree

4 files changed

+86
-24
lines changed

4 files changed

+86
-24
lines changed

flang/include/flang/Optimizer/HLFIR/HLFIROps.td

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1419,28 +1419,26 @@ def hlfir_YieldOp : hlfir_Op<"yield", [Terminator, ParentOneOf<["RegionAssignOp"
14191419
let assemblyFormat = "$entity attr-dict `:` type($entity) custom<YieldOpCleanup>($cleanup)";
14201420
}
14211421

1422-
def hlfir_ElementalAddrOp : hlfir_Op<"elemental_addr", [Terminator, HasParent<"RegionAssignOp">,
1422+
def hlfir_ElementalAddrOp : hlfir_Op<"elemental_addr", [Terminator,
14231423
RecursiveMemoryEffects, RecursivelySpeculatable, hlfir_ElementalOpInterface,
14241424
AttrSizedOperandSegments]> {
14251425
let summary = "Yield the address of a vector subscripted variable inside an hlfir.region_assign";
14261426
let description = [{
1427-
Special terminator node for the left-hand side region of an hlfir.region_assign
1428-
to a vector subscripted entity.
1427+
Used to get the address of elements in an array which is being subscripted
1428+
by a vector.
14291429

14301430
It represents how the address of an element of such entity is computed given
14311431
one based indices.
14321432

14331433
It is very similar to hlfir.elemental, except that it does not produce an SSA
14341434
value because there is no hlfir type to describe a vector subscripted entity
1435-
(the codegen of such type would be problematic). Hence, it is tightly linked
1436-
to an hlfir.region_assign by its terminator property.
1435+
(the codegen of such type would be problematic).
14371436

14381437
An optional cleanup region may be provided if any of the subscript expressions
14391438
of the designator require a cleanup.
14401439
This allows documenting cleanups that cannot be generated after the vector
14411440
subscripted designator usage (that has not been materizaled yet). The cleanups
1442-
will be evaluated after the assignment once the related
1443-
hlfir.region_assign is lowered.
1441+
should be evaluated after the usage of the elemental addr.
14441442

14451443
Example: "X(VECTOR) = Y"
14461444

flang/lib/Lower/OpenMP/ClauseProcessor.cpp

Lines changed: 48 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -889,13 +889,55 @@ bool ClauseProcessor::processDepend(lower::SymMap &symMap,
889889
} else if (evaluate::IsArrayElement(*object.ref())) {
890890
// Array Section
891891
SomeExpr expr = *object.ref();
892-
if (isVectorSubscript(expr))
893-
TODO(converter.getCurrentLocation(),
894-
"Vector subscripted array section for task dependency");
895892

896-
hlfir::EntityWithAttributes entity = convertExprToHLFIR(
897-
converter.getCurrentLocation(), converter, expr, symMap, stmtCtx);
898-
dependVar = entity.getBase();
893+
if (isVectorSubscript(expr)) {
894+
// OpenMP needs the address of the first indexed element (required by
895+
// the standard to be the lowest index) to identify the dependency. We
896+
// don't need an accurate length for the array section because the
897+
// OpenMP standard forbids overlapping array sections.
898+
899+
// Get a hlfir.elemental_addr op describing the address of the value
900+
// indexed from the original array.
901+
hlfir::ElementalAddrOp addrOp =
902+
convertVectorSubscriptedExprToElementalAddr(
903+
converter.getCurrentLocation(), converter, expr, symMap,
904+
stmtCtx);
905+
if (!addrOp.getCleanup().empty())
906+
TODO(converter.getCurrentLocation(),
907+
"Vector subscript in DEPEND clause requring a cleanup region");
908+
909+
// hlfir.elemental_addr doesn't have a normal lowering because it
910+
// can't return a value. Instead we need to inline it here using
911+
// values for the first element. Similar to hlfir::inlineElementalOp.
912+
913+
mlir::Value one = builder.createIntegerConstant(
914+
converter.getCurrentLocation(), builder.getIndexType(), 1);
915+
mlir::SmallVector<mlir::Value> oneBasedIndices;
916+
oneBasedIndices.resize(addrOp.getIndices().size(), one);
917+
918+
mlir::IRMapping mapper;
919+
mapper.map(addrOp.getIndices(), oneBasedIndices);
920+
assert(addrOp.getElementalRegion().hasOneBlock());
921+
mlir::Operation *newOp;
922+
for (mlir::Operation &op :
923+
addrOp.getElementalRegion().back().getOperations())
924+
newOp = builder.clone(op, mapper);
925+
auto yield = mlir::cast<hlfir::YieldOp>(newOp);
926+
927+
addrOp->erase();
928+
929+
if (!yield.getCleanup().empty())
930+
TODO(converter.getCurrentLocation(),
931+
"Vector subscript in DEPEND clause requring element cleanup");
932+
933+
dependVar = yield.getEntity();
934+
yield->erase();
935+
} else {
936+
// Ordinary array section e.g. A(1:512:2)
937+
hlfir::EntityWithAttributes entity = convertExprToHLFIR(
938+
converter.getCurrentLocation(), converter, expr, symMap, stmtCtx);
939+
dependVar = entity.getBase();
940+
}
899941
} else {
900942
semantics::Symbol *sym = object.sym();
901943
dependVar = converter.getSymbolAddress(*sym);

flang/test/Lower/OpenMP/Todo/depend-clause-vector-subscript-array-section.f90

Lines changed: 0 additions & 11 deletions
This file was deleted.

flang/test/Lower/OpenMP/task-depend-array-section.f90

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,3 +49,36 @@ subroutine assumedShape(array)
4949
! CHECK: }
5050
! CHECK: return
5151
! CHECK: }
52+
53+
subroutine vectorSubscriptArraySection(array, indices)
54+
integer :: array(:)
55+
integer :: indices(:)
56+
57+
!$omp task depend (in: array(indices))
58+
!$omp end task
59+
end subroutine
60+
! CHECK-LABEL: func.func @_QPvectorsubscriptarraysection(
61+
! CHECK-SAME: %[[VAL_0:[0-9]+|[a-zA-Z$._-][a-zA-Z0-9$._-]*]]: !fir.box<!fir.array<?xi32>> {fir.bindc_name = "array"},
62+
! CHECK-SAME: %[[VAL_1:[0-9]+|[a-zA-Z$._-][a-zA-Z0-9$._-]*]]: !fir.box<!fir.array<?xi32>> {fir.bindc_name = "indices"}) {
63+
! CHECK: %[[VAL_2:.*]] = fir.dummy_scope : !fir.dscope
64+
! CHECK: %[[VAL_3:.*]]:2 = hlfir.declare %[[VAL_0]] dummy_scope %[[VAL_2]] {uniq_name = "_QFvectorsubscriptarraysectionEarray"} : (!fir.box<!fir.array<?xi32>>, !fir.dscope) -> (!fir.box<!fir.array<?xi32>>, !fir.box<!fir.array<?xi32>>)
65+
! CHECK: %[[VAL_4:.*]]:2 = hlfir.declare %[[VAL_1]] dummy_scope %[[VAL_2]] {uniq_name = "_QFvectorsubscriptarraysectionEindices"} : (!fir.box<!fir.array<?xi32>>, !fir.dscope) -> (!fir.box<!fir.array<?xi32>>, !fir.box<!fir.array<?xi32>>)
66+
! CHECK: %[[VAL_5:.*]] = arith.constant 0 : index
67+
! CHECK: %[[VAL_6:.*]]:3 = fir.box_dims %[[VAL_4]]#0, %[[VAL_5]] : (!fir.box<!fir.array<?xi32>>, index) -> (index, index, index)
68+
! CHECK: %[[VAL_7:.*]] = fir.shape %[[VAL_6]]#1 : (index) -> !fir.shape<1>
69+
! CHECK: %[[VAL_8:.*]] = hlfir.elemental %[[VAL_7]] unordered : (!fir.shape<1>) -> !hlfir.expr<?xi64> {
70+
! CHECK: ^bb0(%[[VAL_9:.*]]: index):
71+
! CHECK: %[[VAL_10:.*]] = hlfir.designate %[[VAL_4]]#0 (%[[VAL_9]]) : (!fir.box<!fir.array<?xi32>>, index) -> !fir.ref<i32>
72+
! CHECK: %[[VAL_11:.*]] = fir.load %[[VAL_10]] : !fir.ref<i32>
73+
! CHECK: %[[VAL_12:.*]] = fir.convert %[[VAL_11]] : (i32) -> i64
74+
! CHECK: hlfir.yield_element %[[VAL_12]] : i64
75+
! CHECK: }
76+
! CHECK: %[[VAL_13:.*]] = arith.constant 1 : index
77+
! CHECK: %[[VAL_14:.*]] = hlfir.apply %[[VAL_8]], %[[VAL_13]] : (!hlfir.expr<?xi64>, index) -> i64
78+
! CHECK: %[[VAL_15:.*]] = hlfir.designate %[[VAL_3]]#0 (%[[VAL_14]]) : (!fir.box<!fir.array<?xi32>>, i64) -> !fir.ref<i32>
79+
! CHECK: omp.task depend(taskdependin -> %[[VAL_15]] : !fir.ref<i32>) {
80+
! CHECK: omp.terminator
81+
! CHECK: }
82+
! CHECK: hlfir.destroy %[[VAL_8]] : !hlfir.expr<?xi64>
83+
! CHECK: return
84+
! CHECK: }

0 commit comments

Comments
 (0)