diff --git a/flang/include/flang/Optimizer/OpenACC/Support/FIROpenACCTypeInterfaces.h b/flang/include/flang/Optimizer/OpenACC/Support/FIROpenACCTypeInterfaces.h index 10853933317f7..408f0392e4271 100644 --- a/flang/include/flang/Optimizer/OpenACC/Support/FIROpenACCTypeInterfaces.h +++ b/flang/include/flang/Optimizer/OpenACC/Support/FIROpenACCTypeInterfaces.h @@ -57,8 +57,11 @@ struct OpenACCMappableModel mlir::Location loc, mlir::TypedValue var, llvm::StringRef varName, - mlir::ValueRange extents, - mlir::Value initVal) const; + mlir::ValueRange extents, mlir::Value initVal, + bool &needsDestroy) const; + + bool generatePrivateDestroy(mlir::Type type, mlir::OpBuilder &builder, + mlir::Location loc, mlir::Value privatized) const; }; } // namespace fir::acc diff --git a/flang/include/flang/Optimizer/Support/Utils.h b/flang/include/flang/Optimizer/Support/Utils.h index 0b31cfea0430a..bbb7e6e52f56c 100644 --- a/flang/include/flang/Optimizer/Support/Utils.h +++ b/flang/include/flang/Optimizer/Support/Utils.h @@ -200,6 +200,12 @@ std::optional> getComponentLowerBoundsIfNonDefault( fir::RecordType recordType, llvm::StringRef component, mlir::ModuleOp module, const mlir::SymbolTable *symbolTable = nullptr); +/// Indicate if a derived type has final routine. Returns std::nullopt if that +/// information is not in the IR; +std::optional +isRecordWithFinalRoutine(fir::RecordType recordType, mlir::ModuleOp module, + const mlir::SymbolTable *symbolTable = nullptr); + /// Generate a LLVM constant value of type `ity`, using the provided offset. mlir::LLVM::ConstantOp genConstantIndex(mlir::Location loc, mlir::Type ity, diff --git a/flang/lib/Lower/OpenACC.cpp b/flang/lib/Lower/OpenACC.cpp index 62e5c0c830972..cfb18914e8126 100644 --- a/flang/lib/Lower/OpenACC.cpp +++ b/flang/lib/Lower/OpenACC.cpp @@ -978,15 +978,40 @@ static RecipeOp genRecipeOp( auto mappableTy = mlir::dyn_cast(ty); assert(mappableTy && "Expected that all variable types are considered mappable"); + bool needsDestroy = false; auto retVal = mappableTy.generatePrivateInit( builder, loc, mlir::cast>( initBlock->getArgument(0)), initName, initBlock->getArguments().take_back(initBlock->getArguments().size() - 1), - initValue); + initValue, needsDestroy); mlir::acc::YieldOp::create(builder, loc, retVal ? retVal : initBlock->getArgument(0)); + // Create destroy region and generate destruction if requested. + if (needsDestroy) { + llvm::SmallVector destroyArgsTy; + llvm::SmallVector destroyArgsLoc; + // original and privatized/reduction value + destroyArgsTy.push_back(ty); + destroyArgsTy.push_back(ty); + destroyArgsLoc.push_back(loc); + destroyArgsLoc.push_back(loc); + // Append bounds arguments (if any) in the same order as init region + if (argsTy.size() > 1) { + destroyArgsTy.append(argsTy.begin() + 1, argsTy.end()); + destroyArgsLoc.insert(destroyArgsLoc.end(), argsTy.size() - 1, loc); + } + + builder.createBlock(&recipe.getDestroyRegion(), + recipe.getDestroyRegion().end(), destroyArgsTy, + destroyArgsLoc); + builder.setInsertionPointToEnd(&recipe.getDestroyRegion().back()); + // Call interface on the privatized/reduction value (2nd argument). + (void)mappableTy.generatePrivateDestroy( + builder, loc, recipe.getDestroyRegion().front().getArgument(1)); + mlir::acc::TerminatorOp::create(builder, loc); + } return recipe; } diff --git a/flang/lib/Optimizer/OpenACC/Support/FIROpenACCTypeInterfaces.cpp b/flang/lib/Optimizer/OpenACC/Support/FIROpenACCTypeInterfaces.cpp index 89aa010e7d9a1..41383fb8af9c1 100644 --- a/flang/lib/Optimizer/OpenACC/Support/FIROpenACCTypeInterfaces.cpp +++ b/flang/lib/Optimizer/OpenACC/Support/FIROpenACCTypeInterfaces.cpp @@ -21,6 +21,7 @@ #include "flang/Optimizer/Dialect/FIRType.h" #include "flang/Optimizer/Dialect/Support/FIRContext.h" #include "flang/Optimizer/Dialect/Support/KindMapping.h" +#include "flang/Optimizer/Support/Utils.h" #include "mlir/Dialect/Arith/IR/Arith.h" #include "mlir/Dialect/OpenACC/OpenACC.h" #include "mlir/IR/BuiltinOps.h" @@ -548,14 +549,27 @@ template mlir::Value OpenACCMappableModel::generatePrivateInit( mlir::Type type, mlir::OpBuilder &builder, mlir::Location loc, mlir::TypedValue var, llvm::StringRef varName, - mlir::ValueRange extents, mlir::Value initVal) const { + mlir::ValueRange extents, mlir::Value initVal, bool &needsDestroy) const { + needsDestroy = false; mlir::Value retVal; mlir::Type unwrappedTy = fir::unwrapRefType(type); mlir::ModuleOp mod = builder.getInsertionBlock() ->getParent() ->getParentOfType(); - fir::FirOpBuilder firBuilder(builder, mod); + if (auto recType = llvm::dyn_cast( + fir::getFortranElementType(unwrappedTy))) { + // Need to make deep copies of allocatable components. + if (fir::isRecordWithAllocatableMember(recType)) + TODO(loc, + "OpenACC: privatizing derived type with allocatable components"); + // Need to decide if user assignment/final routine should be called. + if (fir::isRecordWithFinalRoutine(recType, mod).value_or(false)) + TODO(loc, "OpenACC: privatizing derived type with user assignment or " + "final routine "); + } + + fir::FirOpBuilder firBuilder(builder, mod); auto getDeclareOpForType = [&](mlir::Type ty) -> hlfir::DeclareOp { auto alloca = fir::AllocaOp::create(firBuilder, loc, ty); return hlfir::DeclareOp::create(firBuilder, loc, alloca, varName); @@ -615,9 +629,11 @@ mlir::Value OpenACCMappableModel::generatePrivateInit( mlir::Value firClass = fir::EmboxOp::create(builder, loc, boxTy, allocatedScalar); fir::StoreOp::create(builder, loc, firClass, retVal); + needsDestroy = true; } else if (mlir::isa(innerTy)) { hlfir::Entity source = hlfir::Entity{var}; - auto [temp, cleanup] = hlfir::createTempFromMold(loc, firBuilder, source); + auto [temp, cleanupFlag] = + hlfir::createTempFromMold(loc, firBuilder, source); if (fir::isa_ref_type(type)) { // When the temp is created - it is not a reference - thus we can // end up with a type inconsistency. Therefore ensure storage is created @@ -636,6 +652,9 @@ mlir::Value OpenACCMappableModel::generatePrivateInit( } else { retVal = temp; } + // If heap was allocated, a destroy is required later. + if (cleanupFlag) + needsDestroy = true; } else { TODO(loc, "Unsupported boxed type for OpenACC private-like recipe"); } @@ -667,23 +686,61 @@ template mlir::Value OpenACCMappableModel::generatePrivateInit( mlir::Type type, mlir::OpBuilder &builder, mlir::Location loc, mlir::TypedValue var, llvm::StringRef varName, - mlir::ValueRange extents, mlir::Value initVal) const; + mlir::ValueRange extents, mlir::Value initVal, bool &needsDestroy) const; template mlir::Value OpenACCMappableModel::generatePrivateInit( mlir::Type type, mlir::OpBuilder &builder, mlir::Location loc, mlir::TypedValue var, llvm::StringRef varName, - mlir::ValueRange extents, mlir::Value initVal) const; + mlir::ValueRange extents, mlir::Value initVal, bool &needsDestroy) const; template mlir::Value OpenACCMappableModel::generatePrivateInit( mlir::Type type, mlir::OpBuilder &builder, mlir::Location loc, mlir::TypedValue var, llvm::StringRef varName, - mlir::ValueRange extents, mlir::Value initVal) const; + mlir::ValueRange extents, mlir::Value initVal, bool &needsDestroy) const; template mlir::Value OpenACCMappableModel::generatePrivateInit( mlir::Type type, mlir::OpBuilder &builder, mlir::Location loc, mlir::TypedValue var, llvm::StringRef varName, - mlir::ValueRange extents, mlir::Value initVal) const; + mlir::ValueRange extents, mlir::Value initVal, bool &needsDestroy) const; + +template +bool OpenACCMappableModel::generatePrivateDestroy( + mlir::Type type, mlir::OpBuilder &builder, mlir::Location loc, + mlir::Value privatized) const { + mlir::Type unwrappedTy = fir::unwrapRefType(type); + // For boxed scalars allocated with AllocMem during init, free the heap. + if (auto boxTy = mlir::dyn_cast_or_null(unwrappedTy)) { + mlir::Value boxVal = privatized; + if (fir::isa_ref_type(boxVal.getType())) + boxVal = fir::LoadOp::create(builder, loc, boxVal); + mlir::Value addr = fir::BoxAddrOp::create(builder, loc, boxVal); + // FreeMem only accepts fir.heap and this may not be represented in the box + // type if the privatized entity is not an allocatable. + mlir::Type heapType = + fir::HeapType::get(fir::unwrapRefType(addr.getType())); + if (heapType != addr.getType()) + addr = fir::ConvertOp::create(builder, loc, heapType, addr); + fir::FreeMemOp::create(builder, loc, addr); + return true; + } + + // Nothing to do for other categories by default, they are stack allocated. + return true; +} + +template bool OpenACCMappableModel::generatePrivateDestroy( + mlir::Type type, mlir::OpBuilder &builder, mlir::Location loc, + mlir::Value privatized) const; +template bool OpenACCMappableModel::generatePrivateDestroy( + mlir::Type type, mlir::OpBuilder &builder, mlir::Location loc, + mlir::Value privatized) const; +template bool OpenACCMappableModel::generatePrivateDestroy( + mlir::Type type, mlir::OpBuilder &builder, mlir::Location loc, + mlir::Value privatized) const; +template bool OpenACCMappableModel::generatePrivateDestroy( + mlir::Type type, mlir::OpBuilder &builder, mlir::Location loc, + mlir::Value privatized) const; } // namespace fir::acc diff --git a/flang/lib/Optimizer/Support/Utils.cpp b/flang/lib/Optimizer/Support/Utils.cpp index c71642ce4e806..92390e4a3a230 100644 --- a/flang/lib/Optimizer/Support/Utils.cpp +++ b/flang/lib/Optimizer/Support/Utils.cpp @@ -51,6 +51,16 @@ std::optional> fir::getComponentLowerBoundsIfNonDefault( return std::nullopt; } +std::optional +fir::isRecordWithFinalRoutine(fir::RecordType recordType, mlir::ModuleOp module, + const mlir::SymbolTable *symbolTable) { + fir::TypeInfoOp typeInfo = + fir::lookupTypeInfoOp(recordType, module, symbolTable); + if (!typeInfo) + return std::nullopt; + return !typeInfo.getNoFinal(); +} + mlir::LLVM::ConstantOp fir::genConstantIndex(mlir::Location loc, mlir::Type ity, mlir::ConversionPatternRewriter &rewriter, diff --git a/flang/test/Lower/OpenACC/acc-firstprivate-derived-allocatable-component.f90 b/flang/test/Lower/OpenACC/acc-firstprivate-derived-allocatable-component.f90 index 429f207bb5669..3987f9f6f5674 100644 --- a/flang/test/Lower/OpenACC/acc-firstprivate-derived-allocatable-component.f90 +++ b/flang/test/Lower/OpenACC/acc-firstprivate-derived-allocatable-component.f90 @@ -4,6 +4,11 @@ ! RUN: bbc -fopenacc -emit-hlfir %s -o - | FileCheck %s ! RUN: bbc -fopenacc -emit-fir %s -o - | FileCheck %s --check-prefix=FIR-CHECK +! TODO: This test hits a fatal TODO. Deal with allocatable component +! destructions. For arrays, allocatable component allocation may also be +! missing. +! XFAIL: * + module m_firstprivate_derived_alloc_comp type point real, allocatable :: x(:) diff --git a/flang/test/Lower/OpenACC/acc-private.f90 b/flang/test/Lower/OpenACC/acc-private.f90 index d37eb8d7aaf6d..485825dfa8129 100644 --- a/flang/test/Lower/OpenACC/acc-private.f90 +++ b/flang/test/Lower/OpenACC/acc-private.f90 @@ -26,6 +26,12 @@ ! CHECK: %[[DES_DST:.*]] = hlfir.designate %[[ARG1]] shape %[[SHAPE]] : (!fir.box>, !fir.shape<3>) -> !fir.box> ! CHECK: hlfir.assign %[[DES_SRC]] to %[[DES_DST]] : !fir.box>, !fir.box> ! CHECK: acc.terminator +! CHECK: } destroy { +! CHECK: ^bb0(%[[ARG0:.*]]: !fir.box>, %[[ARG1:.*]]: !fir.box>): +! CHECK: %[[ADDR:.*]] = fir.box_addr %[[ARG1]] : (!fir.box>) -> !fir.ref> +! CHECK: %[[CAST:.*]] = fir.convert %[[ADDR]] : (!fir.ref>) -> !fir.heap> +! CHECK: fir.freemem %[[CAST]] : !fir.heap> +! CHECK: acc.terminator ! CHECK: } ! CHECK-LABEL: acc.firstprivate.recipe @firstprivatization_section_lb4.ub9_box_Uxi32 : !fir.box> init { @@ -47,6 +53,12 @@ ! CHECK: %[[RIGHT:.*]] = hlfir.designate %[[ARG1]] shape %[[SHAPE]] : (!fir.box>, !fir.shape<1>) -> !fir.box> ! CHECK: hlfir.assign %[[LEFT]] to %[[RIGHT]] : !fir.box>, !fir.box> ! CHECK: acc.terminator +! CHECK: } destroy { +! CHECK: ^bb0(%[[ARG0:.*]]: !fir.box>, %[[ARG1:.*]]: !fir.box>): +! CHECK: %[[ADDR:.*]] = fir.box_addr %[[ARG1]] : (!fir.box>) -> !fir.ref> +! CHECK: %[[CAST:.*]] = fir.convert %[[ADDR]] : (!fir.ref>) -> !fir.heap> +! CHECK: fir.freemem %[[CAST]] : !fir.heap> +! CHECK: acc.terminator ! CHECK: } ! CHECK-LABEL: acc.firstprivate.recipe @firstprivatization_box_Uxi32 : !fir.box> init { @@ -64,6 +76,12 @@ ! CHECK: %[[DES_V2:.*]] = hlfir.designate %[[ARG1]] shape %[[SHAPE]] : (!fir.box>, !fir.shape<1>) -> !fir.box> ! CHECK: hlfir.assign %[[DES_V1]] to %[[DES_V2]] : !fir.box>, !fir.box> ! CHECK: acc.terminator +! CHECK: } destroy { +! CHECK: ^bb0(%[[ARG0:.*]]: !fir.box>, %[[ARG1:.*]]: !fir.box>): +! CHECK: %[[ADDR:.*]] = fir.box_addr %[[ARG1]] : (!fir.box>) -> !fir.ref> +! CHECK: %[[CAST:.*]] = fir.convert %[[ADDR]] : (!fir.ref>) -> !fir.heap> +! CHECK: fir.freemem %[[CAST]] : !fir.heap> +! CHECK: acc.terminator ! CHECK: } ! CHECK-LABEL: acc.private.recipe @privatization_box_UxUx2xi32 : !fir.box> init { @@ -74,6 +92,12 @@ ! CHECK: %[[TEMP:.*]] = fir.allocmem !fir.array, %[[DIM0]]#1, %[[DIM1]]#1 {bindc_name = ".tmp", uniq_name = ""} ! CHECK: %[[DECL:.*]]:2 = hlfir.declare %[[TEMP]](%[[SHAPE]]) {uniq_name = ".tmp"} : (!fir.heap>, !fir.shape<3>) -> (!fir.box>, !fir.heap>) ! CHECK: acc.yield %[[DECL]]#0 : !fir.box> +! CHECK: } destroy { +! CHECK: ^bb0(%[[ARG0:.*]]: !fir.box>, %[[ARG1:.*]]: !fir.box>): +! CHECK: %[[ADDR:.*]] = fir.box_addr %[[ARG1]] : (!fir.box>) -> !fir.ref> +! CHECK: %[[CAST:.*]] = fir.convert %[[ADDR]] : (!fir.ref>) -> !fir.heap> +! CHECK: fir.freemem %[[CAST]] : !fir.heap> +! CHECK: acc.terminator ! CHECK: } ! CHECK-LABEL: acc.private.recipe @privatization_ref_box_ptr_Uxi32 : !fir.ref>>> init { @@ -89,6 +113,13 @@ ! CHECK: %[[CONV:.*]] = fir.convert %[[DECLAREBOX]]#0 : (!fir.ref>>>) -> !fir.ref>> ! CHECK: fir.store %[[DECLARE]]#0 to %[[CONV]] : !fir.ref>> ! CHECK: acc.yield %[[DECLAREBOX]]#0 : !fir.ref>>> +! CHECK: } destroy { +! CHECK: ^bb0(%arg0: !fir.ref>>>, %arg1: !fir.ref>>>): +! CHECK: %[[LOAD:.*]] = fir.load %arg1 : !fir.ref>>> +! CHECK: %[[ADDR:.*]] = fir.box_addr %[[LOAD]] : (!fir.box>>) -> !fir.ptr> +! CHECK: %[[CAST:.*]] = fir.convert %[[ADDR]] : (!fir.ptr>) -> !fir.heap> +! CHECK: fir.freemem %[[CAST]] : !fir.heap> +! CHECK: acc.terminator ! CHECK: } ! CHECK-LABEL: @privatization_ref_box_heap_i32 : !fir.ref>> init { @@ -99,6 +130,12 @@ ! CHECK: %[[BOX:.*]] = fir.embox %[[ALLOCMEM]] : (!fir.heap) -> !fir.box> ! CHECK: fir.store %[[BOX]] to %[[DECLARE]]#0 : !fir.ref>> ! CHECK: acc.yield %[[DECLARE]]#0 : !fir.ref>> +! CHECK: } destroy { +! CHECK: ^bb0(%arg0: !fir.ref>>, %arg1: !fir.ref>>): +! CHECK: %[[LOAD:.*]] = fir.load %arg1 : !fir.ref>> +! CHECK: %[[ADDR:.*]] = fir.box_addr %[[LOAD]] : (!fir.box>) -> !fir.heap +! CHECK: fir.freemem %[[ADDR]] : !fir.heap +! CHECK: acc.terminator ! CHECK: } ! CHECK-LABEL: acc.private.recipe @privatization_ref_box_heap_Uxi32 : !fir.ref>>> init { @@ -114,6 +151,12 @@ ! CHECK: %[[CONV:.*]] = fir.convert %[[DECLAREBOX]]#0 : (!fir.ref>>>) -> !fir.ref>> ! CHECK: fir.store %[[DECLARE]]#0 to %[[CONV]] : !fir.ref>> ! CHECK: acc.yield %[[DECLAREBOX]]#0 : !fir.ref>>> +! CHECK: } destroy { +! CHECK: ^bb0(%arg0: !fir.ref>>>, %arg1: !fir.ref>>>): +! CHECK: %[[LOAD:.*]] = fir.load %arg1 : !fir.ref>>> +! CHECK: %[[ADDR:.*]] = fir.box_addr %[[LOAD]] : (!fir.box>>) -> !fir.heap> +! CHECK: fir.freemem %[[ADDR]] : !fir.heap> +! CHECK: acc.terminator ! CHECK: } ! CHECK-LABEL: acc.private.recipe @privatization_box_Uxi32 : !fir.box> init { @@ -124,6 +167,12 @@ ! CHECK: %[[TEMP:.*]] = fir.allocmem !fir.array, %0#1 {bindc_name = ".tmp", uniq_name = ""} ! CHECK: %[[DECLARE:.*]]:2 = hlfir.declare %[[TEMP]](%[[SHAPE]]) {uniq_name = ".tmp"} : (!fir.heap>, !fir.shape<1>) -> (!fir.box>, !fir.heap>) ! CHECK: acc.yield %[[DECLARE:.*]]#0 : !fir.box> +! CHECK: } destroy { +! CHECK: ^bb0(%[[ARG0:.*]]: !fir.box>, %[[ARG1:.*]]: !fir.box>): +! CHECK: %[[ADDR:.*]] = fir.box_addr %[[ARG1]] : (!fir.box>) -> !fir.ref> +! CHECK: %[[CAST:.*]] = fir.convert %[[ADDR]] : (!fir.ref>) -> !fir.heap> +! CHECK: fir.freemem %[[CAST]] : !fir.heap> +! CHECK: acc.terminator ! CHECK: } ! CHECK-LABEL: acc.firstprivate.recipe @firstprivatization_section_lb50.ub99_ref_50xf32 : !fir.ref> init { @@ -140,6 +189,7 @@ ! CHECK: %[[DES_SRC:.*]] = hlfir.designate %[[DECL_SRC]]#0 shape %[[SHAPE:.*]] : (!fir.ref>, !fir.shape<1>) -> !fir.ref> ! CHECK: %[[DES_DST:.*]] = hlfir.designate %[[DECL_DST]]#0 shape %[[SHAPE:.*]] : (!fir.ref>, !fir.shape<1>) -> !fir.ref> ! CHECK: hlfir.assign %[[DES_SRC]] to %[[DES_DST]] : !fir.ref>, !fir.ref> +! CHECK: acc.terminator ! CHECK: } ! CHECK-LABEL: acc.firstprivate.recipe @firstprivatization_ref_100xf32 : !fir.ref> init { diff --git a/mlir/include/mlir/Dialect/OpenACC/OpenACCTypeInterfaces.td b/mlir/include/mlir/Dialect/OpenACC/OpenACCTypeInterfaces.td index 0d16255c5a994..d269a765ed739 100644 --- a/mlir/include/mlir/Dialect/OpenACC/OpenACCTypeInterfaces.td +++ b/mlir/include/mlir/Dialect/OpenACC/OpenACCTypeInterfaces.td @@ -274,6 +274,14 @@ def OpenACC_MappableTypeInterface : TypeInterface<"MappableType"> { The `initVal` can be empty - it is primarily needed for reductions to ensure the variable is also initialized with appropriate value. + The `needsDestroy` out-parameter is set by implementations to indicate + that destruction code must be generated after the returned private + variable usages, typically in the destroy region of recipe operations + (for example, when heap allocations or temporaries requiring cleanup + are created during initialization). When `needsDestroy` is set, callers + should invoke `generatePrivateDestroy` in the recipe's destroy region + with the privatized value returned by this method. + If the return value is empty, it means that recipe body was not successfully generated. }], @@ -284,12 +292,38 @@ def OpenACC_MappableTypeInterface : TypeInterface<"MappableType"> { "::mlir::TypedValue<::mlir::acc::MappableType>":$var, "::llvm::StringRef":$varName, "::mlir::ValueRange":$extents, - "::mlir::Value":$initVal), + "::mlir::Value":$initVal, + "bool &":$needsDestroy), /*methodBody=*/"", /*defaultImplementation=*/[{ return {}; }] >, + InterfaceMethod< + /*description=*/[{ + Generates destruction operations for a privatized value previously + produced by `generatePrivateInit`. This is typically inserted in a + recipe's destroy region, after all uses of the privatized value. + + The `privatized` value is the SSA value yielded by the init region + (and passed as the privatized argument to the destroy region). + Implementations should free heap-allocated storage or perform any + cleanup required for the given type. If no destruction is required, + this function should be a no-op and return `true`. + + Returns true if destruction was successfully generated or deemed not + necessary, false otherwise. + }], + /*retTy=*/"bool", + /*methodName=*/"generatePrivateDestroy", + /*args=*/(ins "::mlir::OpBuilder &":$builder, + "::mlir::Location":$loc, + "::mlir::Value":$privatized), + /*methodBody=*/"", + /*defaultImplementation=*/[{ + return true; + }] + >, ]; }