Skip to content

Commit

Permalink
[flang] Lower elemental and transformational clean-up in HLFIR
Browse files Browse the repository at this point in the history 
In lowering to hlfir, no clean-up was added yet for
the created hlfir.elemental. Add  the needed hlfir.destroy.

Regarding transformational lowering, clean-ups were created because
they are lowered in memory, but this is inconvenient because this
prevented lowering to hlfir from "moving" the created variable to
an expression. Add a new entry point in IntrinsicCall.h that keeps
track of whether or not the returned storage needs to be deallocated,
but does not insert the deallocation in the StatementContext.
This allows using the newly added hlfir.as_expr "move" aspect to be
used and save creating a copy.

Depends on D141839

Reviewed By: clementval

Differential Revision: https://reviews.llvm.org/D141841
  • Loading branch information
@jeanPerier
jeanPerier committed Jan 17, 2023
1 parent 6c37fbd commit c0b45fe
Show file tree
Hide file tree
Showing 11 changed files with 176 additions and 69 deletions.
12 changes: 8 additions & 4 deletions flang/include/flang/Lower/IntrinsicCall.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,6 @@
#include "flang/Optimizer/Builder/FIRBuilder.h"
#include <optional>

namespace fir {
class ExtendedValue;
}

namespace Fortran::lower {

class StatementContext;
Expand All @@ -32,6 +28,14 @@ fir::ExtendedValue genIntrinsicCall(fir::FirOpBuilder &, mlir::Location,
llvm::ArrayRef<fir::ExtendedValue> args,
StatementContext &);

/// Same as the other genIntrinsicCall version above, except that the result
/// deallocation, if required, is not added to a StatementContext. Instead, an
/// extra boolean result indicates if the result must be freed after use.
std::pair<fir::ExtendedValue, bool>
genIntrinsicCall(fir::FirOpBuilder &, mlir::Location, llvm::StringRef name,
std::optional<mlir::Type> resultType,
llvm::ArrayRef<fir::ExtendedValue> args);

/// Enum specifying how intrinsic argument evaluate::Expr should be
/// lowered to fir::ExtendedValue to be passed to genIntrinsicCall.
enum class LowerIntrinsicArgAs {
Expand Down
57 changes: 41 additions & 16 deletions flang/lib/Lower/ConvertCall.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -663,7 +663,7 @@ static hlfir::EntityWithAttributes genIntrinsicRefCore(
PreparedActualArguments &loweredActuals,
const Fortran::evaluate::SpecificIntrinsic &intrinsic,
const Fortran::lower::IntrinsicArgumentLoweringRules *argLowering,
std::optional<mlir::Type> coreResultType, CallContext &callContext) {
CallContext &callContext) {
llvm::SmallVector<fir::ExtendedValue> operands;
auto &stmtCtx = callContext.stmtCtx;
auto &converter = callContext.converter;
Expand Down Expand Up @@ -710,12 +710,27 @@ static hlfir::EntityWithAttributes genIntrinsicRefCore(
}
llvm_unreachable("bad switch");
}
fir::FirOpBuilder &builder = callContext.getBuilder();
// genIntrinsicCall needs the scalar type, even if this is a transformational
// procedure returning an array.
std::optional<mlir::Type> scalarResultType;
if (callContext.resultType)
scalarResultType = hlfir::getFortranElementType(*callContext.resultType);
// Let the intrinsic library lower the intrinsic procedure call.
fir::ExtendedValue val = Fortran::lower::genIntrinsicCall(
callContext.getBuilder(), loc, intrinsic.name, coreResultType, operands,
stmtCtx);
return extendedValueToHlfirEntity(loc, callContext.getBuilder(), val,
".tmp.intrinsic_result");
auto [resultExv, mustBeFreed] = Fortran::lower::genIntrinsicCall(
callContext.getBuilder(), loc, intrinsic.name, scalarResultType,
operands);
hlfir::EntityWithAttributes resultEntity = extendedValueToHlfirEntity(
loc, builder, resultExv, ".tmp.intrinsic_result");
// Move result into memory into an hlfir.expr since they are immutable from
// that point, and the result storage is some temp.
if (!fir::isa_trivial(resultEntity.getType()))
resultEntity = hlfir::EntityWithAttributes{
builder
.create<hlfir::AsExprOp>(loc, resultEntity,
builder.createBool(loc, mustBeFreed))
.getResult()};
return resultEntity;
}

namespace {
Expand Down Expand Up @@ -763,13 +778,13 @@ class ElementalCallBuilder {
TODO(loc, "ordered elemental calls in HLFIR");
// Push a new local scope so that any temps made inside the elemental
// iterations are cleaned up inside the iterations.
callContext.stmtCtx.pushScope();
if (!callContext.resultType) {
// Subroutine case. Generate call inside loop nest.
auto [innerLoop, oneBasedIndices] =
hlfir::genLoopNest(loc, builder, shape);
auto insPt = builder.saveInsertionPoint();
builder.setInsertionPointToStart(innerLoop.getBody());
callContext.stmtCtx.pushScope();
for (auto &preparedActual : loweredActuals)
if (preparedActual)
preparedActual->actual = hlfir::getElementAt(
Expand All @@ -789,17 +804,24 @@ class ElementalCallBuilder {
TODO(loc, "compute elemental function result length parameters in HLFIR");
auto genKernel = [&](mlir::Location l, fir::FirOpBuilder &b,
mlir::ValueRange oneBasedIndices) -> hlfir::Entity {
callContext.stmtCtx.pushScope();
for (auto &preparedActual : loweredActuals)
if (preparedActual)
preparedActual->actual = hlfir::getElementAt(
l, b, preparedActual->actual, oneBasedIndices);
auto res = *impl().genElementalKernel(loweredActuals, callContext);
callContext.stmtCtx.finalizeAndPop();
// Note that an hlfir.destroy is not emitted for the result since it
// is still used by the hlfir.yield_element that also marks its last
// use.
return res;
};
// TODO: deal with hlfir.elemental result destruction.
return hlfir::EntityWithAttributes{hlfir::genElementalOp(
loc, builder, elementType, shape, typeParams, genKernel)};
mlir::Value elemental = hlfir::genElementalOp(loc, builder, elementType,
shape, typeParams, genKernel);
fir::FirOpBuilder *bldr = &builder;
callContext.stmtCtx.attachCleanup(
[=]() { bldr->create<hlfir::DestroyOp>(loc, elemental); });
return hlfir::EntityWithAttributes{elemental};
}

private:
Expand Down Expand Up @@ -853,11 +875,8 @@ class ElementalIntrinsicCallBuilder
std::optional<hlfir::Entity>
genElementalKernel(PreparedActualArguments &loweredActuals,
CallContext &callContext) {
std::optional<mlir::Type> coreResultType;
if (callContext.resultType.has_value())
coreResultType = hlfir::getFortranElementType(*callContext.resultType);
return genIntrinsicRefCore(loweredActuals, intrinsic, argLowering,
coreResultType, callContext);
callContext);
}
// Elemental intrinsic functions cannot modify their arguments.
bool argMayBeModifiedByCall(int) const { return !isFunction; }
Expand Down Expand Up @@ -917,8 +936,14 @@ genIntrinsicRef(const Fortran::evaluate::SpecificIntrinsic &intrinsic,
.genElementalCall(loweredActuals, /*isImpure=*/!isFunction, callContext)
.value();
}
return genIntrinsicRefCore(loweredActuals, intrinsic, argLowering,
callContext.resultType, callContext);
hlfir::EntityWithAttributes result =
genIntrinsicRefCore(loweredActuals, intrinsic, argLowering, callContext);
if (result.getType().isa<hlfir::ExprType>()) {
fir::FirOpBuilder *bldr = &callContext.getBuilder();
callContext.stmtCtx.attachCleanup(
[=]() { bldr->create<hlfir::DestroyOp>(loc, result); });
}
return result;
}

/// Main entry point to lower procedure references, regardless of what they are.
Expand Down
18 changes: 12 additions & 6 deletions flang/lib/Lower/ConvertExprToHLFIR.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1060,9 +1060,12 @@ class HlfirBuilder {
auto leftVal = hlfir::loadTrivialScalar(l, b, leftElement);
return unaryOp.gen(l, b, op.derived(), leftVal);
};
// TODO: deal with hlfir.elemental result destruction.
return hlfir::EntityWithAttributes{hlfir::genElementalOp(
loc, builder, elementType, shape, typeParams, genKernel)};
mlir::Value elemental = hlfir::genElementalOp(loc, builder, elementType,
shape, typeParams, genKernel);
fir::FirOpBuilder *bldr = &builder;
getStmtCtx().attachCleanup(
[=]() { bldr->create<hlfir::DestroyOp>(loc, elemental); });
return hlfir::EntityWithAttributes{elemental};
}

template <typename D, typename R, typename LO, typename RO>
Expand Down Expand Up @@ -1102,9 +1105,12 @@ class HlfirBuilder {
auto rightVal = hlfir::loadTrivialScalar(l, b, rightElement);
return binaryOp.gen(l, b, op.derived(), leftVal, rightVal);
};
// TODO: deal with hlfir.elemental result destruction.
return hlfir::EntityWithAttributes{hlfir::genElementalOp(
loc, builder, elementType, shape, typeParams, genKernel)};
mlir::Value elemental = hlfir::genElementalOp(loc, builder, elementType,
shape, typeParams, genKernel);
fir::FirOpBuilder *bldr = &builder;
getStmtCtx().attachCleanup(
[=]() { bldr->create<hlfir::DestroyOp>(loc, elemental); });
return hlfir::EntityWithAttributes{elemental};
}

hlfir::EntityWithAttributes
Expand Down
88 changes: 48 additions & 40 deletions flang/lib/Lower/IntrinsicCall.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,17 +132,17 @@ static bool isStaticallyPresent(const fir::ExtendedValue &exv) {
struct IntrinsicLibrary {

// Constructors.
explicit IntrinsicLibrary(fir::FirOpBuilder &builder, mlir::Location loc,
Fortran::lower::StatementContext *stmtCtx = nullptr)
: builder{builder}, loc{loc}, stmtCtx{stmtCtx} {}
explicit IntrinsicLibrary(fir::FirOpBuilder &builder, mlir::Location loc)
: builder{builder}, loc{loc} {}
IntrinsicLibrary() = delete;
IntrinsicLibrary(const IntrinsicLibrary &) = delete;

/// Generate FIR for call to Fortran intrinsic \p name with arguments \p arg
/// and expected result type \p resultType.
fir::ExtendedValue genIntrinsicCall(llvm::StringRef name,
std::optional<mlir::Type> resultType,
llvm::ArrayRef<fir::ExtendedValue> arg);
/// and expected result type \p resultType. Return the result and a boolean
/// that, if true, indicates that the result must be freed after use.
std::pair<fir::ExtendedValue, bool>
genIntrinsicCall(llvm::StringRef name, std::optional<mlir::Type> resultType,
llvm::ArrayRef<fir::ExtendedValue> arg);

/// Search a runtime function that is associated to the generic intrinsic name
/// and whose signature matches the intrinsic arguments and result types.
Expand Down Expand Up @@ -394,16 +394,16 @@ struct IntrinsicLibrary {
getUnrestrictedIntrinsicSymbolRefAttr(llvm::StringRef name,
mlir::FunctionType signature);

/// Add clean-up for \p temp to the current statement context;
void addCleanUpForTemp(mlir::Location loc, mlir::Value temp);
/// Helper function for generating code clean-up for result descriptors
fir::ExtendedValue readAndAddCleanUp(fir::MutableBoxValue resultMutableBox,
mlir::Type resultType,
llvm::StringRef errMsg);

void setResultMustBeFreed() { resultMustBeFreed = true; }

fir::FirOpBuilder &builder;
mlir::Location loc;
Fortran::lower::StatementContext *stmtCtx;
bool resultMustBeFreed = false;
};

struct IntrinsicDummyArgument {
Expand Down Expand Up @@ -1719,19 +1719,20 @@ invokeHandler(IntrinsicLibrary::SubroutineGenerator generator,
return mlir::Value{};
}

fir::ExtendedValue
std::pair<fir::ExtendedValue, bool>
IntrinsicLibrary::genIntrinsicCall(llvm::StringRef specificName,
std::optional<mlir::Type> resultType,
llvm::ArrayRef<fir::ExtendedValue> args) {
llvm::StringRef name = genericName(specificName);
if (const IntrinsicHandler *handler = findIntrinsicHandler(name)) {
bool outline = handler->outline || outlineAllIntrinsics;
return std::visit(
[&](auto &generator) -> fir::ExtendedValue {
return invokeHandler(generator, *handler, resultType, args, outline,
*this);
},
handler->generator);
return {std::visit(
[&](auto &generator) -> fir::ExtendedValue {
return invokeHandler(generator, *handler, resultType, args,
outline, *this);
},
handler->generator),
this->resultMustBeFreed};
}

if (!resultType)
Expand All @@ -1758,8 +1759,9 @@ IntrinsicLibrary::genIntrinsicCall(llvm::StringRef specificName,

IntrinsicLibrary::RuntimeCallGenerator runtimeCallGenerator =
getRuntimeCallGenerator(name, soughtFuncType);
return genElementalCall(runtimeCallGenerator, name, *resultType, args,
/*outline=*/outlineAllIntrinsics);
return {genElementalCall(runtimeCallGenerator, name, *resultType, args,
/*outline=*/outlineAllIntrinsics),
resultMustBeFreed};
}

mlir::Value
Expand Down Expand Up @@ -1987,12 +1989,6 @@ mlir::SymbolRefAttr IntrinsicLibrary::getUnrestrictedIntrinsicSymbolRefAttr(
return mlir::SymbolRefAttr::get(funcOp);
}

void IntrinsicLibrary::addCleanUpForTemp(mlir::Location loc, mlir::Value temp) {
assert(stmtCtx);
fir::FirOpBuilder *bldr = &builder;
stmtCtx->attachCleanup([=]() { bldr->create<fir::FreeMemOp>(loc, temp); });
}

fir::ExtendedValue
IntrinsicLibrary::readAndAddCleanUp(fir::MutableBoxValue resultMutableBox,
mlir::Type resultType,
Expand All @@ -2001,30 +1997,25 @@ IntrinsicLibrary::readAndAddCleanUp(fir::MutableBoxValue resultMutableBox,
fir::factory::genMutableBoxRead(builder, loc, resultMutableBox);
return res.match(
[&](const fir::ArrayBoxValue &box) -> fir::ExtendedValue {
// Add cleanup code
addCleanUpForTemp(loc, box.getAddr());
setResultMustBeFreed();
return box;
},
[&](const fir::BoxValue &box) -> fir::ExtendedValue {
// Add cleanup code
auto addr =
builder.create<fir::BoxAddrOp>(loc, box.getMemTy(), box.getAddr());
addCleanUpForTemp(loc, addr);
setResultMustBeFreed();
return box;
},
[&](const fir::CharArrayBoxValue &box) -> fir::ExtendedValue {
// Add cleanup code
addCleanUpForTemp(loc, box.getAddr());
setResultMustBeFreed();
return box;
},
[&](const mlir::Value &tempAddr) -> fir::ExtendedValue {
// Add cleanup code
addCleanUpForTemp(loc, tempAddr);
return builder.create<fir::LoadOp>(loc, resultType, tempAddr);
auto load = builder.create<fir::LoadOp>(loc, resultType, tempAddr);
// Temp can be freed right away since it was loaded.
builder.create<fir::FreeMemOp>(loc, tempAddr);
return load;
},
[&](const fir::CharBoxValue &box) -> fir::ExtendedValue {
// Add cleanup code
addCleanUpForTemp(loc, box.getAddr());
setResultMustBeFreed();
return box;
},
[&](const auto &) -> fir::ExtendedValue {
Expand Down Expand Up @@ -5216,8 +5207,25 @@ Fortran::lower::genIntrinsicCall(fir::FirOpBuilder &builder, mlir::Location loc,
std::optional<mlir::Type> resultType,
llvm::ArrayRef<fir::ExtendedValue> args,
Fortran::lower::StatementContext &stmtCtx) {
return IntrinsicLibrary{builder, loc, &stmtCtx}.genIntrinsicCall(
name, resultType, args);
auto [result, mustBeFreed] =
IntrinsicLibrary{builder, loc}.genIntrinsicCall(name, resultType, args);
if (mustBeFreed) {
mlir::Value addr = fir::getBase(result);
if (auto *box = result.getBoxOf<fir::BoxValue>())
addr =
builder.create<fir::BoxAddrOp>(loc, box->getMemTy(), box->getAddr());
fir::FirOpBuilder *bldr = &builder;
stmtCtx.attachCleanup([=]() { bldr->create<fir::FreeMemOp>(loc, addr); });
}
return result;
}
std::pair<fir::ExtendedValue, bool>
Fortran::lower::genIntrinsicCall(fir::FirOpBuilder &builder, mlir::Location loc,
llvm::StringRef name,
std::optional<mlir::Type> resultType,
llvm::ArrayRef<fir::ExtendedValue> args) {
return IntrinsicLibrary{builder, loc}.genIntrinsicCall(name, resultType,
args);
}

mlir::Value Fortran::lower::genMax(fir::FirOpBuilder &builder,
Expand Down
2 changes: 1 addition & 1 deletion flang/test/HLFIR/destroy-codegen.fir
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Test hlfir.destroy code generation and hlfir.yield_element "implicit
// Test hlfir.destroy code generation and hlfir.yield_element "implicit
// hlfir.destroy" aspect.

// RUN: fir-opt %s -bufferize-hlfir | FileCheck %s
Expand Down
Loading

0 comments on commit c0b45fe

Please sign in to comment.