Skip to content

Commit 8a9e767

Browse files
bhandarkar-pranavbhandarkar-pranav
authored
[Flang][MLIR] - Handle the mapping of subroutine arguments when they are subsequently used inside the region of an omp.target Op (llvm#134967)
This is a fix for llvm#134912 which is a problem with mapping `fir.boxchar<k>` type values to the target i.e an `omp.target` op. There really are two problems. Fixing the first exposed the second. The first problem is that OpenMP lowering of maps in `omp.target` in Flang cannot handle the mapping of a value that doesnt have a defining operation. In other words, a value that is a block argument. This is handled by mapping the value using a `MapInfoOp`. The second problem this fixes is that it adds bounds to `omp.map.info` ops that map `fir.char<k, ?>` types by extracting the length from the corresponding `fir.boxchar`
1 parent 6b111df commit 8a9e767

File tree

4 files changed

+133
-40
lines changed

4 files changed

+133
-40
lines changed

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
#ifndef FORTRAN_OPTIMIZER_BUILDER_DIRECTIVESCOMMON_H_
1818
#define FORTRAN_OPTIMIZER_BUILDER_DIRECTIVESCOMMON_H_
1919

20+
#include "BoxValue.h"
21+
#include "FIRBuilder.h"
2022
#include "flang/Optimizer/Builder/BoxValue.h"
2123
#include "flang/Optimizer/Builder/FIRBuilder.h"
2224
#include "flang/Optimizer/Builder/Todo.h"
@@ -131,6 +133,31 @@ gatherBoundsOrBoundValues(fir::FirOpBuilder &builder, mlir::Location loc,
131133
}
132134
return values;
133135
}
136+
template <typename BoundsOp, typename BoundsType>
137+
mlir::Value
138+
genBoundsOpFromBoxChar(fir::FirOpBuilder &builder, mlir::Location loc,
139+
fir::ExtendedValue dataExv, AddrAndBoundsInfo &info) {
140+
// TODO: Handle info.isPresent.
141+
if (auto boxCharType =
142+
mlir::dyn_cast<fir::BoxCharType>(info.addr.getType())) {
143+
mlir::Type idxTy = builder.getIndexType();
144+
mlir::Type lenType = builder.getCharacterLengthType();
145+
mlir::Type refType = builder.getRefType(boxCharType.getEleTy());
146+
auto unboxed =
147+
builder.create<fir::UnboxCharOp>(loc, refType, lenType, info.addr);
148+
mlir::Value zero = builder.createIntegerConstant(loc, idxTy, 0);
149+
mlir::Value one = builder.createIntegerConstant(loc, idxTy, 1);
150+
mlir::Value extent = unboxed.getResult(1);
151+
mlir::Value stride = one;
152+
mlir::Value ub = builder.create<mlir::arith::SubIOp>(loc, extent, one);
153+
mlir::Type boundTy = builder.getType<mlir::omp::MapBoundsType>();
154+
return builder.create<mlir::omp::MapBoundsOp>(
155+
loc, boundTy, /*lower_bound=*/zero,
156+
/*upper_bound=*/ub, /*extent=*/extent, /*stride=*/stride,
157+
/*stride_in_bytes=*/true, /*start_idx=*/zero);
158+
}
159+
return mlir::Value{};
160+
}
134161

135162
/// Generate the bounds operation from the descriptor information.
136163
template <typename BoundsOp, typename BoundsType>
@@ -269,6 +296,10 @@ genImplicitBoundsOps(fir::FirOpBuilder &builder, AddrAndBoundsInfo &info,
269296
bounds = genBaseBoundsOps<BoundsOp, BoundsType>(builder, loc, dataExv,
270297
dataExvIsAssumedSize);
271298
}
299+
if (characterWithDynamicLen(fir::unwrapRefType(baseOp.getType()))) {
300+
bounds = {genBoundsOpFromBoxChar<BoundsOp, BoundsType>(builder, loc,
301+
dataExv, info)};
302+
}
272303

273304
return bounds;
274305
}

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

Lines changed: 21 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -217,59 +217,36 @@ static void bindEntryBlockArgs(lower::AbstractConverter &converter,
217217
assert(args.isValid() && "invalid args");
218218
fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
219219

220-
auto bindSingleMapLike = [&converter,
221-
&firOpBuilder](const semantics::Symbol &sym,
222-
const mlir::BlockArgument &arg) {
223-
// Clones the `bounds` placing them inside the entry block and returns
224-
// them.
225-
auto cloneBound = [&](mlir::Value bound) {
226-
if (mlir::isMemoryEffectFree(bound.getDefiningOp())) {
227-
mlir::Operation *clonedOp = firOpBuilder.clone(*bound.getDefiningOp());
228-
return clonedOp->getResult(0);
229-
}
230-
TODO(converter.getCurrentLocation(),
231-
"target map-like clause operand unsupported bound type");
232-
};
233-
234-
auto cloneBounds = [cloneBound](llvm::ArrayRef<mlir::Value> bounds) {
235-
llvm::SmallVector<mlir::Value> clonedBounds;
236-
llvm::transform(bounds, std::back_inserter(clonedBounds),
237-
[&](mlir::Value bound) { return cloneBound(bound); });
238-
return clonedBounds;
239-
};
240-
220+
auto bindSingleMapLike = [&converter](const semantics::Symbol &sym,
221+
const mlir::BlockArgument &arg) {
241222
fir::ExtendedValue extVal = converter.getSymbolExtendedValue(sym);
242223
auto refType = mlir::dyn_cast<fir::ReferenceType>(arg.getType());
243224
if (refType && fir::isa_builtin_cptr_type(refType.getElementType())) {
244225
converter.bindSymbol(sym, arg);
245226
} else {
246227
extVal.match(
247228
[&](const fir::BoxValue &v) {
248-
converter.bindSymbol(sym,
249-
fir::BoxValue(arg, cloneBounds(v.getLBounds()),
250-
v.getExplicitParameters(),
251-
v.getExplicitExtents()));
229+
converter.bindSymbol(sym, fir::BoxValue(arg, v.getLBounds(),
230+
v.getExplicitParameters(),
231+
v.getExplicitExtents()));
252232
},
253233
[&](const fir::MutableBoxValue &v) {
254234
converter.bindSymbol(
255-
sym, fir::MutableBoxValue(arg, cloneBounds(v.getLBounds()),
235+
sym, fir::MutableBoxValue(arg, v.getLBounds(),
256236
v.getMutableProperties()));
257237
},
258238
[&](const fir::ArrayBoxValue &v) {
259-
converter.bindSymbol(
260-
sym, fir::ArrayBoxValue(arg, cloneBounds(v.getExtents()),
261-
cloneBounds(v.getLBounds()),
262-
v.getSourceBox()));
239+
converter.bindSymbol(sym, fir::ArrayBoxValue(arg, v.getExtents(),
240+
v.getLBounds(),
241+
v.getSourceBox()));
263242
},
264243
[&](const fir::CharArrayBoxValue &v) {
265-
converter.bindSymbol(
266-
sym, fir::CharArrayBoxValue(arg, cloneBound(v.getLen()),
267-
cloneBounds(v.getExtents()),
268-
cloneBounds(v.getLBounds())));
244+
converter.bindSymbol(sym, fir::CharArrayBoxValue(arg, v.getLen(),
245+
v.getExtents(),
246+
v.getLBounds()));
269247
},
270248
[&](const fir::CharBoxValue &v) {
271-
converter.bindSymbol(
272-
sym, fir::CharBoxValue(arg, cloneBound(v.getLen())));
249+
converter.bindSymbol(sym, fir::CharBoxValue(arg, v.getLen()));
273250
},
274251
[&](const fir::UnboxedValue &v) { converter.bindSymbol(sym, arg); },
275252
[&](const auto &) {
@@ -1487,14 +1464,13 @@ static void genBodyOfTargetOp(
14871464
while (!valuesDefinedAbove.empty()) {
14881465
for (mlir::Value val : valuesDefinedAbove) {
14891466
mlir::Operation *valOp = val.getDefiningOp();
1490-
assert(valOp != nullptr);
14911467

14921468
// NOTE: We skip BoxDimsOp's as the lesser of two evils is to map the
14931469
// indices separately, as the alternative is to eventually map the Box,
14941470
// which comes with a fairly large overhead comparatively. We could be
14951471
// more robust about this and check using a BackwardsSlice to see if we
14961472
// run the risk of mapping a box.
1497-
if (mlir::isMemoryEffectFree(valOp) &&
1473+
if (valOp && mlir::isMemoryEffectFree(valOp) &&
14981474
!mlir::isa<fir::BoxDimsOp>(valOp)) {
14991475
mlir::Operation *clonedOp = valOp->clone();
15001476
entryBlock->push_front(clonedOp);
@@ -1507,7 +1483,13 @@ static void genBodyOfTargetOp(
15071483
valOp->replaceUsesWithIf(clonedOp, replace);
15081484
} else {
15091485
auto savedIP = firOpBuilder.getInsertionPoint();
1510-
firOpBuilder.setInsertionPointAfter(valOp);
1486+
1487+
if (valOp)
1488+
firOpBuilder.setInsertionPointAfter(valOp);
1489+
else
1490+
// This means val is a block argument
1491+
firOpBuilder.setInsertionPoint(targetOp);
1492+
15111493
auto copyVal =
15121494
firOpBuilder.createTemporary(val.getLoc(), val.getType());
15131495
firOpBuilder.createStoreWithConvert(copyVal.getLoc(), val, copyVal);

‎flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp‎

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@
4747
#include <iterator>
4848
#include <numeric>
4949

50+
#define DEBUG_TYPE "omp-map-info-finalization"
51+
#define PDBGS() (llvm::dbgs() << "[" << DEBUG_TYPE << "]: ")
5052
namespace flangomp {
5153
#define GEN_PASS_DEF_MAPINFOFINALIZATIONPASS
5254
#include "flang/Optimizer/OpenMP/Passes.h.inc"
@@ -557,7 +559,38 @@ class MapInfoFinalizationPass
557559
// iterations from previous function scopes.
558560
localBoxAllocas.clear();
559561

560-
// First, walk `omp.map.info` ops to see if any record members should be
562+
// First, walk `omp.map.info` ops to see if any of them have varPtrs
563+
// with an underlying type of fir.char<k, ?>, i.e a character
564+
// with dynamic length. If so, check if they need bounds added.
565+
func->walk([&](mlir::omp::MapInfoOp op) {
566+
if (!op.getBounds().empty())
567+
return;
568+
569+
mlir::Value varPtr = op.getVarPtr();
570+
mlir::Type underlyingVarType = fir::unwrapRefType(varPtr.getType());
571+
572+
if (!fir::characterWithDynamicLen(underlyingVarType))
573+
return;
574+
575+
fir::factory::AddrAndBoundsInfo info =
576+
fir::factory::getDataOperandBaseAddr(
577+
builder, varPtr, /*isOptional=*/false, varPtr.getLoc());
578+
fir::ExtendedValue extendedValue =
579+
hlfir::translateToExtendedValue(varPtr.getLoc(), builder,
580+
hlfir::Entity{info.addr},
581+
/*continguousHint=*/true)
582+
.first;
583+
builder.setInsertionPoint(op);
584+
llvm::SmallVector<mlir::Value> boundsOps =
585+
fir::factory::genImplicitBoundsOps<mlir::omp::MapBoundsOp,
586+
mlir::omp::MapBoundsType>(
587+
builder, info, extendedValue,
588+
/*dataExvIsAssumedSize=*/false, varPtr.getLoc());
589+
590+
op.getBoundsMutable().append(boundsOps);
591+
});
592+
593+
// Next, walk `omp.map.info` ops to see if any record members should be
561594
// implicitly mapped.
562595
func->walk([&](mlir::omp::MapInfoOp op) {
563596
mlir::Type underlyingType =

‎flang/test/Lower/OpenMP/map-character.f90‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
! RUN: %flang_fc1 -emit-hlfir -fopenmp -o - %s 2>&1 | FileCheck %s
2+
3+
subroutine TestOfCharacter(a0, a1, l)
4+
character(len=*), intent(in) :: a0
5+
character(len=*), intent(inout):: a1
6+
integer, intent(in) :: l
7+
8+
!$omp target map(to:a0) map(from: a1)
9+
a1 = a0
10+
!$omp end target
11+
end subroutine TestOfCharacter
12+
13+
14+
!CHECK: func.func @_QPtestofcharacter(%[[ARG0:.*]]: !fir.boxchar<1> {{.*}}, %[[ARG1:.*]]: !fir.boxchar<1> {{.*}}
15+
!CHECK: %[[A0_BOXCHAR_ALLOCA:.*]] = fir.alloca !fir.boxchar<1>
16+
!CHECK: %[[A1_BOXCHAR_ALLOCA:.*]] = fir.alloca !fir.boxchar<1>
17+
!CHECK: %[[UNBOXED_ARG0:.*]]:2 = fir.unboxchar %[[ARG0]] : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index)
18+
!CHECK: %[[A0_DECL:.*]]:2 = hlfir.declare %[[UNBOXED_ARG0]]#0 typeparams %[[UNBOXED_ARG0]]#1 dummy_scope {{.*}} -> (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>)
19+
!CHECK: %[[UNBOXED_ARG1:.*]]:2 = fir.unboxchar %[[ARG1]] : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index)
20+
!CHECK: %[[A1_DECL:.*]]:2 = hlfir.declare %[[UNBOXED_ARG1]]#0 typeparams %[[UNBOXED_ARG1]]#1 dummy_scope {{.*}} -> (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>)
21+
!CHECK: %[[UNBOXED_A0_DECL:.*]]:2 = fir.unboxchar %[[A0_DECL]]#0 : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index)
22+
!CHECK: %[[A0_LB:.*]] = arith.constant 0 : index
23+
!CHECK: %[[A0_STRIDE:.*]] = arith.constant 1 : index
24+
!CHECK: %[[A0_UB:.*]] = arith.subi %[[UNBOXED_A0_DECL]]#1, %[[A0_STRIDE]] : index
25+
!CHECK: %[[A0_BOUNDS:.*]] = omp.map.bounds lower_bound(%[[A0_LB]] : index) upper_bound(%[[A0_UB]] : index) extent(%[[UNBOXED_A0_DECL]]#1 : index)
26+
!CHECK-SAME: stride(%[[A0_STRIDE]] : index) start_idx(%[[A0_LB]] : index) {stride_in_bytes = true}
27+
!CHECK: %[[A0_MAP:.*]] = omp.map.info var_ptr(%[[A0_DECL]]#1 : !fir.ref<!fir.char<1,?>>, !fir.char<1,?>) map_clauses(to) capture(ByRef) bounds(%[[A0_BOUNDS]]) -> !fir.ref<!fir.char<1,?>> {name = "a0"}
28+
!CHECK: %[[UNBOXED_A1_DECL:.*]]:2 = fir.unboxchar %[[A1_DECL]]#0 : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index)
29+
!CHECK: %[[A1_LB:.*]] = arith.constant 0 : index
30+
!CHECK: %[[A1_STRIDE:.*]] = arith.constant 1 : index
31+
!CHECK: %[[A1_UB:.*]] = arith.subi %[[UNBOXED_A1_DECL]]#1, %[[A1_STRIDE]] : index
32+
!CHECK: %[[A1_BOUNDS:.*]] = omp.map.bounds lower_bound(%[[A1_LB]] : index) upper_bound(%[[A1_UB]] : index) extent(%[[UNBOXED_A1_DECL]]#1 : index)
33+
!CHECKL-SAME: stride(%[[A1_STRIDE]] : index) start_idx(%[[A1_LB]] : index) {stride_in_bytes = true}
34+
!CHECK: %[[A1_MAP:.*]] = omp.map.info var_ptr(%[[A1_DECL]]#1 : !fir.ref<!fir.char<1,?>>, !fir.char<1,?>) map_clauses(from) capture(ByRef) bounds(%[[A1_BOUNDS]]) -> !fir.ref<!fir.char<1,?>> {name = "a1"}
35+
!CHECK: fir.store %[[ARG1]] to %[[A1_BOXCHAR_ALLOCA]] : !fir.ref<!fir.boxchar<1>>
36+
!CHECK: %[[A1_BOXCHAR_MAP:.*]] = omp.map.info var_ptr(%[[A1_BOXCHAR_ALLOCA]] : !fir.ref<!fir.boxchar<1>>, !fir.boxchar<1>) map_clauses(implicit, to) capture(ByRef) -> !fir.ref<!fir.boxchar<1>> {name = ""}
37+
!CHECK: fir.store %[[ARG0]] to %[[A0_BOXCHAR_ALLOCA]] : !fir.ref<!fir.boxchar<1>>
38+
!CHECK: %[[A0_BOXCHAR_MAP:.*]] = omp.map.info var_ptr(%[[A0_BOXCHAR_ALLOCA]] : !fir.ref<!fir.boxchar<1>>, !fir.boxchar<1>) map_clauses(implicit, to) capture(ByRef) -> !fir.ref<!fir.boxchar<1>> {name = ""}
39+
40+
!CHECK: omp.target map_entries(%[[A0_MAP]] -> %[[TGT_A0:.*]], %[[A1_MAP]] -> %[[TGT_A1:.*]], %[[A1_BOXCHAR_MAP]] -> %[[TGT_A1_BOXCHAR:.*]], %[[A0_BOXCHAR_MAP]] -> %[[TGT_A0_BOXCHAR:.*]] : !fir.ref<!fir.char<1,?>>, !fir.ref<!fir.char<1,?>>, !fir.ref<!fir.boxchar<1>>, !fir.ref<!fir.boxchar<1>>) {
41+
!CHECK: %[[TGT_A0_BC_LD:.*]] = fir.load %[[TGT_A0_BOXCHAR]] : !fir.ref<!fir.boxchar<1>>
42+
!CHECK: %[[TGT_A1_BC_LD:.*]] = fir.load %[[TGT_A1_BOXCHAR]] : !fir.ref<!fir.boxchar<1>>
43+
!CHECK: %[[UNBOXED_TGT_A1:.*]]:2 = fir.unboxchar %[[TGT_A1_BC_LD]] : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index)
44+
!CHECK: %[[UNBOXED_TGT_A0:.*]]:2 = fir.unboxchar %[[TGT_A0_BC_LD]] : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index)
45+
!CHECK: %[[TGT_A0_DECL:.*]]:2 = hlfir.declare %[[TGT_A0]] typeparams %[[UNBOXED_TGT_A0]]#1 {{.*}} -> (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>)
46+
!CHECK: %[[TGT_A1_DECL:.*]]:2 = hlfir.declare %[[TGT_A1]] typeparams %[[UNBOXED_TGT_A1]]#1 {{.*}} -> (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>)
47+

0 commit comments

Comments
 (0)