Author: Arthur Eubanks Date: 2021-10-14T13:43:53-07:00 New Revision: d0a5f61c4f6fccec87fd5207e3fcd9502dd59854
URL: https://github.com/llvm/llvm-project/commit/d0a5f61c4f6fccec87fd5207e3fcd9502dd59854 DIFF: https://github.com/llvm/llvm-project/commit/d0a5f61c4f6fccec87fd5207e3fcd9502dd59854.diff LOG: [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 Added: Modified: clang/include/clang/AST/ASTContext.h clang/lib/AST/ASTContext.cpp clang/lib/AST/DeclBase.cpp clang/lib/CodeGen/CodeGenAction.cpp Removed: ################################################################################ diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h index b21f9b83b9d86..b9aa2d2cfadb5 100644 --- a/clang/include/clang/AST/ASTContext.h +++ b/clang/include/clang/AST/ASTContext.h @@ -695,6 +695,12 @@ class ASTContext : public RefCountedBase<ASTContext> { SourceManager& getSourceManager() { return SourceMgr; } const SourceManager& getSourceManager() const { return SourceMgr; } + // Cleans up some of the data structures. This allows us to do cleanup + // normally done in the destructor earlier. Renders much of the ASTContext + // unusable, mostly the actual AST nodes, so should be called when we no + // longer need access to the AST. + void cleanup(); + llvm::BumpPtrAllocator &getAllocator() const { return BumpAlloc; } diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index a6b11465d4bcd..d9017b347d812 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -990,7 +990,7 @@ ASTContext::ASTContext(LangOptions &LOpts, SourceManager &SM, addTranslationUnitDecl(); } -ASTContext::~ASTContext() { +void ASTContext::cleanup() { // Release the DenseMaps associated with DeclContext objects. // FIXME: Is this the ideal solution? ReleaseDeclContextMaps(); @@ -998,6 +998,7 @@ ASTContext::~ASTContext() { // Call all of the deallocation functions on all of their targets. for (auto &Pair : Deallocations) (Pair.first)(Pair.second); + Deallocations.clear(); // ASTRecordLayout objects in ASTRecordLayouts must always be destroyed // because they can contain DenseMaps. @@ -1007,6 +1008,7 @@ ASTContext::~ASTContext() { // Increment in loop to prevent using deallocated memory. if (auto *R = const_cast<ASTRecordLayout *>((I++)->second)) R->Destroy(*this); + ObjCLayouts.clear(); for (llvm::DenseMap<const RecordDecl*, const ASTRecordLayout*>::iterator I = ASTRecordLayouts.begin(), E = ASTRecordLayouts.end(); I != E; ) { @@ -1014,16 +1016,21 @@ ASTContext::~ASTContext() { if (auto *R = const_cast<ASTRecordLayout *>((I++)->second)) R->Destroy(*this); } + ASTRecordLayouts.clear(); for (llvm::DenseMap<const Decl*, AttrVec*>::iterator A = DeclAttrs.begin(), AEnd = DeclAttrs.end(); A != AEnd; ++A) A->second->~AttrVec(); + DeclAttrs.clear(); for (const auto &Value : ModuleInitializers) Value.second->~PerModuleInitializers(); + ModuleInitializers.clear(); } +ASTContext::~ASTContext() { cleanup(); } + void ASTContext::setTraversalScope(const std::vector<Decl *> &TopLevelDecls) { TraversalScope = TopLevelDecls; getParentMapContext().clear(); diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp index e042ae8dae4ae..4044404f74ef2 100644 --- a/clang/lib/AST/DeclBase.cpp +++ b/clang/lib/AST/DeclBase.cpp @@ -1962,6 +1962,7 @@ void ASTContext::ReleaseDeclContextMaps() { // pointer because the subclass doesn't add anything that needs to // be deleted. StoredDeclsMap::DestroyAll(LastSDM.getPointer(), LastSDM.getInt()); + LastSDM.setPointer(nullptr); } void StoredDeclsMap::DestroyAll(StoredDeclsMap *Map, bool Dependent) { diff --git a/clang/lib/CodeGen/CodeGenAction.cpp b/clang/lib/CodeGen/CodeGenAction.cpp index 4ca34db06ced4..881e30a4c8444 100644 --- a/clang/lib/CodeGen/CodeGenAction.cpp +++ b/clang/lib/CodeGen/CodeGenAction.cpp @@ -351,10 +351,16 @@ namespace clang { } } - // FIXME: Fix cleanup issues with clearing the AST when we properly free - // things. - if (CodeGenOpts.DisableFree && CodeGenOpts.ClearASTBeforeBackend) + if (CodeGenOpts.ClearASTBeforeBackend) { + // Access to the AST is no longer available after this. + // Other things that the ASTContext manages are still available, e.g. + // the SourceManager. It'd be nice if we could separate out all the + // things in ASTContext used after this point and null out the + // ASTContext, but too many various parts of the ASTContext are still + // used in various parts. + C.cleanup(); C.getAllocator().Reset(); + } EmbedBitcode(getModule(), CodeGenOpts, llvm::MemoryBufferRef()); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits