[flang-commits] [flang] [flang] Fix some memory leaks (PR #121050)
Matthias Springer via flang-commits
flang-commits at lists.llvm.org
Tue Dec 24 04:09:09 PST 2024
https://github.com/matthias-springer created https://github.com/llvm/llvm-project/pull/121050
None
>From c702bf461747940f4e44677c120b0b0d5290a588 Mon Sep 17 00:00:00 2001
From: Matthias Springer <mspringer at nvidia.com>
Date: Tue, 24 Dec 2024 13:08:09 +0100
Subject: [PATCH] [flang] Fix some memory leaks
---
.../include/flang/Frontend/FrontendActions.h | 3 +-
flang/include/flang/Lower/AbstractConverter.h | 4 +--
flang/include/flang/Lower/Bridge.h | 6 ++--
flang/include/flang/Lower/OpenACC.h | 8 +++---
flang/include/flang/Tools/CrossToolHelpers.h | 4 +--
flang/lib/Frontend/FrontendActions.cpp | 22 +++++++++------
flang/lib/Lower/Bridge.cpp | 28 ++++++++-----------
flang/lib/Lower/OpenACC.cpp | 14 ++++++----
flang/lib/Optimizer/CodeGen/CodeGen.cpp | 2 +-
.../unittests/Frontend/CodeGenActionTest.cpp | 3 +-
.../Builder/Runtime/RuntimeCallTestBase.h | 4 ++-
.../Optimizer/FortranVariableTest.cpp | 4 ++-
flang/unittests/Runtime/ArrayConstructor.cpp | 3 ++
flang/unittests/Runtime/CharacterTest.cpp | 3 ++
14 files changed, 62 insertions(+), 46 deletions(-)
diff --git a/flang/include/flang/Frontend/FrontendActions.h b/flang/include/flang/Frontend/FrontendActions.h
index 374fd76c8ae17d..4e3d3cb2657db7 100644
--- a/flang/include/flang/Frontend/FrontendActions.h
+++ b/flang/include/flang/Frontend/FrontendActions.h
@@ -19,6 +19,7 @@
#include "flang/Semantics/semantics.h"
#include "mlir/IR/BuiltinOps.h"
+#include "mlir/IR/OwningOpRef.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/IR/Module.h"
#include <memory>
@@ -215,8 +216,8 @@ class CodeGenAction : public FrontendAction {
CodeGenAction(BackendActionTy act) : action{act} {};
/// @name MLIR
/// {
- std::unique_ptr<mlir::ModuleOp> mlirModule;
std::unique_ptr<mlir::MLIRContext> mlirCtx;
+ mlir::OwningOpRef<mlir::ModuleOp> mlirModule;
/// }
/// @name LLVM IR
diff --git a/flang/include/flang/Lower/AbstractConverter.h b/flang/include/flang/Lower/AbstractConverter.h
index 8f026ac3280bf4..607aff41f64595 100644
--- a/flang/include/flang/Lower/AbstractConverter.h
+++ b/flang/include/flang/Lower/AbstractConverter.h
@@ -62,7 +62,7 @@ struct SymbolBox;
namespace pft {
struct Variable;
struct FunctionLikeUnit;
-}
+} // namespace pft
using SomeExpr = Fortran::evaluate::Expr<Fortran::evaluate::SomeType>;
using SymbolRef = Fortran::common::Reference<const Fortran::semantics::Symbol>;
@@ -295,7 +295,7 @@ class AbstractConverter {
/// Get the OpBuilder
virtual fir::FirOpBuilder &getFirOpBuilder() = 0;
/// Get the ModuleOp
- virtual mlir::ModuleOp &getModuleOp() = 0;
+ virtual mlir::ModuleOp getModuleOp() = 0;
/// Get the MLIRContext
virtual mlir::MLIRContext &getMLIRContext() = 0;
/// Unique a symbol (add a containing scope specific prefix)
diff --git a/flang/include/flang/Lower/Bridge.h b/flang/include/flang/Lower/Bridge.h
index 8ea5ed52e28218..6404a16f7785ae 100644
--- a/flang/include/flang/Lower/Bridge.h
+++ b/flang/include/flang/Lower/Bridge.h
@@ -23,6 +23,7 @@
#include "flang/Optimizer/Builder/FIRBuilder.h"
#include "flang/Optimizer/Dialect/Support/KindMapping.h"
#include "mlir/IR/BuiltinOps.h"
+#include "mlir/IR/OwningOpRef.h"
#include <set>
namespace llvm {
@@ -83,7 +84,8 @@ class LoweringBridge {
mlir::MLIRContext &getMLIRContext() { return context; }
/// Get the ModuleOp. It can never be null, which is asserted in the ctor.
- mlir::ModuleOp &getModule() { return *module.get(); }
+ mlir::ModuleOp getModule() { return *module; }
+ mlir::ModuleOp getModuleAndRelease() { return module.release(); }
const Fortran::common::IntrinsicTypeDefaultKinds &getDefaultKinds() const {
return defaultKinds;
@@ -166,7 +168,7 @@ class LoweringBridge {
const Fortran::evaluate::TargetCharacteristics &targetCharacteristics;
const Fortran::parser::AllCookedSources *cooked;
mlir::MLIRContext &context;
- std::unique_ptr<mlir::ModuleOp> module;
+ mlir::OwningOpRef<mlir::ModuleOp> module;
fir::KindMapping &kindMap;
const Fortran::lower::LoweringOptions &loweringOptions;
const std::vector<Fortran::lower::EnvironmentDefault> &envDefaults;
diff --git a/flang/include/flang/Lower/OpenACC.h b/flang/include/flang/Lower/OpenACC.h
index fbf61e7184ae27..0d7038a7fd8564 100644
--- a/flang/include/flang/Lower/OpenACC.h
+++ b/flang/include/flang/Lower/OpenACC.h
@@ -19,7 +19,7 @@ namespace llvm {
template <typename T, unsigned N>
class SmallVector;
class StringRef;
-}
+} // namespace llvm
namespace mlir {
class Location;
@@ -44,7 +44,7 @@ struct OpenACCRoutineConstruct;
namespace semantics {
class SemanticsContext;
class Symbol;
-}
+} // namespace semantics
namespace lower {
@@ -78,11 +78,11 @@ void genOpenACCDeclarativeConstruct(AbstractConverter &,
AccRoutineInfoMappingList &);
void genOpenACCRoutineConstruct(AbstractConverter &,
Fortran::semantics::SemanticsContext &,
- mlir::ModuleOp &,
+ mlir::ModuleOp,
const parser::OpenACCRoutineConstruct &,
AccRoutineInfoMappingList &);
-void finalizeOpenACCRoutineAttachment(mlir::ModuleOp &,
+void finalizeOpenACCRoutineAttachment(mlir::ModuleOp,
AccRoutineInfoMappingList &);
/// Get a acc.private.recipe op for the given type or create it if it does not
diff --git a/flang/include/flang/Tools/CrossToolHelpers.h b/flang/include/flang/Tools/CrossToolHelpers.h
index c0091e1c953b8f..0286f2aa145192 100644
--- a/flang/include/flang/Tools/CrossToolHelpers.h
+++ b/flang/include/flang/Tools/CrossToolHelpers.h
@@ -174,7 +174,7 @@ struct OffloadModuleOpts {
// Shares assinging of the OpenMP OffloadModuleInterface and its assorted
// attributes accross Flang tools (bbc/flang)
[[maybe_unused]] static void setOffloadModuleInterfaceAttributes(
- mlir::ModuleOp &module, OffloadModuleOpts Opts) {
+ mlir::ModuleOp module, OffloadModuleOpts Opts) {
// Should be registered by the OpenMPDialect
if (auto offloadMod = llvm::dyn_cast<mlir::omp::OffloadModuleInterface>(
module.getOperation())) {
@@ -198,7 +198,7 @@ struct OffloadModuleOpts {
}
[[maybe_unused]] static void setOpenMPVersionAttribute(
- mlir::ModuleOp &module, int64_t version) {
+ mlir::ModuleOp module, int64_t version) {
module.getOperation()->setAttr(
mlir::StringAttr::get(module.getContext(), llvm::Twine{"omp.version"}),
mlir::omp::VersionAttr::get(module.getContext(), version));
diff --git a/flang/lib/Frontend/FrontendActions.cpp b/flang/lib/Frontend/FrontendActions.cpp
index 77631f70dfd19f..603cb039d20b14 100644
--- a/flang/lib/Frontend/FrontendActions.cpp
+++ b/flang/lib/Frontend/FrontendActions.cpp
@@ -149,7 +149,7 @@ bool PrescanAndSemaDebugAction::beginSourceFileAction() {
(runSemanticChecks() || true) && (generateRtTypeTables() || true);
}
-static void addDependentLibs(mlir::ModuleOp &mlirModule, CompilerInstance &ci) {
+static void addDependentLibs(mlir::ModuleOp mlirModule, CompilerInstance &ci) {
const std::vector<std::string> &libs =
ci.getInvocation().getCodeGenOpts().DependentLibs;
if (libs.empty()) {
@@ -171,7 +171,7 @@ static void addDependentLibs(mlir::ModuleOp &mlirModule, CompilerInstance &ci) {
// Add to MLIR code target specific items which are dependent on target
// configuration specified by the user.
// Clang equivalent function: AMDGPUTargetCodeGenInfo::emitTargetGlobals
-static void addAMDGPUSpecificMLIRItems(mlir::ModuleOp &mlirModule,
+static void addAMDGPUSpecificMLIRItems(mlir::ModuleOp mlirModule,
CompilerInstance &ci) {
const TargetOptions &targetOpts = ci.getInvocation().getTargetOpts();
const llvm::Triple triple(targetOpts.triple);
@@ -269,7 +269,7 @@ bool CodeGenAction::beginSourceFileAction() {
return false;
}
- mlirModule = std::make_unique<mlir::ModuleOp>(module.release());
+ mlirModule = std::move(module);
const llvm::DataLayout &dl = targetMachine.createDataLayout();
fir::support::setMLIRDataLayout(*mlirModule, dl);
return true;
@@ -303,14 +303,11 @@ bool CodeGenAction::beginSourceFileAction() {
ci.getInvocation().getFrontendOpts().features, targetMachine,
ci.getInvocation().getTargetOpts(), ci.getInvocation().getCodeGenOpts());
- // Fetch module from lb, so we can set
- mlirModule = std::make_unique<mlir::ModuleOp>(lb.getModule());
-
if (ci.getInvocation().getFrontendOpts().features.IsEnabled(
Fortran::common::LanguageFeature::OpenMP)) {
- setOffloadModuleInterfaceAttributes(*mlirModule,
+ setOffloadModuleInterfaceAttributes(lb.getModule(),
ci.getInvocation().getLangOpts());
- setOpenMPVersionAttribute(*mlirModule,
+ setOpenMPVersionAttribute(lb.getModule(),
ci.getInvocation().getLangOpts().OpenMPVersion);
}
@@ -318,6 +315,9 @@ bool CodeGenAction::beginSourceFileAction() {
Fortran::parser::Program &parseTree{*ci.getParsing().parseTree()};
lb.lower(parseTree, ci.getSemanticsContext());
+ // Fetch module from lb, so we can set
+ mlirModule = lb.getModuleAndRelease();
+
// Add target specific items like dependent libraries, target specific
// constants etc.
addDependentLibs(*mlirModule, ci);
@@ -961,6 +961,9 @@ static void generateMachineCodeOrAssemblyImpl(clang::DiagnosticsEngine &diags,
// Run the passes
codeGenPasses.run(llvmModule);
+
+ // Cleanup
+ delete tlii;
}
void CodeGenAction::runOptimizationPipeline(llvm::raw_pwrite_stream &os) {
@@ -1043,6 +1046,9 @@ void CodeGenAction::runOptimizationPipeline(llvm::raw_pwrite_stream &os) {
// Run the passes.
mpm.run(*llvmModule, mam);
+
+ // Cleanup
+ delete tlii;
}
// This class handles optimization remark messages requested if
diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index 17b794d147c6f5..c7e2635230e988 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -1028,7 +1028,7 @@ class FirConverter : public Fortran::lower::AbstractConverter {
fir::FirOpBuilder &getFirOpBuilder() override final { return *builder; }
- mlir::ModuleOp &getModuleOp() override final { return bridge.getModule(); }
+ mlir::ModuleOp getModuleOp() override final { return bridge.getModule(); }
mlir::MLIRContext &getMLIRContext() override final {
return bridge.getMLIRContext();
@@ -6137,10 +6137,7 @@ void Fortran::lower::LoweringBridge::lower(
}
void Fortran::lower::LoweringBridge::parseSourceFile(llvm::SourceMgr &srcMgr) {
- mlir::OwningOpRef<mlir::ModuleOp> owningRef =
- mlir::parseSourceFile<mlir::ModuleOp>(srcMgr, &context);
- module.reset(new mlir::ModuleOp(owningRef.get().getOperation()));
- owningRef.release();
+ module = mlir::parseSourceFile<mlir::ModuleOp>(srcMgr, &context);
}
Fortran::lower::LoweringBridge::LoweringBridge(
@@ -6207,19 +6204,18 @@ Fortran::lower::LoweringBridge::LoweringBridge(
};
// Create the module and attach the attributes.
- module = std::make_unique<mlir::ModuleOp>(
+ module = mlir::OwningOpRef<mlir::ModuleOp>(
mlir::ModuleOp::create(getPathLocation()));
- assert(module.get() && "module was not created");
- fir::setTargetTriple(*module.get(), triple);
- fir::setKindMapping(*module.get(), kindMap);
- fir::setTargetCPU(*module.get(), targetMachine.getTargetCPU());
- fir::setTuneCPU(*module.get(), targetOpts.cpuToTuneFor);
- fir::setTargetFeatures(*module.get(), targetMachine.getTargetFeatureString());
- fir::support::setMLIRDataLayout(*module.get(),
- targetMachine.createDataLayout());
- fir::setIdent(*module.get(), Fortran::common::getFlangFullVersion());
+ assert(*module && "module was not created");
+ fir::setTargetTriple(*module, triple);
+ fir::setKindMapping(*module, kindMap);
+ fir::setTargetCPU(*module, targetMachine.getTargetCPU());
+ fir::setTuneCPU(*module, targetOpts.cpuToTuneFor);
+ fir::setTargetFeatures(*module, targetMachine.getTargetFeatureString());
+ fir::support::setMLIRDataLayout(*module, targetMachine.createDataLayout());
+ fir::setIdent(*module, Fortran::common::getFlangFullVersion());
if (cgOpts.RecordCommandLine)
- fir::setCommandline(*module.get(), *cgOpts.RecordCommandLine);
+ fir::setCommandline(*module, *cgOpts.RecordCommandLine);
}
void Fortran::lower::genCleanUpInRegionIfAny(
diff --git a/flang/lib/Lower/OpenACC.cpp b/flang/lib/Lower/OpenACC.cpp
index ed18ad89c16ef5..8155c36396b112 100644
--- a/flang/lib/Lower/OpenACC.cpp
+++ b/flang/lib/Lower/OpenACC.cpp
@@ -2670,11 +2670,13 @@ static void genACCDataOp(Fortran::lower::AbstractConverter &converter,
asyncOnlyDeviceTypes);
attachEntryOperands.append(dataClauseOperands.begin() + crtDataStart,
dataClauseOperands.end());
- } else if(const auto *defaultClause =
- std::get_if<Fortran::parser::AccClause::Default>(&clause.u)) {
+ } else if (const auto *defaultClause =
+ std::get_if<Fortran::parser::AccClause::Default>(
+ &clause.u)) {
if ((defaultClause->v).v == llvm::acc::DefaultValue::ACC_Default_none)
hasDefaultNone = true;
- else if ((defaultClause->v).v == llvm::acc::DefaultValue::ACC_Default_present)
+ else if ((defaultClause->v).v ==
+ llvm::acc::DefaultValue::ACC_Default_present)
hasDefaultPresent = true;
}
}
@@ -3830,7 +3832,7 @@ genDeclareInFunction(Fortran::lower::AbstractConverter &converter,
static void
genDeclareInModule(Fortran::lower::AbstractConverter &converter,
- mlir::ModuleOp &moduleOp,
+ mlir::ModuleOp moduleOp,
const Fortran::parser::AccClauseList &accClauseList) {
mlir::OpBuilder modBuilder(moduleOp.getBodyRegion());
for (const Fortran::parser::AccClause &clause : accClauseList.v) {
@@ -3981,7 +3983,7 @@ static void attachRoutineInfo(mlir::func::FuncOp func,
void Fortran::lower::genOpenACCRoutineConstruct(
Fortran::lower::AbstractConverter &converter,
- Fortran::semantics::SemanticsContext &semanticsContext, mlir::ModuleOp &mod,
+ Fortran::semantics::SemanticsContext &semanticsContext, mlir::ModuleOp mod,
const Fortran::parser::OpenACCRoutineConstruct &routineConstruct,
Fortran::lower::AccRoutineInfoMappingList &accRoutineInfos) {
fir::FirOpBuilder &builder = converter.getFirOpBuilder();
@@ -4139,7 +4141,7 @@ void Fortran::lower::genOpenACCRoutineConstruct(
}
void Fortran::lower::finalizeOpenACCRoutineAttachment(
- mlir::ModuleOp &mod,
+ mlir::ModuleOp mod,
Fortran::lower::AccRoutineInfoMappingList &accRoutineInfos) {
for (auto &mapping : accRoutineInfos) {
mlir::func::FuncOp funcOp =
diff --git a/flang/lib/Optimizer/CodeGen/CodeGen.cpp b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
index 926f83b9c9a648..4edea86b417c38 100644
--- a/flang/lib/Optimizer/CodeGen/CodeGen.cpp
+++ b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
@@ -3087,7 +3087,7 @@ struct GlobalOpConversion : public fir::FIROpConversion<fir::GlobalOp> {
private:
static void addComdat(mlir::LLVM::GlobalOp &global,
mlir::ConversionPatternRewriter &rewriter,
- mlir::ModuleOp &module) {
+ mlir::ModuleOp module) {
const char *comdatName = "__llvm_comdat";
mlir::LLVM::ComdatOp comdatOp =
module.lookupSymbol<mlir::LLVM::ComdatOp>(comdatName);
diff --git a/flang/unittests/Frontend/CodeGenActionTest.cpp b/flang/unittests/Frontend/CodeGenActionTest.cpp
index 5d75de03d4e55c..e9ff095973b97c 100644
--- a/flang/unittests/Frontend/CodeGenActionTest.cpp
+++ b/flang/unittests/Frontend/CodeGenActionTest.cpp
@@ -72,8 +72,7 @@ class LLVMConversionFailureCodeGenAction : public CodeGenAction {
mlirCtx->loadDialect<test::DummyDialect>();
mlir::Location loc(mlir::UnknownLoc::get(mlirCtx.get()));
- mlirModule =
- std::make_unique<mlir::ModuleOp>(mlir::ModuleOp::create(loc, "mod"));
+ mlirModule = mlir::ModuleOp::create(loc, "mod");
mlir::OpBuilder builder(mlirCtx.get());
builder.setInsertionPointToStart(&mlirModule->getRegion().front());
diff --git a/flang/unittests/Optimizer/Builder/Runtime/RuntimeCallTestBase.h b/flang/unittests/Optimizer/Builder/Runtime/RuntimeCallTestBase.h
index d0ec97733e83d5..fc06d2fb38a158 100644
--- a/flang/unittests/Optimizer/Builder/Runtime/RuntimeCallTestBase.h
+++ b/flang/unittests/Optimizer/Builder/Runtime/RuntimeCallTestBase.h
@@ -25,11 +25,12 @@ struct RuntimeCallTest : public testing::Test {
// Set up a Module with a dummy function operation inside.
// Set the insertion point in the function entry block.
mlir::ModuleOp mod = builder.create<mlir::ModuleOp>(loc);
+ moduleOp = mod;
mlir::func::FuncOp func =
mlir::func::FuncOp::create(loc, "runtime_unit_tests_func",
builder.getFunctionType(std::nullopt, std::nullopt));
auto *entryBlock = func.addEntryBlock();
- mod.push_back(mod);
+ mod.push_back(func);
builder.setInsertionPointToStart(entryBlock);
kindMap = std::make_unique<fir::KindMapping>(&context);
@@ -66,6 +67,7 @@ struct RuntimeCallTest : public testing::Test {
}
mlir::MLIRContext context;
+ mlir::OwningOpRef<mlir::ModuleOp> moduleOp;
std::unique_ptr<fir::KindMapping> kindMap;
std::unique_ptr<fir::FirOpBuilder> firBuilder;
diff --git a/flang/unittests/Optimizer/FortranVariableTest.cpp b/flang/unittests/Optimizer/FortranVariableTest.cpp
index 87efb624735cfd..c3165dc2c08cb0 100644
--- a/flang/unittests/Optimizer/FortranVariableTest.cpp
+++ b/flang/unittests/Optimizer/FortranVariableTest.cpp
@@ -20,11 +20,12 @@ struct FortranVariableTest : public testing::Test {
// Set up a Module with a dummy function operation inside.
// Set the insertion point in the function entry block.
mlir::ModuleOp mod = builder->create<mlir::ModuleOp>(loc);
+ moduleOp = mod;
mlir::func::FuncOp func =
mlir::func::FuncOp::create(loc, "fortran_variable_tests",
builder->getFunctionType(std::nullopt, std::nullopt));
auto *entryBlock = func.addEntryBlock();
- mod.push_back(mod);
+ mod.push_back(func);
builder->setInsertionPointToStart(entryBlock);
}
@@ -40,6 +41,7 @@ struct FortranVariableTest : public testing::Test {
}
mlir::MLIRContext context;
std::unique_ptr<mlir::OpBuilder> builder;
+ mlir::OwningOpRef<mlir::ModuleOp> moduleOp;
};
TEST_F(FortranVariableTest, SimpleScalar) {
diff --git a/flang/unittests/Runtime/ArrayConstructor.cpp b/flang/unittests/Runtime/ArrayConstructor.cpp
index 62e3b780a27e72..53774a0eea07d6 100644
--- a/flang/unittests/Runtime/ArrayConstructor.cpp
+++ b/flang/unittests/Runtime/ArrayConstructor.cpp
@@ -127,6 +127,9 @@ TEST(ArrayConstructor, Character) {
0);
result.Deallocate();
cookieAllocator.deallocate(acVector, 1);
+ x->Deallocate();
+ y->Deallocate();
+ c->Deallocate();
}
TEST(ArrayConstructor, CharacterRuntimeCheck) {
diff --git a/flang/unittests/Runtime/CharacterTest.cpp b/flang/unittests/Runtime/CharacterTest.cpp
index e54fd8a5075f64..d462c9120fd8c7 100644
--- a/flang/unittests/Runtime/CharacterTest.cpp
+++ b/flang/unittests/Runtime/CharacterTest.cpp
@@ -259,6 +259,9 @@ void RunExtremumTests(const char *which,
t.expect[i], t.expect[i] + std::strlen(t.expect[i])};
EXPECT_EQ(expect, got) << "inputs: '" << t.x[i] << "','" << t.y[i] << "'";
}
+
+ x->Deallocate();
+ y->Deallocate();
}
}
More information about the flang-commits
mailing list