Skip to content

Commit c870632

Browse files
matthias-springermatthias-springer
authored
[flang] Fix some memory leaks (#121050)
This commit fixes some but not all memory leaks in Flang. There are still 91 tests that fail with ASAN. - Use `mlir::OwningOpRef` instead of `std::unique_ptr`. The latter does not free allocations of nested blocks. - Pass `ModuleOp` as value instead of reference. - Add few missing deallocations in test cases and other places.
1 parent c29536b commit c870632

18 files changed

+85
-67
lines changed

‎flang/include/flang/Frontend/FrontendActions.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include "flang/Semantics/semantics.h"
2020

2121
#include "mlir/IR/BuiltinOps.h"
22+
#include "mlir/IR/OwningOpRef.h"
2223
#include "llvm/ADT/StringRef.h"
2324
#include "llvm/IR/Module.h"
2425
#include <memory>
@@ -215,8 +216,8 @@ class CodeGenAction : public FrontendAction {
215216
CodeGenAction(BackendActionTy act) : action{act} {};
216217
/// @name MLIR
217218
/// {
218-
std::unique_ptr<mlir::ModuleOp> mlirModule;
219219
std::unique_ptr<mlir::MLIRContext> mlirCtx;
220+
mlir::OwningOpRef<mlir::ModuleOp> mlirModule;
220221
/// }
221222

222223
/// @name LLVM IR

‎flang/include/flang/Lower/AbstractConverter.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ struct SymbolBox;
6262
namespace pft {
6363
struct Variable;
6464
struct FunctionLikeUnit;
65-
}
65+
} // namespace pft
6666

6767
using SomeExpr = Fortran::evaluate::Expr<Fortran::evaluate::SomeType>;
6868
using SymbolRef = Fortran::common::Reference<const Fortran::semantics::Symbol>;
@@ -295,7 +295,7 @@ class AbstractConverter {
295295
/// Get the OpBuilder
296296
virtual fir::FirOpBuilder &getFirOpBuilder() = 0;
297297
/// Get the ModuleOp
298-
virtual mlir::ModuleOp &getModuleOp() = 0;
298+
virtual mlir::ModuleOp getModuleOp() = 0;
299299
/// Get the MLIRContext
300300
virtual mlir::MLIRContext &getMLIRContext() = 0;
301301
/// Unique a symbol (add a containing scope specific prefix)

‎flang/include/flang/Lower/Bridge.h

+4-2
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include "flang/Optimizer/Builder/FIRBuilder.h"
2424
#include "flang/Optimizer/Dialect/Support/KindMapping.h"
2525
#include "mlir/IR/BuiltinOps.h"
26+
#include "mlir/IR/OwningOpRef.h"
2627
#include <set>
2728

2829
namespace llvm {
@@ -83,7 +84,8 @@ class LoweringBridge {
8384
mlir::MLIRContext &getMLIRContext() { return context; }
8485

8586
/// Get the ModuleOp. It can never be null, which is asserted in the ctor.
86-
mlir::ModuleOp &getModule() { return *module.get(); }
87+
mlir::ModuleOp getModule() { return *module; }
88+
mlir::ModuleOp getModuleAndRelease() { return module.release(); }
8789

8890
const Fortran::common::IntrinsicTypeDefaultKinds &getDefaultKinds() const {
8991
return defaultKinds;
@@ -166,7 +168,7 @@ class LoweringBridge {
166168
const Fortran::evaluate::TargetCharacteristics &targetCharacteristics;
167169
const Fortran::parser::AllCookedSources *cooked;
168170
mlir::MLIRContext &context;
169-
std::unique_ptr<mlir::ModuleOp> module;
171+
mlir::OwningOpRef<mlir::ModuleOp> module;
170172
fir::KindMapping &kindMap;
171173
const Fortran::lower::LoweringOptions &loweringOptions;
172174
const std::vector<Fortran::lower::EnvironmentDefault> &envDefaults;

‎flang/include/flang/Lower/OpenACC.h

+4-4
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ namespace llvm {
1919
template <typename T, unsigned N>
2020
class SmallVector;
2121
class StringRef;
22-
}
22+
} // namespace llvm
2323

2424
namespace mlir {
2525
class Location;
@@ -44,7 +44,7 @@ struct OpenACCRoutineConstruct;
4444
namespace semantics {
4545
class SemanticsContext;
4646
class Symbol;
47-
}
47+
} // namespace semantics
4848

4949
namespace lower {
5050

@@ -78,11 +78,11 @@ void genOpenACCDeclarativeConstruct(AbstractConverter &,
7878
AccRoutineInfoMappingList &);
7979
void genOpenACCRoutineConstruct(AbstractConverter &,
8080
Fortran::semantics::SemanticsContext &,
81-
mlir::ModuleOp &,
81+
mlir::ModuleOp,
8282
const parser::OpenACCRoutineConstruct &,
8383
AccRoutineInfoMappingList &);
8484

85-
void finalizeOpenACCRoutineAttachment(mlir::ModuleOp &,
85+
void finalizeOpenACCRoutineAttachment(mlir::ModuleOp,
8686
AccRoutineInfoMappingList &);
8787

8888
/// Get a acc.private.recipe op for the given type or create it if it does not

‎flang/include/flang/Tools/CrossToolHelpers.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ struct OffloadModuleOpts {
174174
// Shares assinging of the OpenMP OffloadModuleInterface and its assorted
175175
// attributes accross Flang tools (bbc/flang)
176176
[[maybe_unused]] static void setOffloadModuleInterfaceAttributes(
177-
mlir::ModuleOp &module, OffloadModuleOpts Opts) {
177+
mlir::ModuleOp module, OffloadModuleOpts Opts) {
178178
// Should be registered by the OpenMPDialect
179179
if (auto offloadMod = llvm::dyn_cast<mlir::omp::OffloadModuleInterface>(
180180
module.getOperation())) {
@@ -198,7 +198,7 @@ struct OffloadModuleOpts {
198198
}
199199

200200
[[maybe_unused]] static void setOpenMPVersionAttribute(
201-
mlir::ModuleOp &module, int64_t version) {
201+
mlir::ModuleOp module, int64_t version) {
202202
module.getOperation()->setAttr(
203203
mlir::StringAttr::get(module.getContext(), llvm::Twine{"omp.version"}),
204204
mlir::omp::VersionAttr::get(module.getContext(), version));

‎flang/lib/Frontend/FrontendActions.cpp

+14-8
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ bool PrescanAndSemaDebugAction::beginSourceFileAction() {
149149
(runSemanticChecks() || true) && (generateRtTypeTables() || true);
150150
}
151151

152-
static void addDependentLibs(mlir::ModuleOp &mlirModule, CompilerInstance &ci) {
152+
static void addDependentLibs(mlir::ModuleOp mlirModule, CompilerInstance &ci) {
153153
const std::vector<std::string> &libs =
154154
ci.getInvocation().getCodeGenOpts().DependentLibs;
155155
if (libs.empty()) {
@@ -171,7 +171,7 @@ static void addDependentLibs(mlir::ModuleOp &mlirModule, CompilerInstance &ci) {
171171
// Add to MLIR code target specific items which are dependent on target
172172
// configuration specified by the user.
173173
// Clang equivalent function: AMDGPUTargetCodeGenInfo::emitTargetGlobals
174-
static void addAMDGPUSpecificMLIRItems(mlir::ModuleOp &mlirModule,
174+
static void addAMDGPUSpecificMLIRItems(mlir::ModuleOp mlirModule,
175175
CompilerInstance &ci) {
176176
const TargetOptions &targetOpts = ci.getInvocation().getTargetOpts();
177177
const llvm::Triple triple(targetOpts.triple);
@@ -269,7 +269,7 @@ bool CodeGenAction::beginSourceFileAction() {
269269
return false;
270270
}
271271

272-
mlirModule = std::make_unique<mlir::ModuleOp>(module.release());
272+
mlirModule = std::move(module);
273273
const llvm::DataLayout &dl = targetMachine.createDataLayout();
274274
fir::support::setMLIRDataLayout(*mlirModule, dl);
275275
return true;
@@ -303,21 +303,21 @@ bool CodeGenAction::beginSourceFileAction() {
303303
ci.getInvocation().getFrontendOpts().features, targetMachine,
304304
ci.getInvocation().getTargetOpts(), ci.getInvocation().getCodeGenOpts());
305305

306-
// Fetch module from lb, so we can set
307-
mlirModule = std::make_unique<mlir::ModuleOp>(lb.getModule());
308-
309306
if (ci.getInvocation().getFrontendOpts().features.IsEnabled(
310307
Fortran::common::LanguageFeature::OpenMP)) {
311-
setOffloadModuleInterfaceAttributes(*mlirModule,
308+
setOffloadModuleInterfaceAttributes(lb.getModule(),
312309
ci.getInvocation().getLangOpts());
313-
setOpenMPVersionAttribute(*mlirModule,
310+
setOpenMPVersionAttribute(lb.getModule(),
314311
ci.getInvocation().getLangOpts().OpenMPVersion);
315312
}
316313

317314
// Create a parse tree and lower it to FIR
318315
Fortran::parser::Program &parseTree{*ci.getParsing().parseTree()};
319316
lb.lower(parseTree, ci.getSemanticsContext());
320317

318+
// Fetch module from lb, so we can set
319+
mlirModule = lb.getModuleAndRelease();
320+
321321
// Add target specific items like dependent libraries, target specific
322322
// constants etc.
323323
addDependentLibs(*mlirModule, ci);
@@ -961,6 +961,9 @@ static void generateMachineCodeOrAssemblyImpl(clang::DiagnosticsEngine &diags,
961961

962962
// Run the passes
963963
codeGenPasses.run(llvmModule);
964+
965+
// Cleanup
966+
delete tlii;
964967
}
965968

966969
void CodeGenAction::runOptimizationPipeline(llvm::raw_pwrite_stream &os) {
@@ -1043,6 +1046,9 @@ void CodeGenAction::runOptimizationPipeline(llvm::raw_pwrite_stream &os) {
10431046

10441047
// Run the passes.
10451048
mpm.run(*llvmModule, mam);
1049+
1050+
// Cleanup
1051+
delete tlii;
10461052
}
10471053

10481054
// This class handles optimization remark messages requested if

‎flang/lib/Lower/Bridge.cpp

+12-16
Original file line numberDiff line numberDiff line change
@@ -1028,7 +1028,7 @@ class FirConverter : public Fortran::lower::AbstractConverter {
10281028

10291029
fir::FirOpBuilder &getFirOpBuilder() override final { return *builder; }
10301030

1031-
mlir::ModuleOp &getModuleOp() override final { return bridge.getModule(); }
1031+
mlir::ModuleOp getModuleOp() override final { return bridge.getModule(); }
10321032

10331033
mlir::MLIRContext &getMLIRContext() override final {
10341034
return bridge.getMLIRContext();
@@ -6137,10 +6137,7 @@ void Fortran::lower::LoweringBridge::lower(
61376137
}
61386138

61396139
void Fortran::lower::LoweringBridge::parseSourceFile(llvm::SourceMgr &srcMgr) {
6140-
mlir::OwningOpRef<mlir::ModuleOp> owningRef =
6141-
mlir::parseSourceFile<mlir::ModuleOp>(srcMgr, &context);
6142-
module.reset(new mlir::ModuleOp(owningRef.get().getOperation()));
6143-
owningRef.release();
6140+
module = mlir::parseSourceFile<mlir::ModuleOp>(srcMgr, &context);
61446141
}
61456142

61466143
Fortran::lower::LoweringBridge::LoweringBridge(
@@ -6207,19 +6204,18 @@ Fortran::lower::LoweringBridge::LoweringBridge(
62076204
};
62086205

62096206
// Create the module and attach the attributes.
6210-
module = std::make_unique<mlir::ModuleOp>(
6207+
module = mlir::OwningOpRef<mlir::ModuleOp>(
62116208
mlir::ModuleOp::create(getPathLocation()));
6212-
assert(module.get() && "module was not created");
6213-
fir::setTargetTriple(*module.get(), triple);
6214-
fir::setKindMapping(*module.get(), kindMap);
6215-
fir::setTargetCPU(*module.get(), targetMachine.getTargetCPU());
6216-
fir::setTuneCPU(*module.get(), targetOpts.cpuToTuneFor);
6217-
fir::setTargetFeatures(*module.get(), targetMachine.getTargetFeatureString());
6218-
fir::support::setMLIRDataLayout(*module.get(),
6219-
targetMachine.createDataLayout());
6220-
fir::setIdent(*module.get(), Fortran::common::getFlangFullVersion());
6209+
assert(*module && "module was not created");
6210+
fir::setTargetTriple(*module, triple);
6211+
fir::setKindMapping(*module, kindMap);
6212+
fir::setTargetCPU(*module, targetMachine.getTargetCPU());
6213+
fir::setTuneCPU(*module, targetOpts.cpuToTuneFor);
6214+
fir::setTargetFeatures(*module, targetMachine.getTargetFeatureString());
6215+
fir::support::setMLIRDataLayout(*module, targetMachine.createDataLayout());
6216+
fir::setIdent(*module, Fortran::common::getFlangFullVersion());
62216217
if (cgOpts.RecordCommandLine)
6222-
fir::setCommandline(*module.get(), *cgOpts.RecordCommandLine);
6218+
fir::setCommandline(*module, *cgOpts.RecordCommandLine);
62236219
}
62246220

62256221
void Fortran::lower::genCleanUpInRegionIfAny(

‎flang/lib/Lower/OpenACC.cpp

+8-6
Original file line numberDiff line numberDiff line change
@@ -2670,11 +2670,13 @@ static void genACCDataOp(Fortran::lower::AbstractConverter &converter,
26702670
asyncOnlyDeviceTypes);
26712671
attachEntryOperands.append(dataClauseOperands.begin() + crtDataStart,
26722672
dataClauseOperands.end());
2673-
} else if(const auto *defaultClause =
2674-
std::get_if<Fortran::parser::AccClause::Default>(&clause.u)) {
2673+
} else if (const auto *defaultClause =
2674+
std::get_if<Fortran::parser::AccClause::Default>(
2675+
&clause.u)) {
26752676
if ((defaultClause->v).v == llvm::acc::DefaultValue::ACC_Default_none)
26762677
hasDefaultNone = true;
2677-
else if ((defaultClause->v).v == llvm::acc::DefaultValue::ACC_Default_present)
2678+
else if ((defaultClause->v).v ==
2679+
llvm::acc::DefaultValue::ACC_Default_present)
26782680
hasDefaultPresent = true;
26792681
}
26802682
}
@@ -3830,7 +3832,7 @@ genDeclareInFunction(Fortran::lower::AbstractConverter &converter,
38303832

38313833
static void
38323834
genDeclareInModule(Fortran::lower::AbstractConverter &converter,
3833-
mlir::ModuleOp &moduleOp,
3835+
mlir::ModuleOp moduleOp,
38343836
const Fortran::parser::AccClauseList &accClauseList) {
38353837
mlir::OpBuilder modBuilder(moduleOp.getBodyRegion());
38363838
for (const Fortran::parser::AccClause &clause : accClauseList.v) {
@@ -3981,7 +3983,7 @@ static void attachRoutineInfo(mlir::func::FuncOp func,
39813983

39823984
void Fortran::lower::genOpenACCRoutineConstruct(
39833985
Fortran::lower::AbstractConverter &converter,
3984-
Fortran::semantics::SemanticsContext &semanticsContext, mlir::ModuleOp &mod,
3986+
Fortran::semantics::SemanticsContext &semanticsContext, mlir::ModuleOp mod,
39853987
const Fortran::parser::OpenACCRoutineConstruct &routineConstruct,
39863988
Fortran::lower::AccRoutineInfoMappingList &accRoutineInfos) {
39873989
fir::FirOpBuilder &builder = converter.getFirOpBuilder();
@@ -4139,7 +4141,7 @@ void Fortran::lower::genOpenACCRoutineConstruct(
41394141
}
41404142

41414143
void Fortran::lower::finalizeOpenACCRoutineAttachment(
4142-
mlir::ModuleOp &mod,
4144+
mlir::ModuleOp mod,
41434145
Fortran::lower::AccRoutineInfoMappingList &accRoutineInfos) {
41444146
for (auto &mapping : accRoutineInfos) {
41454147
mlir::func::FuncOp funcOp =

‎flang/lib/Optimizer/CodeGen/CodeGen.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -3087,7 +3087,7 @@ struct GlobalOpConversion : public fir::FIROpConversion<fir::GlobalOp> {
30873087
private:
30883088
static void addComdat(mlir::LLVM::GlobalOp &global,
30893089
mlir::ConversionPatternRewriter &rewriter,
3090-
mlir::ModuleOp &module) {
3090+
mlir::ModuleOp module) {
30913091
const char *comdatName = "__llvm_comdat";
30923092
mlir::LLVM::ComdatOp comdatOp =
30933093
module.lookupSymbol<mlir::LLVM::ComdatOp>(comdatName);

‎flang/unittests/Frontend/CodeGenActionTest.cpp

+1-2
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,7 @@ class LLVMConversionFailureCodeGenAction : public CodeGenAction {
7272
mlirCtx->loadDialect<test::DummyDialect>();
7373

7474
mlir::Location loc(mlir::UnknownLoc::get(mlirCtx.get()));
75-
mlirModule =
76-
std::make_unique<mlir::ModuleOp>(mlir::ModuleOp::create(loc, "mod"));
75+
mlirModule = mlir::ModuleOp::create(loc, "mod");
7776

7877
mlir::OpBuilder builder(mlirCtx.get());
7978
builder.setInsertionPointToStart(&mlirModule->getRegion().front());

‎flang/unittests/Optimizer/Builder/CharacterTest.cpp

+5-4
Original file line numberDiff line numberDiff line change
@@ -26,19 +26,20 @@ struct CharacterTest : public testing::Test {
2626

2727
// Set up a Module with a dummy function operation inside.
2828
// Set the insertion point in the function entry block.
29-
mlir::ModuleOp mod = builder.create<mlir::ModuleOp>(loc);
30-
mlir::func::FuncOp func = mlir::func::FuncOp::create(
29+
moduleOp = builder.create<mlir::ModuleOp>(loc);
30+
builder.setInsertionPointToStart(moduleOp->getBody());
31+
mlir::func::FuncOp func = builder.create<mlir::func::FuncOp>(
3132
loc, "func1", builder.getFunctionType(std::nullopt, std::nullopt));
3233
auto *entryBlock = func.addEntryBlock();
33-
mod.push_back(mod);
3434
builder.setInsertionPointToStart(entryBlock);
3535

36-
firBuilder = std::make_unique<fir::FirOpBuilder>(mod, *kindMap);
36+
firBuilder = std::make_unique<fir::FirOpBuilder>(builder, *kindMap);
3737
}
3838

3939
fir::FirOpBuilder &getBuilder() { return *firBuilder; }
4040

4141
mlir::MLIRContext context;
42+
mlir::OwningOpRef<mlir::ModuleOp> moduleOp;
4243
std::unique_ptr<fir::KindMapping> kindMap;
4344
std::unique_ptr<fir::FirOpBuilder> firBuilder;
4445
};

‎flang/unittests/Optimizer/Builder/ComplexTest.cpp

+5-4
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,15 @@ struct ComplexTest : public testing::Test {
2222

2323
// Set up a Module with a dummy function operation inside.
2424
// Set the insertion point in the function entry block.
25-
mlir::ModuleOp mod = builder.create<mlir::ModuleOp>(loc);
26-
mlir::func::FuncOp func = mlir::func::FuncOp::create(
25+
moduleOp = builder.create<mlir::ModuleOp>(loc);
26+
builder.setInsertionPointToStart(moduleOp->getBody());
27+
mlir::func::FuncOp func = builder.create<mlir::func::FuncOp>(
2728
loc, "func1", builder.getFunctionType(std::nullopt, std::nullopt));
2829
auto *entryBlock = func.addEntryBlock();
29-
mod.push_back(mod);
3030
builder.setInsertionPointToStart(entryBlock);
3131

3232
kindMap = std::make_unique<fir::KindMapping>(&context);
33-
firBuilder = std::make_unique<fir::FirOpBuilder>(mod, *kindMap);
33+
firBuilder = std::make_unique<fir::FirOpBuilder>(builder, *kindMap);
3434
helper = std::make_unique<fir::factory::Complex>(*firBuilder, loc);
3535

3636
// Init commonly used types
@@ -46,6 +46,7 @@ struct ComplexTest : public testing::Test {
4646
}
4747

4848
mlir::MLIRContext context;
49+
mlir::OwningOpRef<mlir::ModuleOp> moduleOp;
4950
std::unique_ptr<fir::KindMapping> kindMap;
5051
std::unique_ptr<fir::FirOpBuilder> firBuilder;
5152
std::unique_ptr<fir::factory::Complex> helper;

‎flang/unittests/Optimizer/Builder/FIRBuilderTest.cpp

+5-4
Original file line numberDiff line numberDiff line change
@@ -26,19 +26,20 @@ struct FIRBuilderTest : public testing::Test {
2626

2727
// Set up a Module with a dummy function operation inside.
2828
// Set the insertion point in the function entry block.
29-
mlir::ModuleOp mod = builder.create<mlir::ModuleOp>(loc);
30-
mlir::func::FuncOp func = mlir::func::FuncOp::create(
29+
moduleOp = builder.create<mlir::ModuleOp>(loc);
30+
builder.setInsertionPointToStart(moduleOp->getBody());
31+
mlir::func::FuncOp func = builder.create<mlir::func::FuncOp>(
3132
loc, "func1", builder.getFunctionType(std::nullopt, std::nullopt));
3233
auto *entryBlock = func.addEntryBlock();
33-
mod.push_back(mod);
3434
builder.setInsertionPointToStart(entryBlock);
3535

36-
firBuilder = std::make_unique<fir::FirOpBuilder>(mod, kindMap);
36+
firBuilder = std::make_unique<fir::FirOpBuilder>(builder, kindMap);
3737
}
3838

3939
fir::FirOpBuilder &getBuilder() { return *firBuilder; }
4040

4141
mlir::MLIRContext context;
42+
mlir::OwningOpRef<mlir::ModuleOp> moduleOp;
4243
std::unique_ptr<fir::FirOpBuilder> firBuilder;
4344
};
4445

0 commit comments

Comments
 (0)