[flang-commits] [flang] [Flang][MLIR] - Access the LEN for a `fir.boxchar<k>` and use it to set the bounds `omp.map.info` ops. (PR #134967)
Pranav Bhandarkar via flang-commits
flang-commits at lists.llvm.org
Fri May 2 15:30:23 PDT 2025
https://github.com/bhandarkar-pranav updated https://github.com/llvm/llvm-project/pull/134967
>From 2b2e70f5bf635d573d54465c2d2af0150e5509be Mon Sep 17 00:00:00 2001
From: Pranav Bhandarkar <pranav.bhandarkar at amd.com>
Date: Tue, 8 Apr 2025 15:31:24 -0500
Subject: [PATCH 01/10] end-to-end test8 working. Need to get rid of lots of
debugging commits
---
flang/include/flang/Lower/DirectivesCommon.h | 25 +++-
flang/lib/Lower/OpenMP/ClauseProcessor.cpp | 39 +++++-
flang/lib/Lower/OpenMP/OpenMP.cpp | 114 ++++++++++++++++--
.../Optimizer/OpenMP/MapInfoFinalization.cpp | 46 ++++++-
4 files changed, 208 insertions(+), 16 deletions(-)
diff --git a/flang/include/flang/Lower/DirectivesCommon.h b/flang/include/flang/Lower/DirectivesCommon.h
index 93ab2e350d035..ce03b0751b56a 100644
--- a/flang/include/flang/Lower/DirectivesCommon.h
+++ b/flang/include/flang/Lower/DirectivesCommon.h
@@ -424,24 +424,30 @@ fir::factory::AddrAndBoundsInfo gatherDataOperandAddrAndBounds(
auto arrayBase = toMaybeExpr(arrayRef->base());
assert(arrayBase);
-
+ llvm::errs() << "arrayBase = ";
+ arrayBase.value().dump();
if (detail::getRef<evaluate::Component>(*arrayBase)) {
+ llvm::errs() << "detail::getRef<evaluate::Component>(*arrayBase)\n";
dataExv = converter.genExprAddr(operandLocation, *arrayBase, stmtCtx);
info.addr = fir::getBase(dataExv);
info.rawInput = info.addr;
asFortran << arrayBase->AsFortran();
} else {
+ llvm::errs() << "ELSE -> detail::getRef<evaluate::Component>(*arrayBase)\n";
const semantics::Symbol &sym = arrayRef->GetLastSymbol();
dataExvIsAssumedSize =
Fortran::semantics::IsAssumedSizeArray(sym.GetUltimate());
info = getDataOperandBaseAddr(converter, builder, sym, operandLocation,
unwrapFirBox);
dataExv = converter.getSymbolExtendedValue(sym);
+ llvm::errs() << "isAssumedSizeArray? = " << dataExvIsAssumedSize << "\n";
+ llvm::errs() << "dataExv = " << dataExv << "\n";
asFortran << sym.name().ToString();
}
if (!arrayRef->subscript().empty()) {
asFortran << '(';
+ llvm::errs() << "!arrayRef->subscript().empty()\n";
bounds = genBoundsOps<BoundsOp, BoundsType>(
builder, operandLocation, converter, stmtCtx, arrayRef->subscript(),
asFortran, dataExv, dataExvIsAssumedSize, info, treatIndexAsSection,
@@ -453,8 +459,10 @@ fir::factory::AddrAndBoundsInfo gatherDataOperandAddrAndBounds(
converter.genExprAddr(operandLocation, designator, stmtCtx);
info.addr = fir::getBase(compExv);
info.rawInput = info.addr;
+ llvm::errs() << "compRef = detail::getRef<evaluate::Component>(designator)\n";
if (genDefaultBounds &&
- mlir::isa<fir::SequenceType>(fir::unwrapRefType(info.addr.getType())))
+ mlir::isa<fir::SequenceType>(fir::unwrapRefType(info.addr.getType()))) {
+ llvm::errs() << "genDefaultBounds && mlir::isa<fir::SequenceType>(fir::unwrapRefType(info.addr.getType()))\n";
bounds = fir::factory::genBaseBoundsOps<BoundsOp, BoundsType>(
builder, operandLocation, compExv,
/*isAssumedSize=*/false, strideIncludeLowerExtent);
@@ -492,6 +500,7 @@ fir::factory::AddrAndBoundsInfo gatherDataOperandAddrAndBounds(
builder, operandLocation, compExv, info);
}
} else {
+ llvm::errs() << "All else\n";
if (detail::getRef<evaluate::ArrayRef>(designator)) {
fir::ExtendedValue compExv =
converter.genExprAddr(operandLocation, designator, stmtCtx);
@@ -500,18 +509,26 @@ fir::factory::AddrAndBoundsInfo gatherDataOperandAddrAndBounds(
asFortran << designator.AsFortran();
} else if (auto symRef = detail::getRef<semantics::SymbolRef>(designator)) {
// Scalar or full array.
+ llvm::errs() << "symRef = detail::getRef<semantics::SymbolRef>(designator)\n";
fir::ExtendedValue dataExv = converter.getSymbolExtendedValue(*symRef);
info = getDataOperandBaseAddr(converter, builder, *symRef,
operandLocation, unwrapFirBox);
+ llvm::errs() << "dataExv = " << dataExv << "\n";
+ llvm::errs() << "info is \n";
+ info.dump(llvm::errs());
+ llvm ::errs() << "info.addr.getType()" << info.addr.getType() << "\n";
if (genDefaultBounds && mlir::isa<fir::BaseBoxType>(
fir::unwrapRefType(info.addr.getType()))) {
+ llvm::errs() << "genDefaultBounds && mlir::isa<fir::BaseBoxType>(fir::unwrapRefType(info.addr.getType())) \n";
info.boxType = fir::unwrapRefType(info.addr.getType());
bounds = fir::factory::genBoundsOpsFromBox<BoundsOp, BoundsType>(
builder, operandLocation, dataExv, info);
- }
+ } else
+ llvm::errs()<< "ELSE => genDefaultBounds && mlir::isa<fir::BaseBoxType>(fir::unwrapRefType(info.addr.getType())) \n";
bool dataExvIsAssumedSize =
Fortran::semantics::IsAssumedSizeArray(symRef->get().GetUltimate());
- if (genDefaultBounds &&
+ llvm::errs() << "isAssumedSizeArray? = " << dataExvIsAssumedSize << "\n";
+ if (genDefaultBounds &&
mlir::isa<fir::SequenceType>(fir::unwrapRefType(info.addr.getType())))
bounds = fir::factory::genBaseBoundsOps<BoundsOp, BoundsType>(
builder, operandLocation, dataExv, dataExvIsAssumedSize,
diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
index 77b4622547d7a..73d3f2a1cfe02 100644
--- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
@@ -21,6 +21,9 @@
#include "llvm/Frontend/OpenMP/OMP.h.inc"
#include "llvm/Frontend/OpenMP/OMPIRBuilder.h"
+#define DEBUG_TYPE "flang-openmp-lowering"
+#define PDBGS() (llvm::dbgs() << "[" << DEBUG_TYPE << "]: ")
+
namespace Fortran {
namespace lower {
namespace omp {
@@ -1054,7 +1057,15 @@ void ClauseProcessor::processMapObjects(
llvm::SmallVector<mlir::Value> bounds;
std::stringstream asFortran;
std::optional<omp::Object> parentObj;
-
+ LLVM_DEBUG(
+ LLVM_DEBUG(PDBGS() << "Sym = " << *object.sym() << "\n");
+ if (object.ref()) {
+ PDBGS() << "Designator = ";
+ object.ref().value().dump();
+ PDBGS() << "\n";
+ }
+ else
+ PDBGS() << "No Designator\n";);
fir::factory::AddrAndBoundsInfo info =
lower::gatherDataOperandAddrAndBounds<mlir::omp::MapBoundsOp,
mlir::omp::MapBoundsType>(
@@ -1062,6 +1073,17 @@ void ClauseProcessor::processMapObjects(
object.ref(), clauseLocation, asFortran, bounds,
treatIndexAsSection);
+ LLVM_DEBUG(
+ if (bounds.empty())
+ PDBGS() << "Bounds empty\n";
+ else {
+ PDBGS() << "Bounds:\n";
+ for (auto v : bounds) {
+ PDBGS() << v << "\n";
+ }
+ }
+ );
+
mlir::Value baseOp = info.rawInput;
if (object.sym()->owner().IsDerivedType()) {
omp::ObjectList objectList = gatherObjectsOf(object, semaCtx);
@@ -1089,13 +1111,26 @@ void ClauseProcessor::processMapObjects(
mapperIdName)
: mlir::FlatSymbolRefAttr();
}
-
// Explicit map captures are captured ByRef by default,
// optimisation passes may alter this to ByCopy or other capture
// types to optimise
auto location = mlir::NameLoc::get(
mlir::StringAttr::get(firOpBuilder.getContext(), asFortran.str()),
baseOp.getLoc());
+ // mlir::Type idxTy = firOpBuilder.getIndexType();
+ // mlir::Value one = firOpBuilder.createIntegerConstant(location, idxTy, 1);
+ // mlir::Value zero = firOpBuilder.createIntegerConstant(location, idxTy, 0);
+ // auto normalizedLB = zero;
+ // auto ub = firOpBuilder.createIntegerConstant(location, idxTy, 7);
+ // auto extent = firOpBuilder.createIntegerConstant(location, idxTy, 8);
+ // auto stride = one;
+ // mlir::Type boundTy = firOpBuilder.getType<mlir::omp::MapBoundsType>();
+ // mlir::Value boundsOp = firOpBuilder.create<mlir::omp::MapBoundsOp>(
+ // location, boundTy, /*lower_bound=*/normalizedLB,
+ // /*upper_bound=*/ub, /*extent=*/extent, /*stride=*/stride,
+ // /*stride_in_bytes = */ true, /*start_idx=*/normalizedLB);
+ // bounds.push_back(boundsOp);
+ // LLVM_DEBUG(PDBGS() << "Created bounds " << boundsOp << "\n");
mlir::omp::MapInfoOp mapOp = createMapInfoOp(
firOpBuilder, location, baseOp,
/*varPtrPtr=*/mlir::Value{}, asFortran.str(), bounds,
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index f099028c23323..76748d1bd9476 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -41,6 +41,28 @@
#include "llvm/ADT/STLExtras.h"
#include "llvm/Frontend/OpenMP/OMPConstants.h"
+#define DEBUG_TYPE "flang-openmp-lowering"
+#define PDBGS() (llvm::dbgs() << "[" << DEBUG_TYPE << "]: ")
+
+static void printOperation(mlir::Operation *op) { llvm::dbgs() << *op << "\n"; }
+static void printBlock(mlir::Block &block) {
+ llvm::dbgs() << "Block with " << block.getNumArguments() << " arguments, "
+ << block.getNumSuccessors()
+ << " successors, and "
+ // Note, this `.size()` is traversing a linked-list and is O(n).
+ << block.getOperations().size() << " operations\n";
+ for (mlir::Operation &op : block.getOperations())
+ printOperation(&op);
+}
+
+static void printRegion(mlir::Region ®ion) {
+ // A region does not hold anything by itself other than a list of blocks.
+ llvm::dbgs() << "Region with " << region.getBlocks().size()
+ << " blocks:\n";
+ for (mlir::Block &block : region.getBlocks())
+ printBlock(block);
+}
+
using namespace Fortran::lower::omp;
using namespace Fortran::common::openmp;
@@ -219,12 +241,23 @@ static void bindEntryBlockArgs(lower::AbstractConverter &converter,
auto bindSingleMapLike = [&converter,
&firOpBuilder](const semantics::Symbol &sym,
+ const mlir::Value val,
const mlir::BlockArgument &arg) {
// Clones the `bounds` placing them inside the entry block and returns
// them.
auto cloneBound = [&](mlir::Value bound) {
+ LLVM_DEBUG(PDBGS() << "Cloning bound " << bound << "\n");
if (mlir::isMemoryEffectFree(bound.getDefiningOp())) {
+ if (auto unboxCharOp = mlir::dyn_cast<fir::UnboxCharOp>(bound.getDefiningOp())) {
+ LLVM_DEBUG(PDBGS() << "Defining Op of Bound : " << unboxCharOp << "\n");
+ mlir::Operation *clonedOp = firOpBuilder.clone(*unboxCharOp);
+ LLVM_DEBUG(PDBGS() << "Cloned Op of Bound : " << *clonedOp << "\n");
+ return clonedOp->getResult(1);
+ }
+ mlir::Operation *defOp = bound.getDefiningOp();
+ LLVM_DEBUG(PDBGS() << "Defining Op of Bound : " << *defOp << "\n");
mlir::Operation *clonedOp = firOpBuilder.clone(*bound.getDefiningOp());
+ LLVM_DEBUG(PDBGS() << "Cloned Op of Bound : " << *clonedOp << "\n");
return clonedOp->getResult(0);
}
TODO(converter.getCurrentLocation(),
@@ -239,39 +272,72 @@ static void bindEntryBlockArgs(lower::AbstractConverter &converter,
};
fir::ExtendedValue extVal = converter.getSymbolExtendedValue(sym);
+ LLVM_DEBUG(PDBGS() << "In bindEntryBlockArgs\n");
+ LLVM_DEBUG(PDBGS() << "Sym: " << sym << "\n");
+ LLVM_DEBUG(PDBGS() << "Extended Value:\n " << extVal << "\n");
+ LLVM_DEBUG(PDBGS() << "mapBaseValue: \n" << val << "\n");
auto refType = mlir::dyn_cast<fir::ReferenceType>(arg.getType());
if (refType && fir::isa_builtin_cptr_type(refType.getElementType())) {
+ LLVM_DEBUG(PDBGS() << "binding builting_cptr_type\n");
converter.bindSymbol(sym, arg);
} else {
extVal.match(
[&](const fir::BoxValue &v) {
+ LLVM_DEBUG(PDBGS() << "binding BoxValue " << v << "\n");
converter.bindSymbol(sym,
fir::BoxValue(arg, cloneBounds(v.getLBounds()),
v.getExplicitParameters(),
v.getExplicitExtents()));
},
[&](const fir::MutableBoxValue &v) {
+ LLVM_DEBUG(PDBGS() << "binding MutableBoxValue " << v << "\n");
converter.bindSymbol(
sym, fir::MutableBoxValue(arg, cloneBounds(v.getLBounds()),
v.getMutableProperties()));
},
[&](const fir::ArrayBoxValue &v) {
+ LLVM_DEBUG(PDBGS() << "binding ArrayBoxValue " << v << "\n");
converter.bindSymbol(
sym, fir::ArrayBoxValue(arg, cloneBounds(v.getExtents()),
cloneBounds(v.getLBounds()),
v.getSourceBox()));
},
[&](const fir::CharArrayBoxValue &v) {
+ LLVM_DEBUG(PDBGS() << "binding CharArrayBoxValue " << v << "\n");
converter.bindSymbol(
sym, fir::CharArrayBoxValue(arg, cloneBound(v.getLen()),
cloneBounds(v.getExtents()),
cloneBounds(v.getLBounds())));
},
[&](const fir::CharBoxValue &v) {
- converter.bindSymbol(
- sym, fir::CharBoxValue(arg, cloneBound(v.getLen())));
+ // PDB: THe problem here is that v is
+ // [flang-openmp-lowering]: Sym: a0, INTENT(IN) (OmpMapTo) size=24 offset=0: ObjectEntity dummy type: CHARACTER(*,1)
+ // [flang-openmp-lowering]: Extended Value:
+ // boxchar { addr: %9:2 = "hlfir.declare"(%8#0, %8#1, %7) <{fortran_attrs = #fir.var_attrs<intent_in>, operandSegmentSizes = array<i32: 1, 0, 1, 1>, uniq_name = "_QFFrealtest_Ea0"}> : (!fir.ref<!fir.char<1,?>>, index, !fir.dscope) -> (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>), len: %8:2 = "fir.unboxchar"(%arg0) : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index) }
+ // Porblem above is that "len:" references the input to the hlfir.declare. It could get it directly from the hlfir.declare.
+ // PDB: start thinking from here - it looks like we'll have to map %arg after all because getting the length will still need us to access the defining op of the len, which is the unboxchar.
+ // Maybe we should use val which could be the hlfir.declare for the symbol. Use the len from that instead of cloning the len from the extended value.
+ LLVM_DEBUG(PDBGS() << "binding CharBoxValue " << v << "\n");
+ mlir::Value len = v.getLen();
+ LLVM_DEBUG(PDBGS() << "Starting with len = v.getLen() = " << len << "\n");
+ if (auto declareOp = val.getDefiningOp<hlfir::DeclareOp>()) {
+ mlir::Value base = declareOp.getBase();
+ if (auto boxCharType = mlir::dyn_cast<fir::BoxCharType>(base.getType())) {
+ LLVM_DEBUG(PDBGS() << "Type of declareOp.getBase() = " << boxCharType << "\n");
+ mlir::Type lenType = firOpBuilder.getCharacterLengthType();
+ mlir::Type refType = firOpBuilder.getRefType(boxCharType.getEleTy());
+ mlir::Location loc = converter.getCurrentLocation();
+ LLVM_DEBUG(PDBGS() << "lenType = " << lenType << "\n");
+ LLVM_DEBUG(PDBGS() << "refType = " << refType << "\n");
+ auto unboxed = firOpBuilder.create<fir::UnboxCharOp>(loc, refType, lenType, base);
+ len = unboxed.getResult(1);
+ }
+ }
+ auto charBoxValue = fir::CharBoxValue(arg, cloneBound(len));
+ LLVM_DEBUG(PDBGS() << "Binding " << sym << " to " << charBoxValue << "\n");
+ converter.bindSymbol(sym, charBoxValue);
},
- [&](const fir::UnboxedValue &v) { converter.bindSymbol(sym, arg); },
+ [&](const fir::UnboxedValue &v) { LLVM_DEBUG(PDBGS() << "binding Unboxed Value " << v << " \n");converter.bindSymbol(sym, arg); },
[&](const auto &) {
TODO(converter.getCurrentLocation(),
"target map clause operand unsupported type");
@@ -281,6 +347,7 @@ static void bindEntryBlockArgs(lower::AbstractConverter &converter,
auto bindMapLike =
[&bindSingleMapLike](llvm::ArrayRef<const semantics::Symbol *> syms,
+ llvm::ArrayRef<mlir::Value> vars,
llvm::ArrayRef<mlir::BlockArgument> args) {
// Structure component symbols don't have bindings, and can only be
// explicitly mapped individually. If a member is captured implicitly
@@ -289,8 +356,8 @@ static void bindEntryBlockArgs(lower::AbstractConverter &converter,
llvm::copy_if(syms, std::back_inserter(processedSyms),
[](auto *sym) { return !sym->owner().IsDerivedType(); });
- for (auto [sym, arg] : llvm::zip_equal(processedSyms, args))
- bindSingleMapLike(*sym, arg);
+ for (auto [sym, var, arg] : llvm::zip_equal(processedSyms, vars, args))
+ bindSingleMapLike(*sym, var, arg);
};
auto bindPrivateLike = [&converter, &firOpBuilder](
@@ -321,17 +388,17 @@ static void bindEntryBlockArgs(lower::AbstractConverter &converter,
// Process in clause name alphabetical order to match block arguments order.
// Do not bind host_eval variables because they cannot be used inside of the
// corresponding region, except for very specific cases handled separately.
- bindMapLike(args.hasDeviceAddr.syms, op.getHasDeviceAddrBlockArgs());
+ bindMapLike(args.hasDeviceAddr.syms, args.hasDeviceAddr.vars, op.getHasDeviceAddrBlockArgs());
bindPrivateLike(args.inReduction.syms, args.inReduction.vars,
op.getInReductionBlockArgs());
- bindMapLike(args.map.syms, op.getMapBlockArgs());
+ bindMapLike(args.map.syms, args.map.vars, op.getMapBlockArgs());
bindPrivateLike(args.priv.syms, args.priv.vars, op.getPrivateBlockArgs());
bindPrivateLike(args.reduction.syms, args.reduction.vars,
op.getReductionBlockArgs());
bindPrivateLike(args.taskReduction.syms, args.taskReduction.vars,
op.getTaskReductionBlockArgs());
- bindMapLike(args.useDeviceAddr.syms, op.getUseDeviceAddrBlockArgs());
- bindMapLike(args.useDevicePtr.syms, op.getUseDevicePtrBlockArgs());
+ bindMapLike(args.useDeviceAddr.syms, args.useDeviceAddr.vars, op.getUseDeviceAddrBlockArgs());
+ bindMapLike(args.useDevicePtr.syms, args.useDevicePtr.vars, op.getUseDevicePtrBlockArgs());
}
/// Get the list of base values that the specified map-like variables point to.
@@ -1357,8 +1424,13 @@ static void genBodyOfTargetOp(
auto argIface = llvm::cast<mlir::omp::BlockArgOpenMPOpInterface>(*targetOp);
mlir::Region ®ion = targetOp.getRegion();
+ mlir::func::FuncOp func = targetOp->getParentOfType<mlir::func::FuncOp>();
+ LLVM_DEBUG(PDBGS() << "Function before genEntryBlock\n " << func << "\n\n");
mlir::Block *entryBlock = genEntryBlock(firOpBuilder, args, region);
+ LLVM_DEBUG(PDBGS() << "Function after genEntryBlock\n" << func << "\n\n");
+ LLVM_DEBUG(PDBGS() << "entryBlock before bindEntryBlockArgs\n" << *entryBlock << "\n\n");
bindEntryBlockArgs(converter, targetOp, args);
+ LLVM_DEBUG(PDBGS() << "entryBlock after bindEntryBlockArgs\n" << *entryBlock << "\n\n");
if (!hostEvalInfo.empty())
hostEvalInfo.back().bindOperands(argIface.getHostEvalBlockArgs());
@@ -1368,9 +1440,26 @@ static void genBodyOfTargetOp(
// lists and replace their uses with the new temporary.
llvm::SetVector<mlir::Value> valuesDefinedAbove;
mlir::getUsedValuesDefinedAbove(region, valuesDefinedAbove);
+ LLVM_DEBUG(PDBGS() << "region is \n");
+ LLVM_DEBUG(printRegion(region));
+ LLVM_DEBUG(PDBGS() << "valuesDefinedAbove.empty() : " << valuesDefinedAbove.empty() << "\n");
while (!valuesDefinedAbove.empty()) {
for (mlir::Value val : valuesDefinedAbove) {
+ LLVM_DEBUG(PDBGS() << "Value defined above is \n" << val << "\n");
mlir::Operation *valOp = val.getDefiningOp();
+ // if (!valOp) {
+ // // This means val is a blockArg.
+ // assert(mlir::isa<mlir::BlockArgument>(val));
+ // auto blockArg = llvm::cast<mlir::BlockArgument>(val);
+ // LLVM_DEBUG(PDBGS() << "val is a BlockArgument: Arg Number: " << blockArg.getArgNumber() << "\n");
+ // mlir::OpBuilder::InsertionGuard guard(firOpBuilder);
+ // firOpBuilder.setInsertionPoint(targetOp);
+ // // firOpBuilder.setInsertionPointAfter(valOp);
+ // auto copyVal =
+ // firOpBuilder.createTemporary(val.getLoc(), val.getType());
+ // firOpBuilder.createStoreWithConvert(copyVal.getLoc(), val, copyVal);
+ // LLVM_DEBUG(PDBGS() << "Function after processing null valOp\n" << func << "\n\n");
+ // }
assert(valOp != nullptr);
// NOTE: We skip BoxDimsOp's as the lesser of two evils is to map the
@@ -2397,6 +2486,13 @@ genTargetOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
extractMappedBaseValues(clauseOps.hasDeviceAddrVars, hasDeviceAddrBaseValues);
extractMappedBaseValues(clauseOps.mapVars, mapBaseValues);
+ LLVM_DEBUG(PDBGS() << "mapVars and mapBaseValues are \n";
+ for (auto [mapSym, mapVar, mapBaseValue] : llvm::zip(mapSyms, clauseOps.mapVars, mapBaseValues)) {
+ PDBGS() << "(mapSym): " << *mapSym << "\n";
+ PDBGS() << "(mapVar): " << mapVar << "\n";
+ PDBGS() << "(mapBaseValue): " << mapBaseValue << "\n";
+ }
+ );
EntryBlockArgs args;
args.hasDeviceAddr.syms = hasDeviceAddrSyms;
args.hasDeviceAddr.vars = hasDeviceAddrBaseValues;
diff --git a/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp b/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp
index 3fcb4b04a7b76..1f116d2e243e6 100644
--- a/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp
+++ b/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp
@@ -550,7 +550,51 @@ 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
+ func->walk([&](mlir::omp::MapInfoOp op) {
+ mlir::Value varPtr = op.getVarPtr();
+ mlir::Type underlyingVarType = fir::unwrapRefType(varPtr.getType());
+ if (!mlir::isa<fir::CharacterType>(underlyingVarType))
+ return mlir::WalkResult::advance();
+
+ fir::CharacterType cType = mlir::cast<fir::CharacterType>(underlyingVarType);
+ if (!cType.hasDynamicLen())
+ return mlir::WalkResult::advance();
+
+ // This means varPtr is a BlockArgument. I do not know how to get to a
+ // fir.boxchar<> type of mlir::Value for varPtr. So, skipping this for now.
+ mlir::Operation *definingOp = varPtr.getDefiningOp();
+ if (!definingOp)
+ return mlir::WalkResult::advance();
+
+ if (auto declOp = mlir::dyn_cast<hlfir::DeclareOp>(definingOp)) {
+ mlir::Value base = declOp.getBase();
+ assert(mlir::isa<fir::BoxCharType>(base.getType()));
+ // mlir::value unboxChar
+ builder.setInsertionPoint(op);
+ fir::BoxCharType boxCharType = mlir::cast<fir::BoxCharType>(base.getType());
+ mlir::Type idxTy = builder.getIndexType();
+ mlir::Type lenType = builder.getCharacterLengthType();
+ mlir::Type refType = builder.getRefType(boxCharType.getEleTy());
+ mlir::Location location = op.getLoc();
+ auto unboxed = builder.create<fir::UnboxCharOp>(location, refType, lenType, base);
+ // len = unboxed.getResult(1);
+ mlir::Value zero = builder.createIntegerConstant(location, idxTy, 0);
+ mlir::Value one = builder.createIntegerConstant(location, idxTy, 1);
+ mlir::Value extent = unboxed.getResult(1);
+ mlir::Value stride = one;
+ mlir::Value ub = builder.create<mlir::arith::SubIOp>(location, extent, one);
+ mlir::Type boundTy = builder.getType<mlir::omp::MapBoundsType>();
+ mlir::Value boundsOp = builder.create<mlir::omp::MapBoundsOp>(
+ location, boundTy, /*lower_bound=*/zero,
+ /*upper_bound=*/ub, /*extent=*/extent, /*stride=*/stride,
+ /*stride_in_bytes = */ true, /*start_idx=*/zero);
+ op.getBoundsMutable().append({boundsOp});
+ }
+ return mlir::WalkResult::advance();
+ });
+
+ // 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 =
>From e245e4b35b960a779ede1f3713f8d48ba0cb8654 Mon Sep 17 00:00:00 2001
From: Pranav Bhandarkar <pranav.bhandarkar at amd.com>
Date: Tue, 8 Apr 2025 15:56:26 -0500
Subject: [PATCH 02/10] Removed unconditional debugging prints
---
flang/include/flang/Lower/DirectivesCommon.h | 22 +---
flang/lib/Lower/OpenMP/ClauseProcessor.cpp | 46 ++++----
flang/lib/Lower/OpenMP/OpenMP.cpp | 107 ++++++------------
.../Optimizer/OpenMP/MapInfoFinalization.cpp | 17 ++-
4 files changed, 69 insertions(+), 123 deletions(-)
diff --git a/flang/include/flang/Lower/DirectivesCommon.h b/flang/include/flang/Lower/DirectivesCommon.h
index ce03b0751b56a..de9008a9010c4 100644
--- a/flang/include/flang/Lower/DirectivesCommon.h
+++ b/flang/include/flang/Lower/DirectivesCommon.h
@@ -424,30 +424,23 @@ fir::factory::AddrAndBoundsInfo gatherDataOperandAddrAndBounds(
auto arrayBase = toMaybeExpr(arrayRef->base());
assert(arrayBase);
- llvm::errs() << "arrayBase = ";
- arrayBase.value().dump();
if (detail::getRef<evaluate::Component>(*arrayBase)) {
- llvm::errs() << "detail::getRef<evaluate::Component>(*arrayBase)\n";
dataExv = converter.genExprAddr(operandLocation, *arrayBase, stmtCtx);
info.addr = fir::getBase(dataExv);
info.rawInput = info.addr;
asFortran << arrayBase->AsFortran();
} else {
- llvm::errs() << "ELSE -> detail::getRef<evaluate::Component>(*arrayBase)\n";
const semantics::Symbol &sym = arrayRef->GetLastSymbol();
dataExvIsAssumedSize =
Fortran::semantics::IsAssumedSizeArray(sym.GetUltimate());
info = getDataOperandBaseAddr(converter, builder, sym, operandLocation,
unwrapFirBox);
dataExv = converter.getSymbolExtendedValue(sym);
- llvm::errs() << "isAssumedSizeArray? = " << dataExvIsAssumedSize << "\n";
- llvm::errs() << "dataExv = " << dataExv << "\n";
asFortran << sym.name().ToString();
}
if (!arrayRef->subscript().empty()) {
asFortran << '(';
- llvm::errs() << "!arrayRef->subscript().empty()\n";
bounds = genBoundsOps<BoundsOp, BoundsType>(
builder, operandLocation, converter, stmtCtx, arrayRef->subscript(),
asFortran, dataExv, dataExvIsAssumedSize, info, treatIndexAsSection,
@@ -459,10 +452,8 @@ fir::factory::AddrAndBoundsInfo gatherDataOperandAddrAndBounds(
converter.genExprAddr(operandLocation, designator, stmtCtx);
info.addr = fir::getBase(compExv);
info.rawInput = info.addr;
- llvm::errs() << "compRef = detail::getRef<evaluate::Component>(designator)\n";
if (genDefaultBounds &&
mlir::isa<fir::SequenceType>(fir::unwrapRefType(info.addr.getType()))) {
- llvm::errs() << "genDefaultBounds && mlir::isa<fir::SequenceType>(fir::unwrapRefType(info.addr.getType()))\n";
bounds = fir::factory::genBaseBoundsOps<BoundsOp, BoundsType>(
builder, operandLocation, compExv,
/*isAssumedSize=*/false, strideIncludeLowerExtent);
@@ -500,7 +491,6 @@ fir::factory::AddrAndBoundsInfo gatherDataOperandAddrAndBounds(
builder, operandLocation, compExv, info);
}
} else {
- llvm::errs() << "All else\n";
if (detail::getRef<evaluate::ArrayRef>(designator)) {
fir::ExtendedValue compExv =
converter.genExprAddr(operandLocation, designator, stmtCtx);
@@ -509,26 +499,18 @@ fir::factory::AddrAndBoundsInfo gatherDataOperandAddrAndBounds(
asFortran << designator.AsFortran();
} else if (auto symRef = detail::getRef<semantics::SymbolRef>(designator)) {
// Scalar or full array.
- llvm::errs() << "symRef = detail::getRef<semantics::SymbolRef>(designator)\n";
fir::ExtendedValue dataExv = converter.getSymbolExtendedValue(*symRef);
info = getDataOperandBaseAddr(converter, builder, *symRef,
operandLocation, unwrapFirBox);
- llvm::errs() << "dataExv = " << dataExv << "\n";
- llvm::errs() << "info is \n";
- info.dump(llvm::errs());
- llvm ::errs() << "info.addr.getType()" << info.addr.getType() << "\n";
if (genDefaultBounds && mlir::isa<fir::BaseBoxType>(
fir::unwrapRefType(info.addr.getType()))) {
- llvm::errs() << "genDefaultBounds && mlir::isa<fir::BaseBoxType>(fir::unwrapRefType(info.addr.getType())) \n";
info.boxType = fir::unwrapRefType(info.addr.getType());
bounds = fir::factory::genBoundsOpsFromBox<BoundsOp, BoundsType>(
builder, operandLocation, dataExv, info);
- } else
- llvm::errs()<< "ELSE => genDefaultBounds && mlir::isa<fir::BaseBoxType>(fir::unwrapRefType(info.addr.getType())) \n";
+ }
bool dataExvIsAssumedSize =
Fortran::semantics::IsAssumedSizeArray(symRef->get().GetUltimate());
- llvm::errs() << "isAssumedSizeArray? = " << dataExvIsAssumedSize << "\n";
- if (genDefaultBounds &&
+ if (genDefaultBounds &&
mlir::isa<fir::SequenceType>(fir::unwrapRefType(info.addr.getType())))
bounds = fir::factory::genBaseBoundsOps<BoundsOp, BoundsType>(
builder, operandLocation, dataExv, dataExvIsAssumedSize,
diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
index 73d3f2a1cfe02..6cf0fccc81b75 100644
--- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
@@ -1057,15 +1057,13 @@ void ClauseProcessor::processMapObjects(
llvm::SmallVector<mlir::Value> bounds;
std::stringstream asFortran;
std::optional<omp::Object> parentObj;
- LLVM_DEBUG(
- LLVM_DEBUG(PDBGS() << "Sym = " << *object.sym() << "\n");
- if (object.ref()) {
- PDBGS() << "Designator = ";
- object.ref().value().dump();
- PDBGS() << "\n";
- }
- else
- PDBGS() << "No Designator\n";);
+ LLVM_DEBUG(LLVM_DEBUG(PDBGS() << "Sym = " << *object.sym() << "\n");
+ if (object.ref()) {
+ PDBGS() << "Designator = ";
+ object.ref().value().dump();
+ PDBGS() << "\n";
+ } else PDBGS()
+ << "No Designator\n";);
fir::factory::AddrAndBoundsInfo info =
lower::gatherDataOperandAddrAndBounds<mlir::omp::MapBoundsOp,
mlir::omp::MapBoundsType>(
@@ -1073,16 +1071,12 @@ void ClauseProcessor::processMapObjects(
object.ref(), clauseLocation, asFortran, bounds,
treatIndexAsSection);
- LLVM_DEBUG(
- if (bounds.empty())
- PDBGS() << "Bounds empty\n";
- else {
- PDBGS() << "Bounds:\n";
- for (auto v : bounds) {
- PDBGS() << v << "\n";
- }
- }
- );
+ LLVM_DEBUG(if (bounds.empty()) PDBGS() << "Bounds empty\n"; else {
+ PDBGS() << "Bounds:\n";
+ for (auto v : bounds) {
+ PDBGS() << v << "\n";
+ }
+ });
mlir::Value baseOp = info.rawInput;
if (object.sym()->owner().IsDerivedType()) {
@@ -1119,13 +1113,13 @@ void ClauseProcessor::processMapObjects(
baseOp.getLoc());
// mlir::Type idxTy = firOpBuilder.getIndexType();
// mlir::Value one = firOpBuilder.createIntegerConstant(location, idxTy, 1);
- // mlir::Value zero = firOpBuilder.createIntegerConstant(location, idxTy, 0);
- // auto normalizedLB = zero;
- // auto ub = firOpBuilder.createIntegerConstant(location, idxTy, 7);
- // auto extent = firOpBuilder.createIntegerConstant(location, idxTy, 8);
- // auto stride = one;
- // mlir::Type boundTy = firOpBuilder.getType<mlir::omp::MapBoundsType>();
- // mlir::Value boundsOp = firOpBuilder.create<mlir::omp::MapBoundsOp>(
+ // mlir::Value zero = firOpBuilder.createIntegerConstant(location, idxTy,
+ // 0); auto normalizedLB = zero; auto ub =
+ // firOpBuilder.createIntegerConstant(location, idxTy, 7); auto extent =
+ // firOpBuilder.createIntegerConstant(location, idxTy, 8); auto stride =
+ // one; mlir::Type boundTy =
+ // firOpBuilder.getType<mlir::omp::MapBoundsType>(); mlir::Value boundsOp =
+ // firOpBuilder.create<mlir::omp::MapBoundsOp>(
// location, boundTy, /*lower_bound=*/normalizedLB,
// /*upper_bound=*/ub, /*extent=*/extent, /*stride=*/stride,
// /*stride_in_bytes = */ true, /*start_idx=*/normalizedLB);
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 76748d1bd9476..aef34cc920bf8 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -41,24 +41,20 @@
#include "llvm/ADT/STLExtras.h"
#include "llvm/Frontend/OpenMP/OMPConstants.h"
-#define DEBUG_TYPE "flang-openmp-lowering"
-#define PDBGS() (llvm::dbgs() << "[" << DEBUG_TYPE << "]: ")
-
static void printOperation(mlir::Operation *op) { llvm::dbgs() << *op << "\n"; }
static void printBlock(mlir::Block &block) {
llvm::dbgs() << "Block with " << block.getNumArguments() << " arguments, "
- << block.getNumSuccessors()
- << " successors, and "
- // Note, this `.size()` is traversing a linked-list and is O(n).
- << block.getOperations().size() << " operations\n";
+ << block.getNumSuccessors()
+ << " successors, and "
+ // Note, this `.size()` is traversing a linked-list and is O(n).
+ << block.getOperations().size() << " operations\n";
for (mlir::Operation &op : block.getOperations())
printOperation(&op);
}
static void printRegion(mlir::Region ®ion) {
// A region does not hold anything by itself other than a list of blocks.
- llvm::dbgs() << "Region with " << region.getBlocks().size()
- << " blocks:\n";
+ llvm::dbgs() << "Region with " << region.getBlocks().size() << " blocks:\n";
for (mlir::Block &block : region.getBlocks())
printBlock(block);
}
@@ -246,18 +242,13 @@ static void bindEntryBlockArgs(lower::AbstractConverter &converter,
// Clones the `bounds` placing them inside the entry block and returns
// them.
auto cloneBound = [&](mlir::Value bound) {
- LLVM_DEBUG(PDBGS() << "Cloning bound " << bound << "\n");
if (mlir::isMemoryEffectFree(bound.getDefiningOp())) {
- if (auto unboxCharOp = mlir::dyn_cast<fir::UnboxCharOp>(bound.getDefiningOp())) {
- LLVM_DEBUG(PDBGS() << "Defining Op of Bound : " << unboxCharOp << "\n");
+ if (auto unboxCharOp =
+ mlir::dyn_cast<fir::UnboxCharOp>(bound.getDefiningOp())) {
mlir::Operation *clonedOp = firOpBuilder.clone(*unboxCharOp);
- LLVM_DEBUG(PDBGS() << "Cloned Op of Bound : " << *clonedOp << "\n");
return clonedOp->getResult(1);
}
- mlir::Operation *defOp = bound.getDefiningOp();
- LLVM_DEBUG(PDBGS() << "Defining Op of Bound : " << *defOp << "\n");
mlir::Operation *clonedOp = firOpBuilder.clone(*bound.getDefiningOp());
- LLVM_DEBUG(PDBGS() << "Cloned Op of Bound : " << *clonedOp << "\n");
return clonedOp->getResult(0);
}
TODO(converter.getCurrentLocation(),
@@ -272,38 +263,29 @@ static void bindEntryBlockArgs(lower::AbstractConverter &converter,
};
fir::ExtendedValue extVal = converter.getSymbolExtendedValue(sym);
- LLVM_DEBUG(PDBGS() << "In bindEntryBlockArgs\n");
- LLVM_DEBUG(PDBGS() << "Sym: " << sym << "\n");
- LLVM_DEBUG(PDBGS() << "Extended Value:\n " << extVal << "\n");
- LLVM_DEBUG(PDBGS() << "mapBaseValue: \n" << val << "\n");
auto refType = mlir::dyn_cast<fir::ReferenceType>(arg.getType());
if (refType && fir::isa_builtin_cptr_type(refType.getElementType())) {
- LLVM_DEBUG(PDBGS() << "binding builting_cptr_type\n");
converter.bindSymbol(sym, arg);
} else {
extVal.match(
[&](const fir::BoxValue &v) {
- LLVM_DEBUG(PDBGS() << "binding BoxValue " << v << "\n");
converter.bindSymbol(sym,
fir::BoxValue(arg, cloneBounds(v.getLBounds()),
v.getExplicitParameters(),
v.getExplicitExtents()));
},
[&](const fir::MutableBoxValue &v) {
- LLVM_DEBUG(PDBGS() << "binding MutableBoxValue " << v << "\n");
converter.bindSymbol(
sym, fir::MutableBoxValue(arg, cloneBounds(v.getLBounds()),
v.getMutableProperties()));
},
[&](const fir::ArrayBoxValue &v) {
- LLVM_DEBUG(PDBGS() << "binding ArrayBoxValue " << v << "\n");
converter.bindSymbol(
sym, fir::ArrayBoxValue(arg, cloneBounds(v.getExtents()),
cloneBounds(v.getLBounds()),
v.getSourceBox()));
},
[&](const fir::CharArrayBoxValue &v) {
- LLVM_DEBUG(PDBGS() << "binding CharArrayBoxValue " << v << "\n");
converter.bindSymbol(
sym, fir::CharArrayBoxValue(arg, cloneBound(v.getLen()),
cloneBounds(v.getExtents()),
@@ -311,33 +293,41 @@ static void bindEntryBlockArgs(lower::AbstractConverter &converter,
},
[&](const fir::CharBoxValue &v) {
// PDB: THe problem here is that v is
- // [flang-openmp-lowering]: Sym: a0, INTENT(IN) (OmpMapTo) size=24 offset=0: ObjectEntity dummy type: CHARACTER(*,1)
+ // [flang-openmp-lowering]: Sym: a0, INTENT(IN) (OmpMapTo) size=24
+ // offset=0: ObjectEntity dummy type: CHARACTER(*,1)
// [flang-openmp-lowering]: Extended Value:
- // boxchar { addr: %9:2 = "hlfir.declare"(%8#0, %8#1, %7) <{fortran_attrs = #fir.var_attrs<intent_in>, operandSegmentSizes = array<i32: 1, 0, 1, 1>, uniq_name = "_QFFrealtest_Ea0"}> : (!fir.ref<!fir.char<1,?>>, index, !fir.dscope) -> (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>), len: %8:2 = "fir.unboxchar"(%arg0) : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index) }
- // Porblem above is that "len:" references the input to the hlfir.declare. It could get it directly from the hlfir.declare.
- // PDB: start thinking from here - it looks like we'll have to map %arg after all because getting the length will still need us to access the defining op of the len, which is the unboxchar.
- // Maybe we should use val which could be the hlfir.declare for the symbol. Use the len from that instead of cloning the len from the extended value.
- LLVM_DEBUG(PDBGS() << "binding CharBoxValue " << v << "\n");
+ // boxchar { addr: %9:2 = "hlfir.declare"(%8#0, %8#1, %7)
+ // <{fortran_attrs = #fir.var_attrs<intent_in>, operandSegmentSizes
+ // = array<i32: 1, 0, 1, 1>, uniq_name = "_QFFrealtest_Ea0"}> :
+ // (!fir.ref<!fir.char<1,?>>, index, !fir.dscope) ->
+ // (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>), len: %8:2 =
+ // "fir.unboxchar"(%arg0) : (!fir.boxchar<1>) ->
+ // (!fir.ref<!fir.char<1,?>>, index) } Porblem above is that "len:"
+ // references the input to the hlfir.declare. It could get it
+ // directly from the hlfir.declare. PDB: start thinking from here -
+ // it looks like we'll have to map %arg after all because getting
+ // the length will still need us to access the defining op of the
+ // len, which is the unboxchar. Maybe we should use val which could
+ // be the hlfir.declare for the symbol. Use the len from that
+ // instead of cloning the len from the extended value.
mlir::Value len = v.getLen();
- LLVM_DEBUG(PDBGS() << "Starting with len = v.getLen() = " << len << "\n");
if (auto declareOp = val.getDefiningOp<hlfir::DeclareOp>()) {
mlir::Value base = declareOp.getBase();
- if (auto boxCharType = mlir::dyn_cast<fir::BoxCharType>(base.getType())) {
- LLVM_DEBUG(PDBGS() << "Type of declareOp.getBase() = " << boxCharType << "\n");
+ if (auto boxCharType =
+ mlir::dyn_cast<fir::BoxCharType>(base.getType())) {
mlir::Type lenType = firOpBuilder.getCharacterLengthType();
- mlir::Type refType = firOpBuilder.getRefType(boxCharType.getEleTy());
+ mlir::Type refType =
+ firOpBuilder.getRefType(boxCharType.getEleTy());
mlir::Location loc = converter.getCurrentLocation();
- LLVM_DEBUG(PDBGS() << "lenType = " << lenType << "\n");
- LLVM_DEBUG(PDBGS() << "refType = " << refType << "\n");
- auto unboxed = firOpBuilder.create<fir::UnboxCharOp>(loc, refType, lenType, base);
+ auto unboxed = firOpBuilder.create<fir::UnboxCharOp>(
+ loc, refType, lenType, base);
len = unboxed.getResult(1);
}
}
auto charBoxValue = fir::CharBoxValue(arg, cloneBound(len));
- LLVM_DEBUG(PDBGS() << "Binding " << sym << " to " << charBoxValue << "\n");
converter.bindSymbol(sym, charBoxValue);
},
- [&](const fir::UnboxedValue &v) { LLVM_DEBUG(PDBGS() << "binding Unboxed Value " << v << " \n");converter.bindSymbol(sym, arg); },
+ [&](const fir::UnboxedValue &v) { converter.bindSymbol(sym, arg); },
[&](const auto &) {
TODO(converter.getCurrentLocation(),
"target map clause operand unsupported type");
@@ -388,7 +378,8 @@ static void bindEntryBlockArgs(lower::AbstractConverter &converter,
// Process in clause name alphabetical order to match block arguments order.
// Do not bind host_eval variables because they cannot be used inside of the
// corresponding region, except for very specific cases handled separately.
- bindMapLike(args.hasDeviceAddr.syms, args.hasDeviceAddr.vars, op.getHasDeviceAddrBlockArgs());
+ bindMapLike(args.hasDeviceAddr.syms, args.hasDeviceAddr.vars,
+ op.getHasDeviceAddrBlockArgs());
bindPrivateLike(args.inReduction.syms, args.inReduction.vars,
op.getInReductionBlockArgs());
bindMapLike(args.map.syms, args.map.vars, op.getMapBlockArgs());
@@ -397,8 +388,10 @@ static void bindEntryBlockArgs(lower::AbstractConverter &converter,
op.getReductionBlockArgs());
bindPrivateLike(args.taskReduction.syms, args.taskReduction.vars,
op.getTaskReductionBlockArgs());
- bindMapLike(args.useDeviceAddr.syms, args.useDeviceAddr.vars, op.getUseDeviceAddrBlockArgs());
- bindMapLike(args.useDevicePtr.syms, args.useDevicePtr.vars, op.getUseDevicePtrBlockArgs());
+ bindMapLike(args.useDeviceAddr.syms, args.useDeviceAddr.vars,
+ op.getUseDeviceAddrBlockArgs());
+ bindMapLike(args.useDevicePtr.syms, args.useDevicePtr.vars,
+ op.getUseDevicePtrBlockArgs());
}
/// Get the list of base values that the specified map-like variables point to.
@@ -1425,12 +1418,8 @@ static void genBodyOfTargetOp(
mlir::Region ®ion = targetOp.getRegion();
mlir::func::FuncOp func = targetOp->getParentOfType<mlir::func::FuncOp>();
- LLVM_DEBUG(PDBGS() << "Function before genEntryBlock\n " << func << "\n\n");
mlir::Block *entryBlock = genEntryBlock(firOpBuilder, args, region);
- LLVM_DEBUG(PDBGS() << "Function after genEntryBlock\n" << func << "\n\n");
- LLVM_DEBUG(PDBGS() << "entryBlock before bindEntryBlockArgs\n" << *entryBlock << "\n\n");
bindEntryBlockArgs(converter, targetOp, args);
- LLVM_DEBUG(PDBGS() << "entryBlock after bindEntryBlockArgs\n" << *entryBlock << "\n\n");
if (!hostEvalInfo.empty())
hostEvalInfo.back().bindOperands(argIface.getHostEvalBlockArgs());
@@ -1440,26 +1429,9 @@ static void genBodyOfTargetOp(
// lists and replace their uses with the new temporary.
llvm::SetVector<mlir::Value> valuesDefinedAbove;
mlir::getUsedValuesDefinedAbove(region, valuesDefinedAbove);
- LLVM_DEBUG(PDBGS() << "region is \n");
- LLVM_DEBUG(printRegion(region));
- LLVM_DEBUG(PDBGS() << "valuesDefinedAbove.empty() : " << valuesDefinedAbove.empty() << "\n");
while (!valuesDefinedAbove.empty()) {
for (mlir::Value val : valuesDefinedAbove) {
- LLVM_DEBUG(PDBGS() << "Value defined above is \n" << val << "\n");
mlir::Operation *valOp = val.getDefiningOp();
- // if (!valOp) {
- // // This means val is a blockArg.
- // assert(mlir::isa<mlir::BlockArgument>(val));
- // auto blockArg = llvm::cast<mlir::BlockArgument>(val);
- // LLVM_DEBUG(PDBGS() << "val is a BlockArgument: Arg Number: " << blockArg.getArgNumber() << "\n");
- // mlir::OpBuilder::InsertionGuard guard(firOpBuilder);
- // firOpBuilder.setInsertionPoint(targetOp);
- // // firOpBuilder.setInsertionPointAfter(valOp);
- // auto copyVal =
- // firOpBuilder.createTemporary(val.getLoc(), val.getType());
- // firOpBuilder.createStoreWithConvert(copyVal.getLoc(), val, copyVal);
- // LLVM_DEBUG(PDBGS() << "Function after processing null valOp\n" << func << "\n\n");
- // }
assert(valOp != nullptr);
// NOTE: We skip BoxDimsOp's as the lesser of two evils is to map the
@@ -2486,13 +2458,6 @@ genTargetOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
extractMappedBaseValues(clauseOps.hasDeviceAddrVars, hasDeviceAddrBaseValues);
extractMappedBaseValues(clauseOps.mapVars, mapBaseValues);
- LLVM_DEBUG(PDBGS() << "mapVars and mapBaseValues are \n";
- for (auto [mapSym, mapVar, mapBaseValue] : llvm::zip(mapSyms, clauseOps.mapVars, mapBaseValues)) {
- PDBGS() << "(mapSym): " << *mapSym << "\n";
- PDBGS() << "(mapVar): " << mapVar << "\n";
- PDBGS() << "(mapBaseValue): " << mapBaseValue << "\n";
- }
- );
EntryBlockArgs args;
args.hasDeviceAddr.syms = hasDeviceAddrSyms;
args.hasDeviceAddr.vars = hasDeviceAddrBaseValues;
diff --git a/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp b/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp
index 1f116d2e243e6..7cb0f63ba9b69 100644
--- a/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp
+++ b/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp
@@ -550,19 +550,21 @@ class MapInfoFinalizationPass
// iterations from previous function scopes.
localBoxAllocas.clear();
- // First, walk `omp.map.info` ops to see if any
+ // First, walk `omp.map.info` ops to see if any
func->walk([&](mlir::omp::MapInfoOp op) {
mlir::Value varPtr = op.getVarPtr();
mlir::Type underlyingVarType = fir::unwrapRefType(varPtr.getType());
if (!mlir::isa<fir::CharacterType>(underlyingVarType))
return mlir::WalkResult::advance();
- fir::CharacterType cType = mlir::cast<fir::CharacterType>(underlyingVarType);
+ fir::CharacterType cType =
+ mlir::cast<fir::CharacterType>(underlyingVarType);
if (!cType.hasDynamicLen())
return mlir::WalkResult::advance();
// This means varPtr is a BlockArgument. I do not know how to get to a
- // fir.boxchar<> type of mlir::Value for varPtr. So, skipping this for now.
+ // fir.boxchar<> type of mlir::Value for varPtr. So, skipping this for
+ // now.
mlir::Operation *definingOp = varPtr.getDefiningOp();
if (!definingOp)
return mlir::WalkResult::advance();
@@ -572,18 +574,21 @@ class MapInfoFinalizationPass
assert(mlir::isa<fir::BoxCharType>(base.getType()));
// mlir::value unboxChar
builder.setInsertionPoint(op);
- fir::BoxCharType boxCharType = mlir::cast<fir::BoxCharType>(base.getType());
+ fir::BoxCharType boxCharType =
+ mlir::cast<fir::BoxCharType>(base.getType());
mlir::Type idxTy = builder.getIndexType();
mlir::Type lenType = builder.getCharacterLengthType();
mlir::Type refType = builder.getRefType(boxCharType.getEleTy());
mlir::Location location = op.getLoc();
- auto unboxed = builder.create<fir::UnboxCharOp>(location, refType, lenType, base);
+ auto unboxed = builder.create<fir::UnboxCharOp>(location, refType,
+ lenType, base);
// len = unboxed.getResult(1);
mlir::Value zero = builder.createIntegerConstant(location, idxTy, 0);
mlir::Value one = builder.createIntegerConstant(location, idxTy, 1);
mlir::Value extent = unboxed.getResult(1);
mlir::Value stride = one;
- mlir::Value ub = builder.create<mlir::arith::SubIOp>(location, extent, one);
+ mlir::Value ub =
+ builder.create<mlir::arith::SubIOp>(location, extent, one);
mlir::Type boundTy = builder.getType<mlir::omp::MapBoundsType>();
mlir::Value boundsOp = builder.create<mlir::omp::MapBoundsOp>(
location, boundTy, /*lower_bound=*/zero,
>From 79054de361a9e890eccbb401c66a2a38a94d7311 Mon Sep 17 00:00:00 2001
From: Pranav Bhandarkar <pranav.bhandarkar at amd.com>
Date: Tue, 8 Apr 2025 17:07:30 -0500
Subject: [PATCH 03/10] More cleanup
---
flang/include/flang/Lower/DirectivesCommon.h | 3 +-
flang/lib/Lower/OpenMP/ClauseProcessor.cpp | 31 --------------------
2 files changed, 2 insertions(+), 32 deletions(-)
diff --git a/flang/include/flang/Lower/DirectivesCommon.h b/flang/include/flang/Lower/DirectivesCommon.h
index de9008a9010c4..93ab2e350d035 100644
--- a/flang/include/flang/Lower/DirectivesCommon.h
+++ b/flang/include/flang/Lower/DirectivesCommon.h
@@ -424,6 +424,7 @@ fir::factory::AddrAndBoundsInfo gatherDataOperandAddrAndBounds(
auto arrayBase = toMaybeExpr(arrayRef->base());
assert(arrayBase);
+
if (detail::getRef<evaluate::Component>(*arrayBase)) {
dataExv = converter.genExprAddr(operandLocation, *arrayBase, stmtCtx);
info.addr = fir::getBase(dataExv);
@@ -453,7 +454,7 @@ fir::factory::AddrAndBoundsInfo gatherDataOperandAddrAndBounds(
info.addr = fir::getBase(compExv);
info.rawInput = info.addr;
if (genDefaultBounds &&
- mlir::isa<fir::SequenceType>(fir::unwrapRefType(info.addr.getType()))) {
+ mlir::isa<fir::SequenceType>(fir::unwrapRefType(info.addr.getType())))
bounds = fir::factory::genBaseBoundsOps<BoundsOp, BoundsType>(
builder, operandLocation, compExv,
/*isAssumedSize=*/false, strideIncludeLowerExtent);
diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
index 6cf0fccc81b75..6322e7c0c5b41 100644
--- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
@@ -21,9 +21,6 @@
#include "llvm/Frontend/OpenMP/OMP.h.inc"
#include "llvm/Frontend/OpenMP/OMPIRBuilder.h"
-#define DEBUG_TYPE "flang-openmp-lowering"
-#define PDBGS() (llvm::dbgs() << "[" << DEBUG_TYPE << "]: ")
-
namespace Fortran {
namespace lower {
namespace omp {
@@ -1057,13 +1054,6 @@ void ClauseProcessor::processMapObjects(
llvm::SmallVector<mlir::Value> bounds;
std::stringstream asFortran;
std::optional<omp::Object> parentObj;
- LLVM_DEBUG(LLVM_DEBUG(PDBGS() << "Sym = " << *object.sym() << "\n");
- if (object.ref()) {
- PDBGS() << "Designator = ";
- object.ref().value().dump();
- PDBGS() << "\n";
- } else PDBGS()
- << "No Designator\n";);
fir::factory::AddrAndBoundsInfo info =
lower::gatherDataOperandAddrAndBounds<mlir::omp::MapBoundsOp,
mlir::omp::MapBoundsType>(
@@ -1071,13 +1061,6 @@ void ClauseProcessor::processMapObjects(
object.ref(), clauseLocation, asFortran, bounds,
treatIndexAsSection);
- LLVM_DEBUG(if (bounds.empty()) PDBGS() << "Bounds empty\n"; else {
- PDBGS() << "Bounds:\n";
- for (auto v : bounds) {
- PDBGS() << v << "\n";
- }
- });
-
mlir::Value baseOp = info.rawInput;
if (object.sym()->owner().IsDerivedType()) {
omp::ObjectList objectList = gatherObjectsOf(object, semaCtx);
@@ -1111,20 +1094,6 @@ void ClauseProcessor::processMapObjects(
auto location = mlir::NameLoc::get(
mlir::StringAttr::get(firOpBuilder.getContext(), asFortran.str()),
baseOp.getLoc());
- // mlir::Type idxTy = firOpBuilder.getIndexType();
- // mlir::Value one = firOpBuilder.createIntegerConstant(location, idxTy, 1);
- // mlir::Value zero = firOpBuilder.createIntegerConstant(location, idxTy,
- // 0); auto normalizedLB = zero; auto ub =
- // firOpBuilder.createIntegerConstant(location, idxTy, 7); auto extent =
- // firOpBuilder.createIntegerConstant(location, idxTy, 8); auto stride =
- // one; mlir::Type boundTy =
- // firOpBuilder.getType<mlir::omp::MapBoundsType>(); mlir::Value boundsOp =
- // firOpBuilder.create<mlir::omp::MapBoundsOp>(
- // location, boundTy, /*lower_bound=*/normalizedLB,
- // /*upper_bound=*/ub, /*extent=*/extent, /*stride=*/stride,
- // /*stride_in_bytes = */ true, /*start_idx=*/normalizedLB);
- // bounds.push_back(boundsOp);
- // LLVM_DEBUG(PDBGS() << "Created bounds " << boundsOp << "\n");
mlir::omp::MapInfoOp mapOp = createMapInfoOp(
firOpBuilder, location, baseOp,
/*varPtrPtr=*/mlir::Value{}, asFortran.str(), bounds,
>From ed1f64160be37dd7db98c95c521ac79e30cb9189 Mon Sep 17 00:00:00 2001
From: Pranav Bhandarkar <pranav.bhandarkar at amd.com>
Date: Tue, 8 Apr 2025 17:18:17 -0500
Subject: [PATCH 04/10] add a testcase
---
flang/test/Lower/OpenMP/map-character.f90 | 47 +++++++++++++++++++++++
1 file changed, 47 insertions(+)
create mode 100644 flang/test/Lower/OpenMP/map-character.f90
diff --git a/flang/test/Lower/OpenMP/map-character.f90 b/flang/test/Lower/OpenMP/map-character.f90
new file mode 100644
index 0000000000000..2ed2397713b5d
--- /dev/null
+++ b/flang/test/Lower/OpenMP/map-character.f90
@@ -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: %[[A1_BOXCHAR_ALLOCA:.*]] = fir.alloca !fir.boxchar<1>
+!CHECK: %[[A0_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: fir.store %[[A0_DECL]]#0 to %[[A0_BOXCHAR_ALLOCA]] : !fir.ref<!fir.boxchar<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: fir.store %[[A1_DECL]]#0 to %[[A1_BOXCHAR_ALLOCA]] : !fir.ref<!fir.boxchar<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: %[[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: %[[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: omp.target map_entries(%[[A0_MAP]] -> %[[TGT_A0:.*]], %[[A1_MAP]] -> %[[TGT_A1:.*]], %[[A0_BOXCHAR_MAP]] -> %[[TGT_A0_BOXCHAR:.*]], %[[A1_BOXCHAR_MAP]] -> %[[TGT_A1_BOXCHAR:.*]] : !fir.ref<!fir.char<1,?>>, !fir.ref<!fir.char<1,?>>, !fir.ref<!fir.boxchar<1>>, !fir.ref<!fir.boxchar<1>>) {
+!CHECK: %[[TGT_A1_BC_LD:.*]] = fir.load %[[TGT_A1_BOXCHAR]] : !fir.ref<!fir.boxchar<1>>
+!CHECK: %[[TGT_A0_BC_LD:.*]] = fir.load %[[TGT_A0_BOXCHAR]] : !fir.ref<!fir.boxchar<1>>
+!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: %[[UNBOXED_TGT_A1:.*]]:2 = fir.unboxchar %[[TGT_A1_BC_LD]] : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index)
+!CHECK: %[[TGT_A1_DECL:.*]]:2 = hlfir.declare %[[TGT_A1]] typeparams %[[UNBOXED_TGT_A1]]#1 {{.*}} -> (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>)
+
>From 715baf491e4f9e3456aa12a4f2b2b6ce6aa03fba Mon Sep 17 00:00:00 2001
From: Pranav Bhandarkar <pranav.bhandarkar at amd.com>
Date: Tue, 8 Apr 2025 17:18:50 -0500
Subject: [PATCH 05/10] clean up som more
---
flang/lib/Lower/OpenMP/ClauseProcessor.cpp | 2 ++
flang/lib/Lower/OpenMP/OpenMP.cpp | 18 ------------------
2 files changed, 2 insertions(+), 18 deletions(-)
diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
index 6322e7c0c5b41..77b4622547d7a 100644
--- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
@@ -1054,6 +1054,7 @@ void ClauseProcessor::processMapObjects(
llvm::SmallVector<mlir::Value> bounds;
std::stringstream asFortran;
std::optional<omp::Object> parentObj;
+
fir::factory::AddrAndBoundsInfo info =
lower::gatherDataOperandAddrAndBounds<mlir::omp::MapBoundsOp,
mlir::omp::MapBoundsType>(
@@ -1088,6 +1089,7 @@ void ClauseProcessor::processMapObjects(
mapperIdName)
: mlir::FlatSymbolRefAttr();
}
+
// Explicit map captures are captured ByRef by default,
// optimisation passes may alter this to ByCopy or other capture
// types to optimise
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index aef34cc920bf8..86974dea8f758 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -41,24 +41,6 @@
#include "llvm/ADT/STLExtras.h"
#include "llvm/Frontend/OpenMP/OMPConstants.h"
-static void printOperation(mlir::Operation *op) { llvm::dbgs() << *op << "\n"; }
-static void printBlock(mlir::Block &block) {
- llvm::dbgs() << "Block with " << block.getNumArguments() << " arguments, "
- << block.getNumSuccessors()
- << " successors, and "
- // Note, this `.size()` is traversing a linked-list and is O(n).
- << block.getOperations().size() << " operations\n";
- for (mlir::Operation &op : block.getOperations())
- printOperation(&op);
-}
-
-static void printRegion(mlir::Region ®ion) {
- // A region does not hold anything by itself other than a list of blocks.
- llvm::dbgs() << "Region with " << region.getBlocks().size() << " blocks:\n";
- for (mlir::Block &block : region.getBlocks())
- printBlock(block);
-}
-
using namespace Fortran::lower::omp;
using namespace Fortran::common::openmp;
>From f142c969d59ad9a2e1b389ffc1df054f21c77c8f Mon Sep 17 00:00:00 2001
From: Pranav Bhandarkar <pranav.bhandarkar at amd.com>
Date: Tue, 8 Apr 2025 17:38:59 -0500
Subject: [PATCH 06/10] clean up flang/lib/Lower/OpenMP/OpenMP.cpp
---
flang/lib/Lower/OpenMP/OpenMP.cpp | 60 +++++++++++++++++++------------
1 file changed, 37 insertions(+), 23 deletions(-)
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 86974dea8f758..39c5f82f0101b 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -225,12 +225,12 @@ static void bindEntryBlockArgs(lower::AbstractConverter &converter,
// them.
auto cloneBound = [&](mlir::Value bound) {
if (mlir::isMemoryEffectFree(bound.getDefiningOp())) {
- if (auto unboxCharOp =
- mlir::dyn_cast<fir::UnboxCharOp>(bound.getDefiningOp())) {
- mlir::Operation *clonedOp = firOpBuilder.clone(*unboxCharOp);
+ mlir::Operation *definingOp = bound.getDefiningOp();
+ mlir::Operation *clonedOp = firOpBuilder.clone(*definingOp);
+ // Todo: Do we need to check for more operation types?
+ // For now, specializing only for fir::UnboxCharOp
+ if (auto unboxCharOp = mlir::dyn_cast<fir::UnboxCharOp>(definingOp))
return clonedOp->getResult(1);
- }
- mlir::Operation *clonedOp = firOpBuilder.clone(*bound.getDefiningOp());
return clonedOp->getResult(0);
}
TODO(converter.getCurrentLocation(),
@@ -274,24 +274,38 @@ static void bindEntryBlockArgs(lower::AbstractConverter &converter,
cloneBounds(v.getLBounds())));
},
[&](const fir::CharBoxValue &v) {
- // PDB: THe problem here is that v is
- // [flang-openmp-lowering]: Sym: a0, INTENT(IN) (OmpMapTo) size=24
- // offset=0: ObjectEntity dummy type: CHARACTER(*,1)
- // [flang-openmp-lowering]: Extended Value:
- // boxchar { addr: %9:2 = "hlfir.declare"(%8#0, %8#1, %7)
- // <{fortran_attrs = #fir.var_attrs<intent_in>, operandSegmentSizes
- // = array<i32: 1, 0, 1, 1>, uniq_name = "_QFFrealtest_Ea0"}> :
- // (!fir.ref<!fir.char<1,?>>, index, !fir.dscope) ->
- // (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>), len: %8:2 =
- // "fir.unboxchar"(%arg0) : (!fir.boxchar<1>) ->
- // (!fir.ref<!fir.char<1,?>>, index) } Porblem above is that "len:"
- // references the input to the hlfir.declare. It could get it
- // directly from the hlfir.declare. PDB: start thinking from here -
- // it looks like we'll have to map %arg after all because getting
- // the length will still need us to access the defining op of the
- // len, which is the unboxchar. Maybe we should use val which could
- // be the hlfir.declare for the symbol. Use the len from that
- // instead of cloning the len from the extended value.
+ // In some cases, v.len could reference the input the hlfir.declare
+ // which is the corresponding v.addr. While, this isn't a big
+ // problem by itself, it is desirable to extract this out of v.addr
+ // itself since it's first result will be of type fir.boxchar<>. For
+ // example, consider the following
+ //
+ // func.func private @_QFPrealtest(%arg0: !fir.boxchar<1>)
+ // %2 = fir.dummy_scope : !fir.dscope
+ // %3:2 = fir.unboxchar %arg0 : (!fir.boxchar<1>) ->
+ // (!fir.ref<!fir.char<1,?>>, index) %4:2 = hlfir.declare %3#0
+ // typeparams %3#1 dummy_scope %2 : (!fir.ref<!fir.char<1,?>>,
+ // index,
+ // !fir.dscope) -> (!fir.boxchar<1>,
+ // !fir.ref<!fir.char<1,?>>)
+
+ // In the case above,
+ // v.addr is
+ // %4:2 = hlfir.declare %3#0 typeparams %3#1 dummy_scope %2 :
+ // (!fir.ref<!fir.char<1,?>>, index,
+ // !fir.dscope) -> (!fir.boxchar<1>,
+ // !fir.ref<!fir.char<1,?>>)
+ // v.len is
+ // %3:2 = fir.unboxchar %arg0 : (!fir.boxchar<1>) ->
+ // (!fir.ref<!fir.char<1,?>>, index)
+
+ // Mapping this to the target will create a use of %arg0 on the
+ // target. Since omp.target is IsolatedFromAbove, this will have to
+ // be mapped. Presently, OpenMP lowering of target barfs when it has
+ // to map a value that doesnt have a defining op. This can be fixed.
+ // Or we ensure that v.len = fir.unboxchar %4#0. Now if %4:2 is
+ // mapped to the target, there wont be any use of the block argument
+ // %arg0 on the target.
mlir::Value len = v.getLen();
if (auto declareOp = val.getDefiningOp<hlfir::DeclareOp>()) {
mlir::Value base = declareOp.getBase();
>From 06874f5b341ab97694b565c47b9adaab1f91a591 Mon Sep 17 00:00:00 2001
From: Pranav Bhandarkar <pranav.bhandarkar at amd.com>
Date: Tue, 8 Apr 2025 17:45:38 -0500
Subject: [PATCH 07/10] clean up a large comment in
flang/lib/Lower/OpenMP/OpenMP.cpp
---
flang/lib/Lower/OpenMP/OpenMP.cpp | 38 +++++++++++++++----------------
1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 39c5f82f0101b..97331bcbac258 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -274,38 +274,38 @@ static void bindEntryBlockArgs(lower::AbstractConverter &converter,
cloneBounds(v.getLBounds())));
},
[&](const fir::CharBoxValue &v) {
- // In some cases, v.len could reference the input the hlfir.declare
- // which is the corresponding v.addr. While, this isn't a big
- // problem by itself, it is desirable to extract this out of v.addr
- // itself since it's first result will be of type fir.boxchar<>. For
- // example, consider the following
+ // In some cases, v.len could reference the input to the
+ // hlfir.declare which is the corresponding v.addr. While this isn't
+ // a big problem by itself, it is desirable to extract this out of
+ // v.addr itself since it's first result will be of type
+ // fir.boxchar<>. For example, consider the following
//
// func.func private @_QFPrealtest(%arg0: !fir.boxchar<1>)
// %2 = fir.dummy_scope : !fir.dscope
// %3:2 = fir.unboxchar %arg0 : (!fir.boxchar<1>) ->
- // (!fir.ref<!fir.char<1,?>>, index) %4:2 = hlfir.declare %3#0
- // typeparams %3#1 dummy_scope %2 : (!fir.ref<!fir.char<1,?>>,
- // index,
- // !fir.dscope) -> (!fir.boxchar<1>,
- // !fir.ref<!fir.char<1,?>>)
+ // (!fir.ref<!fir.char<1,?>>, index)
+ // %4:2 = hlfir.declare (%3#0, %3#1, %2):(!fir.ref<!fir.char<1,?>>,
+ // index,!fir.dscope) ->
+ // (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>)
// In the case above,
// v.addr is
- // %4:2 = hlfir.declare %3#0 typeparams %3#1 dummy_scope %2 :
- // (!fir.ref<!fir.char<1,?>>, index,
- // !fir.dscope) -> (!fir.boxchar<1>,
- // !fir.ref<!fir.char<1,?>>)
+ // %4:2 = hlfir.declare (%3#0, %3#1, %2):(!fir.ref<!fir.char<1,?>>,
+ // index,!fir.dscope) ->
+ // (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>)
// v.len is
// %3:2 = fir.unboxchar %arg0 : (!fir.boxchar<1>) ->
- // (!fir.ref<!fir.char<1,?>>, index)
+ // (!fir.ref<!fir.char<1,?>>, index)
// Mapping this to the target will create a use of %arg0 on the
- // target. Since omp.target is IsolatedFromAbove, this will have to
+ // target. Since omp.target is IsolatedFromAbove, %arg0 will have to
// be mapped. Presently, OpenMP lowering of target barfs when it has
// to map a value that doesnt have a defining op. This can be fixed.
- // Or we ensure that v.len = fir.unboxchar %4#0. Now if %4:2 is
- // mapped to the target, there wont be any use of the block argument
- // %arg0 on the target.
+ // Or we ensure that v.len is fir.unboxchar %4#0 which will
+ // cause %4#1 to be used on the target and consequently be
+ // mapped to the target. As such then, there wont be any use of the
+ // block argument %arg0 on the target.
+
mlir::Value len = v.getLen();
if (auto declareOp = val.getDefiningOp<hlfir::DeclareOp>()) {
mlir::Value base = declareOp.getBase();
>From 68993c5398c2f73e53ae2ff869dc8bcdfa89c665 Mon Sep 17 00:00:00 2001
From: Pranav Bhandarkar <pranav.bhandarkar at amd.com>
Date: Tue, 8 Apr 2025 17:47:35 -0500
Subject: [PATCH 08/10] Remove an unused variable from genBodyofTargetOp in
flang/lib/Lower/OpenMP/OpenMP.cpp
---
flang/lib/Lower/OpenMP/OpenMP.cpp | 1 -
1 file changed, 1 deletion(-)
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 97331bcbac258..35226ba1332c3 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -1413,7 +1413,6 @@ static void genBodyOfTargetOp(
auto argIface = llvm::cast<mlir::omp::BlockArgOpenMPOpInterface>(*targetOp);
mlir::Region ®ion = targetOp.getRegion();
- mlir::func::FuncOp func = targetOp->getParentOfType<mlir::func::FuncOp>();
mlir::Block *entryBlock = genEntryBlock(firOpBuilder, args, region);
bindEntryBlockArgs(converter, targetOp, args);
if (!hostEvalInfo.empty())
>From 703d7f305b9433dd6dd3280a71ef8e6dd4d7319a Mon Sep 17 00:00:00 2001
From: Pranav Bhandarkar <pranav.bhandarkar at amd.com>
Date: Tue, 8 Apr 2025 21:13:14 -0500
Subject: [PATCH 09/10] Update a comment in MapInfoFinalizationPass and also
add one more check that will advance the walk in case the MapInfoOp already
has bounds
---
flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp b/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp
index 7cb0f63ba9b69..d228507d2d771 100644
--- a/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp
+++ b/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp
@@ -550,7 +550,9 @@ class MapInfoFinalizationPass
// iterations from previous function scopes.
localBoxAllocas.clear();
- // First, walk `omp.map.info` ops to see if any
+ // 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) {
mlir::Value varPtr = op.getVarPtr();
mlir::Type underlyingVarType = fir::unwrapRefType(varPtr.getType());
@@ -562,6 +564,8 @@ class MapInfoFinalizationPass
if (!cType.hasDynamicLen())
return mlir::WalkResult::advance();
+ if (!op.getBounds().empty())
+ return mlir::WalkResult::advance();
// This means varPtr is a BlockArgument. I do not know how to get to a
// fir.boxchar<> type of mlir::Value for varPtr. So, skipping this for
// now.
>From a482741cfc925be3a3ae7562cd295e7297c096eb Mon Sep 17 00:00:00 2001
From: Pranav Bhandarkar <pranav.bhandarkar at amd.com>
Date: Fri, 25 Apr 2025 17:07:42 -0500
Subject: [PATCH 10/10] [Flang][mlir] - In fortran lowering, handle block
arguments defined outside omp target
This patch handles block arguments that are live-in into the target region of
an omp.target op. This is done by simply reusing the mapping mechanism in place
for values that are not block arguments. Further, in MapInfoFinalizationPass, this
patch adds bounds to maps that map `!fir.ref<!fir.char<k, ?>>` types.
Also, we don't clone bounds when binding entry block arguments any more.
---
.../Optimizer/Builder/DirectivesCommon.h | 31 +++++
flang/lib/Lower/OpenMP/OpenMP.cpp | 128 ++++--------------
.../Optimizer/OpenMP/MapInfoFinalization.cpp | 63 +++------
flang/test/Lower/OpenMP/map-character.f90 | 15 +-
4 files changed, 87 insertions(+), 150 deletions(-)
diff --git a/flang/include/flang/Optimizer/Builder/DirectivesCommon.h b/flang/include/flang/Optimizer/Builder/DirectivesCommon.h
index 8684299ab6792..864b8938889f8 100644
--- a/flang/include/flang/Optimizer/Builder/DirectivesCommon.h
+++ b/flang/include/flang/Optimizer/Builder/DirectivesCommon.h
@@ -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>
@@ -258,6 +285,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;
}
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 35226ba1332c3..ca6a7193fd26a 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -217,33 +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::Value val,
- 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 *definingOp = bound.getDefiningOp();
- mlir::Operation *clonedOp = firOpBuilder.clone(*definingOp);
- // Todo: Do we need to check for more operation types?
- // For now, specializing only for fir::UnboxCharOp
- if (auto unboxCharOp = mlir::dyn_cast<fir::UnboxCharOp>(definingOp))
- return clonedOp->getResult(1);
- 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())) {
@@ -251,77 +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) {
- // In some cases, v.len could reference the input to the
- // hlfir.declare which is the corresponding v.addr. While this isn't
- // a big problem by itself, it is desirable to extract this out of
- // v.addr itself since it's first result will be of type
- // fir.boxchar<>. For example, consider the following
- //
- // func.func private @_QFPrealtest(%arg0: !fir.boxchar<1>)
- // %2 = fir.dummy_scope : !fir.dscope
- // %3:2 = fir.unboxchar %arg0 : (!fir.boxchar<1>) ->
- // (!fir.ref<!fir.char<1,?>>, index)
- // %4:2 = hlfir.declare (%3#0, %3#1, %2):(!fir.ref<!fir.char<1,?>>,
- // index,!fir.dscope) ->
- // (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>)
-
- // In the case above,
- // v.addr is
- // %4:2 = hlfir.declare (%3#0, %3#1, %2):(!fir.ref<!fir.char<1,?>>,
- // index,!fir.dscope) ->
- // (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>)
- // v.len is
- // %3:2 = fir.unboxchar %arg0 : (!fir.boxchar<1>) ->
- // (!fir.ref<!fir.char<1,?>>, index)
-
- // Mapping this to the target will create a use of %arg0 on the
- // target. Since omp.target is IsolatedFromAbove, %arg0 will have to
- // be mapped. Presently, OpenMP lowering of target barfs when it has
- // to map a value that doesnt have a defining op. This can be fixed.
- // Or we ensure that v.len is fir.unboxchar %4#0 which will
- // cause %4#1 to be used on the target and consequently be
- // mapped to the target. As such then, there wont be any use of the
- // block argument %arg0 on the target.
-
- mlir::Value len = v.getLen();
- if (auto declareOp = val.getDefiningOp<hlfir::DeclareOp>()) {
- mlir::Value base = declareOp.getBase();
- if (auto boxCharType =
- mlir::dyn_cast<fir::BoxCharType>(base.getType())) {
- mlir::Type lenType = firOpBuilder.getCharacterLengthType();
- mlir::Type refType =
- firOpBuilder.getRefType(boxCharType.getEleTy());
- mlir::Location loc = converter.getCurrentLocation();
- auto unboxed = firOpBuilder.create<fir::UnboxCharOp>(
- loc, refType, lenType, base);
- len = unboxed.getResult(1);
- }
- }
- auto charBoxValue = fir::CharBoxValue(arg, cloneBound(len));
- converter.bindSymbol(sym, charBoxValue);
+ converter.bindSymbol(sym, fir::CharBoxValue(arg, v.getLen()));
},
[&](const fir::UnboxedValue &v) { converter.bindSymbol(sym, arg); },
[&](const auto &) {
@@ -333,7 +258,6 @@ static void bindEntryBlockArgs(lower::AbstractConverter &converter,
auto bindMapLike =
[&bindSingleMapLike](llvm::ArrayRef<const semantics::Symbol *> syms,
- llvm::ArrayRef<mlir::Value> vars,
llvm::ArrayRef<mlir::BlockArgument> args) {
// Structure component symbols don't have bindings, and can only be
// explicitly mapped individually. If a member is captured implicitly
@@ -342,8 +266,8 @@ static void bindEntryBlockArgs(lower::AbstractConverter &converter,
llvm::copy_if(syms, std::back_inserter(processedSyms),
[](auto *sym) { return !sym->owner().IsDerivedType(); });
- for (auto [sym, var, arg] : llvm::zip_equal(processedSyms, vars, args))
- bindSingleMapLike(*sym, var, arg);
+ for (auto [sym, arg] : llvm::zip_equal(processedSyms, args))
+ bindSingleMapLike(*sym, arg);
};
auto bindPrivateLike = [&converter, &firOpBuilder](
@@ -374,20 +298,17 @@ static void bindEntryBlockArgs(lower::AbstractConverter &converter,
// Process in clause name alphabetical order to match block arguments order.
// Do not bind host_eval variables because they cannot be used inside of the
// corresponding region, except for very specific cases handled separately.
- bindMapLike(args.hasDeviceAddr.syms, args.hasDeviceAddr.vars,
- op.getHasDeviceAddrBlockArgs());
+ bindMapLike(args.hasDeviceAddr.syms, op.getHasDeviceAddrBlockArgs());
bindPrivateLike(args.inReduction.syms, args.inReduction.vars,
op.getInReductionBlockArgs());
- bindMapLike(args.map.syms, args.map.vars, op.getMapBlockArgs());
+ bindMapLike(args.map.syms, op.getMapBlockArgs());
bindPrivateLike(args.priv.syms, args.priv.vars, op.getPrivateBlockArgs());
bindPrivateLike(args.reduction.syms, args.reduction.vars,
op.getReductionBlockArgs());
bindPrivateLike(args.taskReduction.syms, args.taskReduction.vars,
op.getTaskReductionBlockArgs());
- bindMapLike(args.useDeviceAddr.syms, args.useDeviceAddr.vars,
- op.getUseDeviceAddrBlockArgs());
- bindMapLike(args.useDevicePtr.syms, args.useDevicePtr.vars,
- op.getUseDevicePtrBlockArgs());
+ bindMapLike(args.useDeviceAddr.syms, op.getUseDeviceAddrBlockArgs());
+ bindMapLike(args.useDevicePtr.syms, op.getUseDevicePtrBlockArgs());
}
/// Get the list of base values that the specified map-like variables point to.
@@ -1427,14 +1348,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);
@@ -1447,7 +1367,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);
diff --git a/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp b/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp
index d228507d2d771..94797f173a5ef 100644
--- a/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp
+++ b/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp
@@ -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"
@@ -554,52 +556,31 @@ class MapInfoFinalizationPass
// 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) {
- mlir::Value varPtr = op.getVarPtr();
- mlir::Type underlyingVarType = fir::unwrapRefType(varPtr.getType());
- if (!mlir::isa<fir::CharacterType>(underlyingVarType))
+ if (!op.getBounds().empty())
return mlir::WalkResult::advance();
- fir::CharacterType cType =
- mlir::cast<fir::CharacterType>(underlyingVarType);
- if (!cType.hasDynamicLen())
- return mlir::WalkResult::advance();
+ mlir::Value varPtr = op.getVarPtr();
+ mlir::Type underlyingVarType = fir::unwrapRefType(varPtr.getType());
- if (!op.getBounds().empty())
- return mlir::WalkResult::advance();
- // This means varPtr is a BlockArgument. I do not know how to get to a
- // fir.boxchar<> type of mlir::Value for varPtr. So, skipping this for
- // now.
- mlir::Operation *definingOp = varPtr.getDefiningOp();
- if (!definingOp)
+ if (!fir::characterWithDynamicLen(underlyingVarType))
return mlir::WalkResult::advance();
- if (auto declOp = mlir::dyn_cast<hlfir::DeclareOp>(definingOp)) {
- mlir::Value base = declOp.getBase();
- assert(mlir::isa<fir::BoxCharType>(base.getType()));
- // mlir::value unboxChar
- builder.setInsertionPoint(op);
- fir::BoxCharType boxCharType =
- mlir::cast<fir::BoxCharType>(base.getType());
- mlir::Type idxTy = builder.getIndexType();
- mlir::Type lenType = builder.getCharacterLengthType();
- mlir::Type refType = builder.getRefType(boxCharType.getEleTy());
- mlir::Location location = op.getLoc();
- auto unboxed = builder.create<fir::UnboxCharOp>(location, refType,
- lenType, base);
- // len = unboxed.getResult(1);
- mlir::Value zero = builder.createIntegerConstant(location, idxTy, 0);
- mlir::Value one = builder.createIntegerConstant(location, idxTy, 1);
- mlir::Value extent = unboxed.getResult(1);
- mlir::Value stride = one;
- mlir::Value ub =
- builder.create<mlir::arith::SubIOp>(location, extent, one);
- mlir::Type boundTy = builder.getType<mlir::omp::MapBoundsType>();
- mlir::Value boundsOp = builder.create<mlir::omp::MapBoundsOp>(
- location, boundTy, /*lower_bound=*/zero,
- /*upper_bound=*/ub, /*extent=*/extent, /*stride=*/stride,
- /*stride_in_bytes = */ true, /*start_idx=*/zero);
- op.getBoundsMutable().append({boundsOp});
- }
+ 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);
return mlir::WalkResult::advance();
});
diff --git a/flang/test/Lower/OpenMP/map-character.f90 b/flang/test/Lower/OpenMP/map-character.f90
index 2ed2397713b5d..76df43f1c2cb3 100644
--- a/flang/test/Lower/OpenMP/map-character.f90
+++ b/flang/test/Lower/OpenMP/map-character.f90
@@ -11,14 +11,12 @@ subroutine TestOfCharacter(a0, a1, l)
end subroutine TestOfCharacter
-!CHECK: %[[A1_BOXCHAR_ALLOCA:.*]] = fir.alloca !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: fir.store %[[A0_DECL]]#0 to %[[A0_BOXCHAR_ALLOCA]] : !fir.ref<!fir.boxchar<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: fir.store %[[A1_DECL]]#0 to %[[A1_BOXCHAR_ALLOCA]] : !fir.ref<!fir.boxchar<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
@@ -33,15 +31,16 @@ end subroutine TestOfCharacter
!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: %[[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: 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:.*]], %[[A0_BOXCHAR_MAP]] -> %[[TGT_A0_BOXCHAR:.*]], %[[A1_BOXCHAR_MAP]] -> %[[TGT_A1_BOXCHAR:.*]] : !fir.ref<!fir.char<1,?>>, !fir.ref<!fir.char<1,?>>, !fir.ref<!fir.boxchar<1>>, !fir.ref<!fir.boxchar<1>>) {
-!CHECK: %[[TGT_A1_BC_LD:.*]] = fir.load %[[TGT_A1_BOXCHAR]] : !fir.ref<!fir.boxchar<1>>
+!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: %[[UNBOXED_TGT_A1:.*]]:2 = fir.unboxchar %[[TGT_A1_BC_LD]] : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index)
!CHECK: %[[TGT_A1_DECL:.*]]:2 = hlfir.declare %[[TGT_A1]] typeparams %[[UNBOXED_TGT_A1]]#1 {{.*}} -> (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>)
More information about the flang-commits
mailing list