Skip to content

Commit db9ee7c

Browse files
committed
Fix a memory leak in FSO
Make sure the destructor of the SmallVector in ProjectionTreeNode gets called when the BumpPtrAllocator is destroy'ed.
1 parent fb3eb0b commit db9ee7c

File tree

4 files changed

+34
-29
lines changed

4 files changed

+34
-29
lines changed

include/swift/SIL/Projection.h

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -808,7 +808,8 @@ class ProjectionTree {
808808

809809
SILModule &Mod;
810810

811-
llvm::BumpPtrAllocator &Allocator;
811+
/// The allocator we use to allocate ProjectionTreeNodes in the tree.
812+
llvm::SpecificBumpPtrAllocator<ProjectionTreeNode> Allocator;
812813

813814
// A common pattern is a 3 field struct.
814815
llvm::SmallVector<ProjectionTreeNode *, 4> ProjectionTreeNodes;
@@ -818,12 +819,10 @@ class ProjectionTree {
818819

819820
public:
820821
/// Construct a projection tree from BaseTy.
821-
ProjectionTree(SILModule &Mod, llvm::BumpPtrAllocator &Allocator,
822-
SILType BaseTy);
822+
ProjectionTree(SILModule &Mod, SILType BaseTy);
823823
/// Construct an uninitialized projection tree, which can then be
824824
/// initialized by initializeWithExistingTree.
825-
ProjectionTree(SILModule &Mod, llvm::BumpPtrAllocator &Allocator)
826-
: Mod(Mod), Allocator(Allocator) {}
825+
ProjectionTree(SILModule &Mod) : Mod(Mod) {}
827826
~ProjectionTree();
828827
ProjectionTree(const ProjectionTree &) = delete;
829828
ProjectionTree(ProjectionTree &&) = default;
@@ -936,15 +935,16 @@ class ProjectionTree {
936935
void createRoot(SILType BaseTy) {
937936
assert(ProjectionTreeNodes.empty() &&
938937
"Should only create root when ProjectionTreeNodes is empty");
939-
auto *Node = new (Allocator) ProjectionTreeNode(BaseTy);
938+
auto *Node = new (Allocator.Allocate()) ProjectionTreeNode(BaseTy);
940939
ProjectionTreeNodes.push_back(Node);
941940
}
942941

943942
ProjectionTreeNode *createChild(ProjectionTreeNode *Parent,
944943
SILType BaseTy,
945944
const Projection &P) {
946945
unsigned Index = ProjectionTreeNodes.size();
947-
auto *Node = new (Allocator) ProjectionTreeNode(Parent, Index, BaseTy, P);
946+
auto *Node = new (Allocator.Allocate()) ProjectionTreeNode(Parent, Index,
947+
BaseTy, P);
948948
ProjectionTreeNodes.push_back(Node);
949949
return ProjectionTreeNodes[Index];
950950
}

include/swift/SILOptimizer/Utils/FunctionSignatureOptUtils.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,13 +76,13 @@ struct ArgumentDescriptor {
7676
/// to the original argument. The reason why we do this is to make sure we
7777
/// have access to the original argument's state if we modify the argument
7878
/// when optimizing.
79-
ArgumentDescriptor(llvm::BumpPtrAllocator &BPA, SILArgument *A)
79+
ArgumentDescriptor(SILArgument *A)
8080
: Arg(A), PInfo(Arg->getKnownParameterInfo()), Index(A->getIndex()),
8181
Decl(A->getDecl()), IsEntirelyDead(false), Explode(false),
8282
OwnedToGuaranteed(false),
8383
IsIndirectResult(A->isIndirectResult()),
8484
CalleeRelease(), CalleeReleaseInThrowBlock(),
85-
ProjTree(A->getModule(), BPA, A->getType()) {}
85+
ProjTree(A->getModule(), A->getType()) {}
8686

8787
ArgumentDescriptor(const ArgumentDescriptor &) = delete;
8888
ArgumentDescriptor(ArgumentDescriptor &&) = default;

lib/SIL/Projection.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1142,8 +1142,7 @@ class NewAggregateBuilderMap {
11421142
//===----------------------------------------------------------------------===//
11431143

11441144
ProjectionTree::
1145-
ProjectionTree(SILModule &Mod, llvm::BumpPtrAllocator &BPA, SILType BaseTy)
1146-
: Mod(Mod), Allocator(BPA) {
1145+
ProjectionTree(SILModule &Mod, SILType BaseTy) : Mod(Mod) {
11471146
DEBUG(llvm::dbgs() << "Constructing Projection Tree For : " << BaseTy);
11481147

11491148
// Create the root node of the tree with our base type.
@@ -1161,7 +1160,8 @@ void
11611160
ProjectionTree::initializeWithExistingTree(const ProjectionTree &PT) {
11621161
LiveLeafIndices = PT.LiveLeafIndices;
11631162
for (const auto &N : PT.ProjectionTreeNodes) {
1164-
ProjectionTreeNodes.push_back(new (Allocator) ProjectionTreeNode(*N));
1163+
ProjectionTreeNodes.push_back(new (Allocator.Allocate())
1164+
ProjectionTreeNode(*N));
11651165
}
11661166
}
11671167

lib/SILOptimizer/Transforms/FunctionSignatureOpts.cpp

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,9 @@
1212
///
1313
/// \file
1414
///
15-
/// This pass defines function signature related optimizations. Everytime a
16-
/// function signature optimization is performed, changes are made to the
17-
/// original function and after all function signature optimizations are
15+
/// This pass defines function signature related optimizations.
16+
/// When a function signature optimization is performed, changes are made to
17+
/// the original function and after all function signature optimizations are
1818
/// finished, a new function is created and the old function is turned into
1919
/// a thunk.
2020
///
@@ -164,8 +164,9 @@ class FunctionSignatureTransform {
164164
/// ----------------------------------------------------------///
165165
/// Find any dead argument opportunities.
166166
bool DeadArgumentAnalyzeParameters();
167-
/// Do the actual dead argument transformations.
168-
void DeadArgumentTransformParameters();
167+
/// Modify the current function so that later function signature analysis
168+
/// are more effective.
169+
void DeadArgumentTransformFunction();
169170
/// Remove the dead argument once the new function is created.
170171
void DeadArgumentFinalizeOptimizedFunction();
171172

@@ -174,8 +175,11 @@ class FunctionSignatureTransform {
174175
/// ----------------------------------------------------------///
175176
bool OwnedToGuaranteedAnalyzeResults();
176177
bool OwnedToGuaranteedAnalyzeParameters();
177-
void OwnedToGuaranteedTransformResults();
178-
void OwnedToGuaranteedTransformParameters();
178+
179+
/// Modify the current function so that later function signature analysis
180+
/// are more effective.
181+
void OwnedToGuaranteedTransformFunctionResults();
182+
void OwnedToGuaranteedTransformFunctionParameters();
179183

180184
/// Find any owned to guaranteed opportunities.
181185
bool OwnedToGuaranteedAnalyze() {
@@ -186,8 +190,8 @@ class FunctionSignatureTransform {
186190

187191
/// Do the actual owned to guaranteeed transformations.
188192
void OwnedToGuaranteedTransform() {
189-
OwnedToGuaranteedTransformResults();
190-
OwnedToGuaranteedTransformParameters();
193+
OwnedToGuaranteedTransformFunctionResults();
194+
OwnedToGuaranteedTransformFunctionParameters();
191195
}
192196

193197
/// Set up epilogue work for the thunk result based in the given argument.
@@ -274,7 +278,7 @@ class FunctionSignatureTransform {
274278
// already created a thunk.
275279
if ((hasCaller || Changed) && DeadArgumentAnalyzeParameters()) {
276280
Changed = true;
277-
DeadArgumentTransformParameters();
281+
DeadArgumentTransformFunction();
278282
}
279283

280284
// Run ArgumentExplosion transformation. We only specialize
@@ -551,7 +555,7 @@ bool FunctionSignatureTransform::DeadArgumentAnalyzeParameters() {
551555
return SignatureOptimize;
552556
}
553557

554-
void FunctionSignatureTransform::DeadArgumentTransformParameters() {
558+
void FunctionSignatureTransform::DeadArgumentTransformFunction() {
555559
SILBasicBlock *BB = &*F->begin();
556560
for (const ArgumentDescriptor &AD : ArgumentDescList) {
557561
if (!AD.IsEntirelyDead)
@@ -643,20 +647,22 @@ bool FunctionSignatureTransform::OwnedToGuaranteedAnalyzeResults() {
643647
return SignatureOptimize;
644648
}
645649

646-
void FunctionSignatureTransform::OwnedToGuaranteedTransformParameters() {
650+
void FunctionSignatureTransform::OwnedToGuaranteedTransformFunctionParameters() {
647651
// And remove all Callee releases that we found and made redundant via owned
648652
// to guaranteed conversion.
649653
for (const ArgumentDescriptor &AD : ArgumentDescList) {
650654
if (!AD.OwnedToGuaranteed)
651655
continue;
652-
for (auto &X : AD.CalleeRelease)
656+
for (auto &X : AD.CalleeRelease) {
653657
X->eraseFromParent();
654-
for (auto &X : AD.CalleeReleaseInThrowBlock)
658+
}
659+
for (auto &X : AD.CalleeReleaseInThrowBlock) {
655660
X->eraseFromParent();
661+
}
656662
}
657663
}
658664

659-
void FunctionSignatureTransform::OwnedToGuaranteedTransformResults() {
665+
void FunctionSignatureTransform::OwnedToGuaranteedTransformFunctionResults() {
660666
// And remove all callee retains that we found and made redundant via owned
661667
// to unowned conversion.
662668
for (const ResultDescriptor &RD : ResultDescList) {
@@ -852,7 +858,6 @@ class FunctionSignatureOpts : public SILFunctionTransform {
852858
if (!canSpecializeFunction(F))
853859
return;
854860

855-
llvm::BumpPtrAllocator BPA;
856861
auto *AA = PM->getAnalysis<AliasAnalysis>();
857862
auto *RCIA = getAnalysis<RCIdentityAnalysis>();
858863

@@ -876,7 +881,7 @@ class FunctionSignatureOpts : public SILFunctionTransform {
876881
llvm::SmallVector<ResultDescriptor, 4> ResultDescList;
877882
ArrayRef<SILArgument *> Args = F->begin()->getBBArgs();
878883
for (unsigned i = 0, e = Args.size(); i != e; ++i) {
879-
ArgumentDescList.emplace_back(BPA, Args[i]);
884+
ArgumentDescList.emplace_back(Args[i]);
880885
}
881886
for (SILResultInfo IR : F->getLoweredFunctionType()->getAllResults()) {
882887
ResultDescList.emplace_back(IR);

0 commit comments

Comments
 (0)