Skip to content

Commit d0a5f61

Browse files
committed
[clang] Support -clear-ast-before-backend without -disable-free
Previously without -disable-free, -clear-ast-before-backend would crash in ~ASTContext() due to various reasons. This works around that by doing a lot of the cleanup ahead of the destructor so that the destructor doesn't actually do any manual cleanup if we've already cleaned up beforehand. This actually does save a measurable amount of memory with -clear-ast-before-backend, although at an almost unnoticeable runtime cost: https://llvm-compile-time-tracker.com/compare.php?from=5d755b32f2775b9219f6d6e2feda5e1417dc993b&to=58ef1c7ad7e2ad45f9c97597905a8cf05a26258c&stat=max-rss Previously we weren't doing any cleanup with -disable-free, so I tried measuring the impact of always doing the cleanup and didn't measure anything noticeable on llvm-compile-time-tracker. Reviewed By: dblaikie Differential Revision: https://reviews.llvm.org/D111767
1 parent 21abe21 commit d0a5f61

File tree

4 files changed

+24
-4
lines changed

4 files changed

+24
-4
lines changed

clang/include/clang/AST/ASTContext.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -695,6 +695,12 @@ class ASTContext : public RefCountedBase<ASTContext> {
695695
SourceManager& getSourceManager() { return SourceMgr; }
696696
const SourceManager& getSourceManager() const { return SourceMgr; }
697697

698+
// Cleans up some of the data structures. This allows us to do cleanup
699+
// normally done in the destructor earlier. Renders much of the ASTContext
700+
// unusable, mostly the actual AST nodes, so should be called when we no
701+
// longer need access to the AST.
702+
void cleanup();
703+
698704
llvm::BumpPtrAllocator &getAllocator() const {
699705
return BumpAlloc;
700706
}

clang/lib/AST/ASTContext.cpp

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -990,14 +990,15 @@ ASTContext::ASTContext(LangOptions &LOpts, SourceManager &SM,
990990
addTranslationUnitDecl();
991991
}
992992

993-
ASTContext::~ASTContext() {
993+
void ASTContext::cleanup() {
994994
// Release the DenseMaps associated with DeclContext objects.
995995
// FIXME: Is this the ideal solution?
996996
ReleaseDeclContextMaps();
997997

998998
// Call all of the deallocation functions on all of their targets.
999999
for (auto &Pair : Deallocations)
10001000
(Pair.first)(Pair.second);
1001+
Deallocations.clear();
10011002

10021003
// ASTRecordLayout objects in ASTRecordLayouts must always be destroyed
10031004
// because they can contain DenseMaps.
@@ -1007,23 +1008,29 @@ ASTContext::~ASTContext() {
10071008
// Increment in loop to prevent using deallocated memory.
10081009
if (auto *R = const_cast<ASTRecordLayout *>((I++)->second))
10091010
R->Destroy(*this);
1011+
ObjCLayouts.clear();
10101012

10111013
for (llvm::DenseMap<const RecordDecl*, const ASTRecordLayout*>::iterator
10121014
I = ASTRecordLayouts.begin(), E = ASTRecordLayouts.end(); I != E; ) {
10131015
// Increment in loop to prevent using deallocated memory.
10141016
if (auto *R = const_cast<ASTRecordLayout *>((I++)->second))
10151017
R->Destroy(*this);
10161018
}
1019+
ASTRecordLayouts.clear();
10171020

10181021
for (llvm::DenseMap<const Decl*, AttrVec*>::iterator A = DeclAttrs.begin(),
10191022
AEnd = DeclAttrs.end();
10201023
A != AEnd; ++A)
10211024
A->second->~AttrVec();
1025+
DeclAttrs.clear();
10221026

10231027
for (const auto &Value : ModuleInitializers)
10241028
Value.second->~PerModuleInitializers();
1029+
ModuleInitializers.clear();
10251030
}
10261031

1032+
ASTContext::~ASTContext() { cleanup(); }
1033+
10271034
void ASTContext::setTraversalScope(const std::vector<Decl *> &TopLevelDecls) {
10281035
TraversalScope = TopLevelDecls;
10291036
getParentMapContext().clear();

clang/lib/AST/DeclBase.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1962,6 +1962,7 @@ void ASTContext::ReleaseDeclContextMaps() {
19621962
// pointer because the subclass doesn't add anything that needs to
19631963
// be deleted.
19641964
StoredDeclsMap::DestroyAll(LastSDM.getPointer(), LastSDM.getInt());
1965+
LastSDM.setPointer(nullptr);
19651966
}
19661967

19671968
void StoredDeclsMap::DestroyAll(StoredDeclsMap *Map, bool Dependent) {

clang/lib/CodeGen/CodeGenAction.cpp

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -351,10 +351,16 @@ namespace clang {
351351
}
352352
}
353353

354-
// FIXME: Fix cleanup issues with clearing the AST when we properly free
355-
// things.
356-
if (CodeGenOpts.DisableFree && CodeGenOpts.ClearASTBeforeBackend)
354+
if (CodeGenOpts.ClearASTBeforeBackend) {
355+
// Access to the AST is no longer available after this.
356+
// Other things that the ASTContext manages are still available, e.g.
357+
// the SourceManager. It'd be nice if we could separate out all the
358+
// things in ASTContext used after this point and null out the
359+
// ASTContext, but too many various parts of the ASTContext are still
360+
// used in various parts.
361+
C.cleanup();
357362
C.getAllocator().Reset();
363+
}
358364

359365
EmbedBitcode(getModule(), CodeGenOpts, llvm::MemoryBufferRef());
360366

0 commit comments

Comments
 (0)