-
Notifications
You must be signed in to change notification settings - Fork 12.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[flang] Fix some memory leaks #121050
Merged
Merged
[flang] Fix some memory leaks #121050
+85
−67
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
flang:driver
flang
Flang issues not falling into any other category
flang:fir-hlfir
openacc
flang:codegen
labels
Dec 24, 2024
@llvm/pr-subscribers-openacc @llvm/pr-subscribers-flang-fir-hlfir Author: Matthias Springer (matthias-springer) ChangesFull diff: https://github.com/llvm/llvm-project/pull/121050.diff 14 Files Affected:
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();
}
}
|
matthias-springer
force-pushed
the
users/matthias-springer/flang_mem_1
branch
from
December 24, 2024 12:46
c702bf4
to
c36385e
Compare
matthias-springer
requested review from
clementval,
agozillon and
jeanPerier
December 24, 2024 12:48
clementval
approved these changes
Dec 25, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
flang:codegen
flang:driver
flang:fir-hlfir
flang
Flang issues not falling into any other category
openacc
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit fixes some but not all memory leaks in Flang. There are still 91 tests that fail with ASAN.
mlir::OwningOpRef
instead ofstd::unique_ptr
. The latter does not free allocations of nested blocks.ModuleOp
as value instead of reference.