caio/clang-p2996
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[Flang][MLIR] - Handle the mapping of subroutine arguments when they are subsequently used inside the region of an omp.target Op (#134967)

Browse Source 
This is a fix for https://github.com/llvm/llvm-project/issues/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`
This commit is contained in:
Pranav Bhandarkar
2025-05-12 22:34:58 -05:00
committed by GitHub
parent 6b111dfdb5
commit 8a9e767fa6
4 changed files with 133 additions and 40 deletions
Download Patch File Download Diff File Expand all files Collapse all files

31
flang/include/flang/Optimizer/Builder/DirectivesCommon.h
View File

@@ -17,6 +17,8 @@
#ifndef FORTRAN_OPTIMIZER_BUILDER_DIRECTIVESCOMMON_H_
#define FORTRAN_OPTIMIZER_BUILDER_DIRECTIVESCOMMON_H_
#include "BoxValue.h"
#include "FIRBuilder.h"
#include "flang/Optimizer/Builder/BoxValue.h"
#include "flang/Optimizer/Builder/FIRBuilder.h"
#include "flang/Optimizer/Builder/Todo.h"
@@ -131,6 +133,31 @@ gatherBoundsOrBoundValues(fir::FirOpBuilder &builder, mlir::Location loc,
}
return values;
}
template <typename BoundsOp, typename BoundsType>
mlir::Value
genBoundsOpFromBoxChar(fir::FirOpBuilder &builder, mlir::Location loc,
fir::ExtendedValue dataExv, AddrAndBoundsInfo &info) {
// TODO: Handle info.isPresent.
if (auto boxCharType =
mlir::dyn_cast<fir::BoxCharType>(info.addr.getType())) {
mlir::Type idxTy = builder.getIndexType();
mlir::Type lenType = builder.getCharacterLengthType();
mlir::Type refType = builder.getRefType(boxCharType.getEleTy());
auto unboxed =
builder.create<fir::UnboxCharOp>(loc, refType, lenType, info.addr);
mlir::Value zero = builder.createIntegerConstant(loc, idxTy, 0);
mlir::Value one = builder.createIntegerConstant(loc, idxTy, 1);
mlir::Value extent = unboxed.getResult(1);
mlir::Value stride = one;
mlir::Value ub = builder.create<mlir::arith::SubIOp>(loc, extent, one);
mlir::Type boundTy = builder.getType<mlir::omp::MapBoundsType>();
return builder.create<mlir::omp::MapBoundsOp>(
loc, boundTy, /*lower_bound=*/zero,
/*upper_bound=*/ub, /*extent=*/extent, /*stride=*/stride,
/*stride_in_bytes=*/true, /*start_idx=*/zero);
}
return mlir::Value{};
}
/// Generate the bounds operation from the descriptor information.
template <typename BoundsOp, typename BoundsType>
@@ -269,6 +296,10 @@ genImplicitBoundsOps(fir::FirOpBuilder &builder, AddrAndBoundsInfo &info,
bounds = genBaseBoundsOps<BoundsOp, BoundsType>(builder, loc, dataExv,
dataExvIsAssumedSize);
}
if (characterWithDynamicLen(fir::unwrapRefType(baseOp.getType()))) {
bounds = {genBoundsOpFromBoxChar<BoundsOp, BoundsType>(builder, loc,
dataExv, info)};
}
return bounds;
}

60
flang/lib/Lower/OpenMP/OpenMP.cpp
View File

@@ -217,27 +217,8 @@ static void bindEntryBlockArgs(lower::AbstractConverter &converter,
assert(args.isValid() && "invalid args");
fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
auto bindSingleMapLike = [&converter,
&firOpBuilder](const semantics::Symbol &sym,
const mlir::BlockArgument &arg) {
// Clones the `bounds` placing them inside the entry block and returns
// them.
auto cloneBound = [&](mlir::Value bound) {
if (mlir::isMemoryEffectFree(bound.getDefiningOp())) {
mlir::Operation *clonedOp = firOpBuilder.clone(*bound.getDefiningOp());
return clonedOp->getResult(0);
}
TODO(converter.getCurrentLocation(),
"target map-like clause operand unsupported bound type");
};
auto cloneBounds = [cloneBound](llvm::ArrayRef<mlir::Value> bounds) {
llvm::SmallVector<mlir::Value> clonedBounds;
llvm::transform(bounds, std::back_inserter(clonedBounds),
[&](mlir::Value bound) { return cloneBound(bound); });
return clonedBounds;
};
auto bindSingleMapLike = [&converter](const semantics::Symbol &sym,
const mlir::BlockArgument &arg) {
fir::ExtendedValue extVal = converter.getSymbolExtendedValue(sym);
auto refType = mlir::dyn_cast<fir::ReferenceType>(arg.getType());
if (refType && fir::isa_builtin_cptr_type(refType.getElementType())) {
@@ -245,31 +226,27 @@ static void bindEntryBlockArgs(lower::AbstractConverter &converter,
} else {
extVal.match(
[&](const fir::BoxValue &v) {
converter.bindSymbol(sym,
fir::BoxValue(arg, cloneBounds(v.getLBounds()),
v.getExplicitParameters(),
v.getExplicitExtents()));
converter.bindSymbol(sym, fir::BoxValue(arg, v.getLBounds(),
v.getExplicitParameters(),
v.getExplicitExtents()));
},
[&](const fir::MutableBoxValue &v) {
converter.bindSymbol(
sym, fir::MutableBoxValue(arg, cloneBounds(v.getLBounds()),
sym, fir::MutableBoxValue(arg, v.getLBounds(),
v.getMutableProperties()));
},
[&](const fir::ArrayBoxValue &v) {
converter.bindSymbol(
sym, fir::ArrayBoxValue(arg, cloneBounds(v.getExtents()),
cloneBounds(v.getLBounds()),
v.getSourceBox()));
converter.bindSymbol(sym, fir::ArrayBoxValue(arg, v.getExtents(),
v.getLBounds(),
v.getSourceBox()));
},
[&](const fir::CharArrayBoxValue &v) {
converter.bindSymbol(
sym, fir::CharArrayBoxValue(arg, cloneBound(v.getLen()),
cloneBounds(v.getExtents()),
cloneBounds(v.getLBounds())));
converter.bindSymbol(sym, fir::CharArrayBoxValue(arg, v.getLen(),
v.getExtents(),
v.getLBounds()));
},
[&](const fir::CharBoxValue &v) {
converter.bindSymbol(
sym, fir::CharBoxValue(arg, cloneBound(v.getLen())));
converter.bindSymbol(sym, fir::CharBoxValue(arg, v.getLen()));
},
[&](const fir::UnboxedValue &v) { converter.bindSymbol(sym, arg); },
[&](const auto &) {
@@ -1487,14 +1464,13 @@ static void genBodyOfTargetOp(
while (!valuesDefinedAbove.empty()) {
for (mlir::Value val : valuesDefinedAbove) {
mlir::Operation *valOp = val.getDefiningOp();
assert(valOp != nullptr);
// NOTE: We skip BoxDimsOp's as the lesser of two evils is to map the
// indices separately, as the alternative is to eventually map the Box,
// which comes with a fairly large overhead comparatively. We could be
// more robust about this and check using a BackwardsSlice to see if we
// run the risk of mapping a box.
if (mlir::isMemoryEffectFree(valOp) &&
if (valOp && mlir::isMemoryEffectFree(valOp) &&
!mlir::isa<fir::BoxDimsOp>(valOp)) {
mlir::Operation *clonedOp = valOp->clone();
entryBlock->push_front(clonedOp);
@@ -1507,7 +1483,13 @@ static void genBodyOfTargetOp(
valOp->replaceUsesWithIf(clonedOp, replace);
} else {
auto savedIP = firOpBuilder.getInsertionPoint();
firOpBuilder.setInsertionPointAfter(valOp);
if (valOp)
firOpBuilder.setInsertionPointAfter(valOp);
else
// This means val is a block argument
firOpBuilder.setInsertionPoint(targetOp);
auto copyVal =
firOpBuilder.createTemporary(val.getLoc(), val.getType());
firOpBuilder.createStoreWithConvert(copyVal.getLoc(), val, copyVal);

35
flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp
View File

@@ -47,6 +47,8 @@
#include <iterator>
#include <numeric>
#define DEBUG_TYPE "omp-map-info-finalization"
#define PDBGS() (llvm::dbgs() << "[" << DEBUG_TYPE << "]: ")
namespace flangomp {
#define GEN_PASS_DEF_MAPINFOFINALIZATIONPASS
#include "flang/Optimizer/OpenMP/Passes.h.inc"
@@ -557,7 +559,38 @@ class MapInfoFinalizationPass
// iterations from previous function scopes.
localBoxAllocas.clear();
// First, walk `omp.map.info` ops to see if any record members should be
// First, walk `omp.map.info` ops to see if any of them have varPtrs
// with an underlying type of fir.char<k, ?>, i.e a character
// with dynamic length. If so, check if they need bounds added.
func->walk([&](mlir::omp::MapInfoOp op) {
if (!op.getBounds().empty())
return;
mlir::Value varPtr = op.getVarPtr();
mlir::Type underlyingVarType = fir::unwrapRefType(varPtr.getType());
if (!fir::characterWithDynamicLen(underlyingVarType))
return;
fir::factory::AddrAndBoundsInfo info =
fir::factory::getDataOperandBaseAddr(
builder, varPtr, /*isOptional=*/false, varPtr.getLoc());
fir::ExtendedValue extendedValue =
hlfir::translateToExtendedValue(varPtr.getLoc(), builder,
hlfir::Entity{info.addr},
/*continguousHint=*/true)
.first;
builder.setInsertionPoint(op);
llvm::SmallVector<mlir::Value> boundsOps =
fir::factory::genImplicitBoundsOps<mlir::omp::MapBoundsOp,
mlir::omp::MapBoundsType>(
builder, info, extendedValue,
/*dataExvIsAssumedSize=*/false, varPtr.getLoc());
op.getBoundsMutable().append(boundsOps);
});
// Next, walk `omp.map.info` ops to see if any record members should be
// implicitly mapped.
func->walk([&](mlir::omp::MapInfoOp op) {
mlir::Type underlyingType =

47
flang/test/Lower/OpenMP/map-character.f90 Normal file
View File

@@ -0,0 +1,47 @@
! RUN: %flang_fc1 -emit-hlfir -fopenmp -o - %s 2>&1 | FileCheck %s
subroutine TestOfCharacter(a0, a1, l)
character(len=*), intent(in) :: a0
character(len=*), intent(inout):: a1
integer, intent(in) :: l
!$omp target map(to:a0) map(from: a1)
a1 = a0
!$omp end target
end subroutine TestOfCharacter
!CHECK: func.func @_QPtestofcharacter(%[[ARG0:.*]]: !fir.boxchar<1> {{.*}}, %[[ARG1:.*]]: !fir.boxchar<1> {{.*}}
!CHECK: %[[A0_BOXCHAR_ALLOCA:.*]] = fir.alloca !fir.boxchar<1>
!CHECK: %[[A1_BOXCHAR_ALLOCA:.*]] = fir.alloca !fir.boxchar<1>
!CHECK: %[[UNBOXED_ARG0:.*]]:2 = fir.unboxchar %[[ARG0]] : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index)
!CHECK: %[[A0_DECL:.*]]:2 = hlfir.declare %[[UNBOXED_ARG0]]#0 typeparams %[[UNBOXED_ARG0]]#1 dummy_scope {{.*}} -> (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>)
!CHECK: %[[UNBOXED_ARG1:.*]]:2 = fir.unboxchar %[[ARG1]] : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index)
!CHECK: %[[A1_DECL:.*]]:2 = hlfir.declare %[[UNBOXED_ARG1]]#0 typeparams %[[UNBOXED_ARG1]]#1 dummy_scope {{.*}} -> (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>)
!CHECK: %[[UNBOXED_A0_DECL:.*]]:2 = fir.unboxchar %[[A0_DECL]]#0 : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index)
!CHECK: %[[A0_LB:.*]] = arith.constant 0 : index
!CHECK: %[[A0_STRIDE:.*]] = arith.constant 1 : index
!CHECK: %[[A0_UB:.*]] = arith.subi %[[UNBOXED_A0_DECL]]#1, %[[A0_STRIDE]] : index
!CHECK: %[[A0_BOUNDS:.*]] = omp.map.bounds lower_bound(%[[A0_LB]] : index) upper_bound(%[[A0_UB]] : index) extent(%[[UNBOXED_A0_DECL]]#1 : index)
!CHECK-SAME: stride(%[[A0_STRIDE]] : index) start_idx(%[[A0_LB]] : index) {stride_in_bytes = true}
!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"}
!CHECK: %[[UNBOXED_A1_DECL:.*]]:2 = fir.unboxchar %[[A1_DECL]]#0 : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index)
!CHECK: %[[A1_LB:.*]] = arith.constant 0 : index
!CHECK: %[[A1_STRIDE:.*]] = arith.constant 1 : index
!CHECK: %[[A1_UB:.*]] = arith.subi %[[UNBOXED_A1_DECL]]#1, %[[A1_STRIDE]] : index
!CHECK: %[[A1_BOUNDS:.*]] = omp.map.bounds lower_bound(%[[A1_LB]] : index) upper_bound(%[[A1_UB]] : index) extent(%[[UNBOXED_A1_DECL]]#1 : index)
!CHECKL-SAME: stride(%[[A1_STRIDE]] : index) start_idx(%[[A1_LB]] : index) {stride_in_bytes = true}
!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"}
!CHECK: fir.store %[[ARG1]] to %[[A1_BOXCHAR_ALLOCA]] : !fir.ref<!fir.boxchar<1>>
!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 = ""}
!CHECK: fir.store %[[ARG0]] to %[[A0_BOXCHAR_ALLOCA]] : !fir.ref<!fir.boxchar<1>>
!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 = ""}
!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>>) {
!CHECK: %[[TGT_A0_BC_LD:.*]] = fir.load %[[TGT_A0_BOXCHAR]] : !fir.ref<!fir.boxchar<1>>
!CHECK: %[[TGT_A1_BC_LD:.*]] = fir.load %[[TGT_A1_BOXCHAR]] : !fir.ref<!fir.boxchar<1>>
!CHECK: %[[UNBOXED_TGT_A1:.*]]:2 = fir.unboxchar %[[TGT_A1_BC_LD]] : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index)
!CHECK: %[[UNBOXED_TGT_A0:.*]]:2 = fir.unboxchar %[[TGT_A0_BC_LD]] : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index)
!CHECK: %[[TGT_A0_DECL:.*]]:2 = hlfir.declare %[[TGT_A0]] typeparams %[[UNBOXED_TGT_A0]]#1 {{.*}} -> (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>)
!CHECK: %[[TGT_A1_DECL:.*]]:2 = hlfir.declare %[[TGT_A1]] typeparams %[[UNBOXED_TGT_A1]]#1 {{.*}} -> (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>)