Skip to content

Commit

Permalink
[flang][hlfir] Fixed some finalization/deallocation issues. (#67047)
Browse files Browse the repository at this point in the history
This set of commits resolves some of the issues with elemental calls producing
results that may require finalization, and also some memory leak issues due to
the missing deallocation of allocatable components of the temporary buffers
created by the bufferization pass.

- [flang][runtime] Expose Finalize API for derived types.

- [flang][hlfir] Add 'finalize' attribute for DestroyOp.

- [flang][hlfir] Postpone result finalization for elemental calls.

    The results of elemental calls generated inside hlfir.elemental must not
    be finalized/destructed before they are copied into the resulting
    array. The finalization must be done on the array as a whole
    (e.g. there might be different scalar and array finalization routines).
    The finalization work is left to the hlfir.destroy corresponding
    to this hlfir.elemental.

- [flang][hlfir] Tighten requirements on hlfir.end_associate operand.

    If component deallocation might be required for the operand of
    hlfir.end_associate, we have to be able to get the variable
    shape/params to create a descriptor for calling the runtime.
    This commit adds verification that we can do so.

- [flang][hlfir] Lower argument clean-ups using valid hlfir.end_associate.

    The operand must be a Fortran entity, when allocatable component
    deallocation may be required.

- [flang][hlfir] Properly clean-up temporary buffers in bufferization pass.

    This commit combines changes for proper finalization and component
    deallocation of the temporary buffers. The finalization part
    relates to hlfir.destroy operations with 'finalize' attribute.
    The component deallocation might be invoked for both hlfir.destroy
    and hlfir.end_associate, if the operand is of a derived type
    with allocatable component(s).

The changes are mostly in one function, so I decided not to split them.

- [flang][hlfir] Disable optimizations for hlfir.elemental requiring finalization.

    If hlfir.elemental is coupled with hlfir.destroy with 'finalize' attribute,
    the temporary array result of hlfir.elemental needs to be created
    for the purpose of finalization. We cannot do certain optimizations
    on such hlfir.elemental operations.

    I was not able to come up with a test for the OptimizedBufferization pass,
    but I put the check there as well.
  • Loading branch information
vzakhari committed Sep 22, 2023
1 parent b38f31a commit ab1db26
Show file tree
Hide file tree
Showing 29 changed files with 680 additions and 69 deletions.
4 changes: 3 additions & 1 deletion flang/include/flang/Lower/ConvertCall.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,13 @@ namespace Fortran::lower {
/// the call and return the result. This function deals with explicit result
/// allocation and lowering if needed. It also deals with passing the host
/// link to internal procedures.
/// \p isElemental must be set to true if elemental call is being produced.
/// It is only used for HLFIR.
fir::ExtendedValue genCallOpAndResult(
mlir::Location loc, Fortran::lower::AbstractConverter &converter,
Fortran::lower::SymMap &symMap, Fortran::lower::StatementContext &stmtCtx,
Fortran::lower::CallerInterface &caller, mlir::FunctionType callSiteType,
std::optional<mlir::Type> resultType);
std::optional<mlir::Type> resultType, bool isElemental = false);

/// If \p arg is the address of a function with a denoted host-association tuple
/// argument, then return the host-associations tuple value of the current
Expand Down
38 changes: 7 additions & 31 deletions flang/include/flang/Optimizer/Builder/HLFIRTools.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,37 +35,6 @@ class ElementalOpInterface;
class ElementalAddrOp;
class YieldElementOp;

/// Is this an SSA value type for the value of a Fortran procedure
/// designator ?
inline bool isFortranProcedureValue(mlir::Type type) {
return type.isa<fir::BoxProcType>() ||
(type.isa<mlir::TupleType>() &&
fir::isCharacterProcedureTuple(type, /*acceptRawFunc=*/false));
}

/// Is this an SSA value type for the value of a Fortran expression?
inline bool isFortranValueType(mlir::Type type) {
return type.isa<hlfir::ExprType>() || fir::isa_trivial(type) ||
isFortranProcedureValue(type);
}

/// Is this the value of a Fortran expression in an SSA value form?
inline bool isFortranValue(mlir::Value value) {
return isFortranValueType(value.getType());
}

/// Is this a Fortran variable?
/// Note that by "variable", it must be understood that the mlir::Value is
/// a memory value of a storage that can be reason about as a Fortran object
/// (its bounds, shape, and type parameters, if any, are retrievable).
/// This does not imply that the mlir::Value points to a variable from the
/// original source or can be legally defined: temporaries created to store
/// expression values are considered to be variables, and so are PARAMETERs
/// global constant address.
inline bool isFortranEntity(mlir::Value value) {
return isFortranValue(value) || isFortranVariableType(value.getType());
}

/// Is this a Fortran variable for which the defining op carrying the Fortran
/// attributes is visible?
inline bool isFortranVariableWithAttributes(mlir::Value value) {
Expand Down Expand Up @@ -442,6 +411,13 @@ hlfir::ElementalOp cloneToElementalOp(mlir::Location loc,
fir::FirOpBuilder &builder,
hlfir::ElementalAddrOp elementalAddrOp);

/// Return true, if \p elemental must produce a temporary array,
/// for example, for the purpose of finalization. Note that such
/// ElementalOp's must be optimized with caution. For example,
/// completely inlining such ElementalOp into another one
/// would be incorrect.
bool elementalOpMustProduceTemp(hlfir::ElementalOp elemental);

} // namespace hlfir

#endif // FORTRAN_OPTIMIZER_BUILDER_HLFIRTOOLS_H
5 changes: 5 additions & 0 deletions flang/include/flang/Optimizer/Builder/Runtime/Derived.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ void genDerivedTypeInitialize(fir::FirOpBuilder &builder, mlir::Location loc,
void genDerivedTypeDestroy(fir::FirOpBuilder &builder, mlir::Location loc,
mlir::Value box);

/// Generate call to derived type finalization runtime routine
/// to finalize \p box.
void genDerivedTypeFinalize(fir::FirOpBuilder &builder, mlir::Location loc,
mlir::Value box);

/// Generate call to derived type destruction runtime routine to
/// destroy \p box without finalization
void genDerivedTypeDestroyWithoutFinalization(fir::FirOpBuilder &builder,
Expand Down
38 changes: 38 additions & 0 deletions flang/include/flang/Optimizer/HLFIR/HLFIRDialect.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,37 @@ inline bool isPolymorphicType(mlir::Type type) {
return fir::isPolymorphicType(type);
}

/// Is this an SSA value type for the value of a Fortran procedure
/// designator ?
inline bool isFortranProcedureValue(mlir::Type type) {
return type.isa<fir::BoxProcType>() ||
(type.isa<mlir::TupleType>() &&
fir::isCharacterProcedureTuple(type, /*acceptRawFunc=*/false));
}

/// Is this an SSA value type for the value of a Fortran expression?
inline bool isFortranValueType(mlir::Type type) {
return type.isa<hlfir::ExprType>() || fir::isa_trivial(type) ||
isFortranProcedureValue(type);
}

/// Is this the value of a Fortran expression in an SSA value form?
inline bool isFortranValue(mlir::Value value) {
return isFortranValueType(value.getType());
}

/// Is this a Fortran variable?
/// Note that by "variable", it must be understood that the mlir::Value is
/// a memory value of a storage that can be reason about as a Fortran object
/// (its bounds, shape, and type parameters, if any, are retrievable).
/// This does not imply that the mlir::Value points to a variable from the
/// original source or can be legally defined: temporaries created to store
/// expression values are considered to be variables, and so are PARAMETERs
/// global constant address.
inline bool isFortranEntity(mlir::Value value) {
return isFortranValue(value) || isFortranVariableType(value.getType());
}

bool isFortranScalarNumericalType(mlir::Type);
bool isFortranNumericalArrayObject(mlir::Type);
bool isFortranNumericalOrLogicalArrayObject(mlir::Type);
Expand All @@ -94,6 +125,13 @@ bool isPolymorphicObject(mlir::Type);
mlir::Value genExprShape(mlir::OpBuilder &builder, const mlir::Location &loc,
const hlfir::ExprType &expr);

/// Return true iff `ty` may have allocatable component.
/// TODO: this actually belongs to FIRType.cpp, but the method's implementation
/// depends on HLFIRDialect component. FIRType.cpp itself is part of FIRDialect
/// that cannot depend on HLFIRBuilder (there will be a cyclic dependency).
/// This has to be cleaned up, when HLFIR is the default.
bool mayHaveAllocatableComponent(mlir::Type ty);

} // namespace hlfir

#endif // FORTRAN_OPTIMIZER_HLFIR_HLFIRDIALECT_H
25 changes: 23 additions & 2 deletions flang/include/flang/Optimizer/HLFIR/HLFIROps.td
Original file line number Diff line number Diff line change
Expand Up @@ -705,6 +705,8 @@ def hlfir_EndAssociateOp : hlfir_Op<"end_associate", [MemoryEffects<[MemFree]>]>

let description = [{
Mark the end of life of a variable associated to an expression.
If the expression has a derived type that may contain allocatable
components, the variable operand must be a Fortran entity.
}];

let arguments = (ins AnyRefOrBoxLike:$var,
Expand All @@ -715,6 +717,7 @@ def hlfir_EndAssociateOp : hlfir_Op<"end_associate", [MemoryEffects<[MemFree]>]>
}];

let builders = [OpBuilder<(ins "hlfir::AssociateOp":$associate)>];
let hasVerifier = 1;
}

def hlfir_AsExprOp : hlfir_Op<"as_expr",
Expand Down Expand Up @@ -981,6 +984,11 @@ def hlfir_DestroyOp : hlfir_Op<"destroy", [MemoryEffects<[MemFree]>]> {
Mark the last use of an hlfir.expr. This will be the point at which the
buffer of an hlfir.expr, if any, will be deallocated if it was heap
allocated.
If "finalize" attribute is set, the hlfir.expr value will be finalized
before the deallocation. Note that this implies that the hlfir.expr
is placed into a memory buffer, so that the library runtime
can be called on it. The element type of the hlfir.expr must be
derived type in this case.
It is not required to create an hlfir.destroy operation for and hlfir.expr
created inside an hlfir.elemental and returned in the hlfir.yield_element.
The last use of such expression is implicit and an hlfir.destroy could
Expand All @@ -995,9 +1003,22 @@ def hlfir_DestroyOp : hlfir_Op<"destroy", [MemoryEffects<[MemFree]>]> {
in bufferization instead.
}];

let arguments = (ins hlfir_ExprType:$expr);
let arguments = (ins
hlfir_ExprType:$expr,
UnitAttr:$finalize
);

let assemblyFormat = [{
$expr (`finalize` $finalize^)? attr-dict `:` qualified(type($expr))
}];

let extraClassDeclaration = [{
bool mustFinalizeExpr() {
return getFinalize();
}
}];

let assemblyFormat = "$expr attr-dict `:` qualified(type($expr))";
let hasVerifier = 1;
}

def hlfir_CopyInOp : hlfir_Op<"copy_in", [MemoryEffects<[MemAlloc]>]> {
Expand Down
4 changes: 4 additions & 0 deletions flang/include/flang/Runtime/derived-api.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ void RTNAME(Initialize)(
// storage.
void RTNAME(Destroy)(const Descriptor &);

// Finalizes the object and its components.
void RTNAME(Finalize)(
const Descriptor &, const char *sourceFile = nullptr, int sourceLine = 0);

/// Deallocates any allocatable/automatic components.
/// Does not deallocate the descriptor's storage.
/// Does not perform any finalization.
Expand Down
64 changes: 55 additions & 9 deletions flang/lib/Lower/ConvertCall.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ fir::ExtendedValue Fortran::lower::genCallOpAndResult(
mlir::Location loc, Fortran::lower::AbstractConverter &converter,
Fortran::lower::SymMap &symMap, Fortran::lower::StatementContext &stmtCtx,
Fortran::lower::CallerInterface &caller, mlir::FunctionType callSiteType,
std::optional<mlir::Type> resultType) {
std::optional<mlir::Type> resultType, bool isElemental) {
fir::FirOpBuilder &builder = converter.getFirOpBuilder();
using PassBy = Fortran::lower::CallerInterface::PassEntityBy;
// Handle cases where caller must allocate the result or a fir.box for it.
Expand Down Expand Up @@ -435,7 +435,13 @@ fir::ExtendedValue Fortran::lower::genCallOpAndResult(
std::optional<Fortran::evaluate::DynamicType> retTy =
caller.getCallDescription().proc().GetType();
bool cleanupWithDestroy = false;
if (!fir::isPointerType(funcType.getResults()[0]) && retTy &&
// With HLFIR lowering, isElemental must be set to true
// if we are producing an elemental call. In this case,
// the elemental results must not be destroyed, instead,
// the resulting array result will be finalized/destroyed
// as needed by hlfir.destroy.
if (!isElemental && !fir::isPointerType(funcType.getResults()[0]) &&
retTy &&
(retTy->category() == Fortran::common::TypeCategory::Derived ||
retTy->IsPolymorphic() || retTy->IsUnlimitedPolymorphic())) {
if (retTy->IsPolymorphic() || retTy->IsUnlimitedPolymorphic()) {
Expand Down Expand Up @@ -692,6 +698,14 @@ struct PreparedDummyArgument {
cleanups.emplace_back(
CallCleanUp{CallCleanUp::ExprAssociate{tempVar, wasCopied}});
}
void pushExprAssociateCleanUp(hlfir::AssociateOp associate) {
mlir::Value hlfirBase = associate.getBase();
mlir::Value firBase = associate.getFirBase();
cleanups.emplace_back(CallCleanUp{CallCleanUp::ExprAssociate{
hlfir::mayHaveAllocatableComponent(hlfirBase.getType()) ? hlfirBase
: firBase,
associate.getMustFreeStrorageFlag()}});
}

mlir::Value dummy;
// NOTE: the clean-ups are executed in reverse order.
Expand Down Expand Up @@ -896,8 +910,7 @@ static PreparedDummyArgument preparePresentUserCallActualArgument(
loc, builder, hlfir::Entity{copy}, storageType, "adapt.valuebyref");
entity = hlfir::Entity{associate.getBase()};
// Register the temporary destruction after the call.
preparedDummy.pushExprAssociateCleanUp(
associate.getFirBase(), associate.getMustFreeStrorageFlag());
preparedDummy.pushExprAssociateCleanUp(associate);
} else if (mustDoCopyInOut) {
// Copy-in non contiguous variables.
assert(entity.getType().isa<fir::BaseBoxType>() &&
Expand All @@ -924,8 +937,7 @@ static PreparedDummyArgument preparePresentUserCallActualArgument(
hlfir::AssociateOp associate = hlfir::genAssociateExpr(
loc, builder, entity, storageType, "adapt.valuebyref");
entity = hlfir::Entity{associate.getBase()};
preparedDummy.pushExprAssociateCleanUp(associate.getFirBase(),
associate.getMustFreeStrorageFlag());
preparedDummy.pushExprAssociateCleanUp(associate);
if (mustSetDynamicTypeToDummyType) {
// Rebox the actual argument to the dummy argument's type, and make
// sure that we pass a contiguous entity (i.e. make copy-in,
Expand Down Expand Up @@ -1201,7 +1213,8 @@ genUserCall(Fortran::lower::PreparedActualArguments &loweredActuals,
// arguments.
fir::ExtendedValue result = Fortran::lower::genCallOpAndResult(
loc, callContext.converter, callContext.symMap, callContext.stmtCtx,
caller, callSiteType, callContext.resultType);
caller, callSiteType, callContext.resultType,
callContext.isElementalProcWithArrayArgs());

/// Clean-up associations and copy-in.
for (auto cleanUp : callCleanUps)
Expand Down Expand Up @@ -1687,9 +1700,14 @@ class ElementalCallBuilder {
mlir::Value elemental =
hlfir::genElementalOp(loc, builder, elementType, shape, typeParams,
genKernel, !mustBeOrdered, polymorphicMold);
// If the function result requires finalization, then it has to be done
// for the array result of the elemental call. We have to communicate
// this via the DestroyOp's attribute.
bool mustFinalizeExpr = impl().resultMayRequireFinalization(callContext);
fir::FirOpBuilder *bldr = &builder;
callContext.stmtCtx.attachCleanup(
[=]() { bldr->create<hlfir::DestroyOp>(loc, elemental); });
callContext.stmtCtx.attachCleanup([=]() {
bldr->create<hlfir::DestroyOp>(loc, elemental, mustFinalizeExpr);
});
return hlfir::EntityWithAttributes{elemental};
}

Expand Down Expand Up @@ -1743,6 +1761,26 @@ class ElementalUserCallBuilder
return {};
}

bool resultMayRequireFinalization(CallContext &callContext) const {
std::optional<Fortran::evaluate::DynamicType> retTy =
caller.getCallDescription().proc().GetType();
if (!retTy)
return false;

if (retTy->IsPolymorphic() || retTy->IsUnlimitedPolymorphic())
fir::emitFatalError(
callContext.loc,
"elemental function call with [unlimited-]polymorphic result");

if (retTy->category() == Fortran::common::TypeCategory::Derived) {
const Fortran::semantics::DerivedTypeSpec &typeSpec =
retTy->GetDerivedTypeSpec();
return Fortran::semantics::IsFinalizable(typeSpec);
}

return false;
}

private:
Fortran::lower::CallerInterface &caller;
mlir::FunctionType callSiteType;
Expand Down Expand Up @@ -1804,6 +1842,14 @@ class ElementalIntrinsicCallBuilder
return {};
}

bool resultMayRequireFinalization(
[[maybe_unused]] CallContext &callContext) const {
// FIXME: need access to the CallerInterface's return type
// to check if the result may need finalization (e.g. the result
// of MERGE).
return false;
}

private:
const Fortran::evaluate::SpecificIntrinsic *intrinsic;
const fir::IntrinsicArgumentLoweringRules *argLowering;
Expand Down
4 changes: 4 additions & 0 deletions flang/lib/Lower/HlfirIntrinsics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,9 @@ std::optional<hlfir::EntityWithAttributes> Fortran::lower::lowerHlfirIntrinsic(
const Fortran::lower::PreparedActualArguments &loweredActuals,
const fir::IntrinsicArgumentLoweringRules *argLowering,
mlir::Type stmtResultType) {
// If the result is of a derived type that may need finalization,
// we have to use DestroyOp with 'finalize' attribute for the result
// of the intrinsic operation.
if (name == "sum")
return HlfirSumLowering{builder, loc}.lower(loweredActuals, argLowering,
stmtResultType);
Expand All @@ -348,6 +351,7 @@ std::optional<hlfir::EntityWithAttributes> Fortran::lower::lowerHlfirIntrinsic(
if (name == "dot_product")
return HlfirDotProductLowering{builder, loc}.lower(
loweredActuals, argLowering, stmtResultType);
// FIXME: the result may need finalization.
if (name == "transpose")
return HlfirTransposeLowering{builder, loc}.lower(
loweredActuals, argLowering, stmtResultType);
Expand Down
9 changes: 9 additions & 0 deletions flang/lib/Optimizer/Builder/HLFIRTools.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1021,3 +1021,12 @@ hlfir::cloneToElementalOp(mlir::Location loc, fir::FirOpBuilder &builder,
elementalAddrOp.getShape(), typeParams,
genKernel, !elementalAddrOp.isOrdered());
}

bool hlfir::elementalOpMustProduceTemp(hlfir::ElementalOp elemental) {
for (mlir::Operation *useOp : elemental->getUsers())
if (auto destroy = mlir::dyn_cast<hlfir::DestroyOp>(useOp))
if (destroy.mustFinalizeExpr())
return true;

return false;
}
12 changes: 12 additions & 0 deletions flang/lib/Optimizer/Builder/Runtime/Derived.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,18 @@ void fir::runtime::genDerivedTypeDestroy(fir::FirOpBuilder &builder,
builder.create<fir::CallOp>(loc, func, args);
}

void fir::runtime::genDerivedTypeFinalize(fir::FirOpBuilder &builder,
mlir::Location loc, mlir::Value box) {
auto func = fir::runtime::getRuntimeFunc<mkRTKey(Finalize)>(loc, builder);
auto fTy = func.getFunctionType();
auto sourceFile = fir::factory::locationToFilename(builder, loc);
auto sourceLine =
fir::factory::locationToLineNo(builder, loc, fTy.getInput(2));
auto args = fir::runtime::createArguments(builder, loc, fTy, box, sourceFile,
sourceLine);
builder.create<fir::CallOp>(loc, func, args);
}

void fir::runtime::genDerivedTypeDestroyWithoutFinalization(
fir::FirOpBuilder &builder, mlir::Location loc, mlir::Value box) {
auto func = fir::runtime::getRuntimeFunc<mkRTKey(DestroyWithoutFinalization)>(
Expand Down
5 changes: 5 additions & 0 deletions flang/lib/Optimizer/HLFIR/IR/HLFIRDialect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -207,3 +207,8 @@ mlir::Value hlfir::genExprShape(mlir::OpBuilder &builder,
fir::ShapeOp shape = builder.create<fir::ShapeOp>(loc, shapeTy, extents);
return shape.getResult();
}

bool hlfir::mayHaveAllocatableComponent(mlir::Type ty) {
return fir::isPolymorphicType(ty) || fir::isUnlimitedPolymorphicType(ty) ||
fir::isRecordWithAllocatableMember(hlfir::getFortranElementType(ty));
}

0 comments on commit ab1db26

Please sign in to comment.