Author: Steffen Larsen Date: 2026-10-20T08:17:56Z New Revision: 1e3163959b4845f3ed6d76acf2fc561b072b10e2
URL: https://github.com/llvm/llvm-project/commit/1e3163959b4845f3ed6d76acf2fc561b072b10e2 DIFF: https://github.com/llvm/llvm-project/commit/1e3163959b4845f3ed6d76acf2fc561b072b10e2.diff LOG: [Clang][AST][NFCI] Make Decl reference attributes directly (#219138) This commit changes `Decl` attributes from being stored in a map to being managed by an allocator and letting the individual `Decl` objects reference the attributes directly. Doing so reduces the additional complexity of maintaining a map and improves performance by reducing the levels of directions used when accessing `Decl` attributes. As an additional benefit, this removes the need for the `hasAttr` field as the pointer can be used to indicate whether a `Decl` has attributes or not. This coincidentally preserves the size of `Decl` on most systems, as the single-bit `hasAttr` field caused additional padding for the class. Assisted-by: Claude Opus 5 --------- Signed-off-by: Steffen Holst Larsen <[email protected]> Added: Modified: clang/include/clang/AST/ASTContext.h clang/include/clang/AST/DeclBase.h clang/lib/AST/ASTContext.cpp clang/lib/AST/DeclBase.cpp llvm/include/llvm/Support/Allocator.h Removed: ################################################################################ diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h index 467f0d7bd0af9..9479545b68814 100644 --- a/clang/include/clang/AST/ASTContext.h +++ b/clang/include/clang/AST/ASTContext.h @@ -549,15 +549,11 @@ class ASTContext : public RefCountedBase<ASTContext> { llvm::DenseMap<const CXXConstructorDecl *, ArrayRef<CXXDefaultArgExpr *>> CtorClosureDefaultArgs; - /// Keeps track of all declaration attributes. + /// Storage for the AttrVecs pointed to by Decl::Attrs. /// - /// Since so few decls have attrs, we keep them in a hash map instead of - /// wasting space in the Decl class. - llvm::DenseMap<const Decl*, AttrVec*> DeclAttrs; - - /// One-entry cache for getDeclAttrs(). - const Decl *LastDeclAttrsDecl = nullptr; - AttrVec *LastDeclAttrs = nullptr; + /// A SpecificBumpPtrAllocator so that the vectors are destroyed when the + /// context dies without having to track them individually. + llvm::SpecificBumpPtrAllocator<AttrVec> DeclAttrAllocator; /// A mapping from non-redeclarable declarations in modules that were /// merged with other declarations to the canonical declaration that they were @@ -1196,11 +1192,8 @@ class ASTContext : public RefCountedBase<ASTContext> { return CommentCommandTraits; } - /// Retrieve the attributes for the given declaration. - AttrVec& getDeclAttrs(const Decl *D); - - /// Erase the attributes corresponding to the given declaration. - void eraseDeclAttrs(const Decl *D); + /// Allocate an empty attribute vector with the lifetime of this context. + AttrVec *allocateAttrVec(); ArrayRef<CXXDefaultArgExpr *> getCtorClosureDefaultArgs(const CXXConstructorDecl *CD); diff --git a/clang/include/clang/AST/DeclBase.h b/clang/include/clang/AST/DeclBase.h index 3ca8603b73d21..2166f9845a9b0 100644 --- a/clang/include/clang/AST/DeclBase.h +++ b/clang/include/clang/AST/DeclBase.h @@ -297,10 +297,6 @@ class alignas(8) Decl { LLVM_PREFERRED_TYPE(bool) unsigned InvalidDecl : 1; - /// HasAttrs - This indicates whether the decl has attributes or not. - LLVM_PREFERRED_TYPE(bool) - unsigned HasAttrs : 1; - /// Implicit - Whether this declaration was implicitly generated by /// the implementation rather than explicitly written by the user. LLVM_PREFERRED_TYPE(bool) @@ -355,6 +351,13 @@ class alignas(8) Decl { LLVM_PREFERRED_TYPE(Linkage) mutable unsigned CacheValidAndLinkage : 3; + /// The attributes attached to this declaration, or null if it has none. + /// + /// This pointer is the sole record of whether the declaration has + /// attributes, so it must be cleared whenever the vector becomes empty. + /// Owned by the ASTContext that allocated it. + AttrVec *Attrs = nullptr; + /// Allocate memory for a deserialized declaration. /// /// This routine must be used to allocate memory for any declaration that is @@ -399,19 +402,18 @@ class alignas(8) Decl { protected: Decl(Kind DK, DeclContext *DC, SourceLocation L) : NextInContextAndBits(nullptr, getModuleOwnershipKindForChildOf(DC)), - DeclCtx(DC), Loc(L), DeclKind(DK), InvalidDecl(false), HasAttrs(false), - Implicit(false), Used(false), Referenced(false), - TopLevelDeclInObjCContainer(false), Access(AS_none), FromASTFile(0), + DeclCtx(DC), Loc(L), DeclKind(DK), InvalidDecl(false), Implicit(false), + Used(false), Referenced(false), TopLevelDeclInObjCContainer(false), + Access(AS_none), FromASTFile(0), IdentifierNamespace(getIdentifierNamespaceForKind(DK)), CacheValidAndLinkage(llvm::to_underlying(Linkage::Invalid)) { if (StatisticsEnabled) add(DK); } Decl(Kind DK, EmptyShell Empty) - : DeclKind(DK), InvalidDecl(false), HasAttrs(false), Implicit(false), - Used(false), Referenced(false), TopLevelDeclInObjCContainer(false), - Access(AS_none), FromASTFile(0), - IdentifierNamespace(getIdentifierNamespaceForKind(DK)), + : DeclKind(DK), InvalidDecl(false), Implicit(false), Used(false), + Referenced(false), TopLevelDeclInObjCContainer(false), Access(AS_none), + FromASTFile(0), IdentifierNamespace(getIdentifierNamespaceForKind(DK)), CacheValidAndLinkage(llvm::to_underlying(Linkage::Invalid)) { if (StatisticsEnabled) add(DK); } @@ -523,7 +525,7 @@ class alignas(8) Decl { return AccessSpecifier(Access); } - bool hasAttrs() const { return HasAttrs; } + bool hasAttrs() const { return Attrs != nullptr; } void setAttrs(const AttrVec& Attrs) { return setAttrsImpl(Attrs, getASTContext()); @@ -552,13 +554,13 @@ class alignas(8) Decl { } template <typename... Ts> void dropAttrs() { - if (!HasAttrs) return; + if (!Attrs) + return; - AttrVec &Vec = getAttrs(); - llvm::erase_if(Vec, [](Attr *A) { return isa<Ts...>(A); }); + llvm::erase_if(*Attrs, [](Attr *A) { return isa<Ts...>(A); }); - if (Vec.empty()) - HasAttrs = false; + if (Attrs->empty()) + dropAttrs(); } template <typename T> void dropAttr() { dropAttrs<T>(); } diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index da5eb61e48a5e..4225d7dc013e2 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -979,13 +979,6 @@ void ASTContext::cleanup() { } ASTRecordLayouts.clear(); - for (llvm::DenseMap<const Decl*, AttrVec*>::iterator A = DeclAttrs.begin(), - AEnd = DeclAttrs.end(); - A != AEnd; ++A) - A->second->~AttrVec(); - DeclAttrs.clear(); - LastDeclAttrsDecl = nullptr; - CtorClosureDefaultArgs.clear(); for (const auto &Value : ModuleInitializers) @@ -1535,33 +1528,8 @@ DiagnosticsEngine &ASTContext::getDiagnostics() const { return SourceMgr.getDiagnostics(); } -AttrVec& ASTContext::getDeclAttrs(const Decl *D) { - // 85% of lookups use the most recent D, so use a one-entry cache. - if (LastDeclAttrsDecl == D) { - assert(LastDeclAttrs != nullptr && LastDeclAttrs == DeclAttrs[D]); - return *LastDeclAttrs; - } - - AttrVec *&Result = DeclAttrs[D]; - if (!Result) { - void *Mem = Allocate(sizeof(AttrVec)); - Result = new (Mem) AttrVec; - } - - LastDeclAttrsDecl = D; - LastDeclAttrs = Result; - return *Result; -} - -/// Erase the attributes corresponding to the given declaration. -void ASTContext::eraseDeclAttrs(const Decl *D) { - llvm::DenseMap<const Decl*, AttrVec*>::iterator Pos = DeclAttrs.find(D); - if (Pos != DeclAttrs.end()) { - Pos->second->~AttrVec(); - DeclAttrs.erase(Pos); - } - if (LastDeclAttrsDecl == D) - LastDeclAttrsDecl = nullptr; +AttrVec *ASTContext::allocateAttrVec() { + return new (DeclAttrAllocator.Allocate()) AttrVec; } ArrayRef<CXXDefaultArgExpr *> @@ -13685,7 +13653,7 @@ size_t ASTContext::getSideTableAllocatedMemory() const { llvm::capacity_in_bytes(KeyFunctions) + llvm::capacity_in_bytes(ObjCImpls) + llvm::capacity_in_bytes(BlockVarCopyInits) + - llvm::capacity_in_bytes(DeclAttrs) + + DeclAttrAllocator.getTotalMemory() + llvm::capacity_in_bytes(TemplateOrInstantiation) + llvm::capacity_in_bytes(InstantiatedFromUsingDecl) + llvm::capacity_in_bytes(InstantiatedFromUsingShadowDecl) + diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp index 226229bcb622f..bf2a8a01f7b9c 100644 --- a/clang/lib/AST/DeclBase.cpp +++ b/clang/lib/AST/DeclBase.cpp @@ -1035,20 +1035,20 @@ unsigned Decl::getIdentifierNamespaceForKind(Kind DeclKind) { } void Decl::setAttrsImpl(const AttrVec &attrs, ASTContext &Ctx) { - assert(!HasAttrs && "Decl already contains attrs."); + assert(!Attrs && "Decl already contains attrs."); - AttrVec &AttrBlank = Ctx.getDeclAttrs(this); - assert(AttrBlank.empty() && "HasAttrs was wrong?"); - - AttrBlank = attrs; - HasAttrs = true; + Attrs = Ctx.allocateAttrVec(); + *Attrs = attrs; } void Decl::dropAttrs() { - if (!HasAttrs) return; + if (!Attrs) + return; - HasAttrs = false; - getASTContext().eraseDeclAttrs(this); + // The vector itself belongs to the ASTContext's allocator and is destroyed + // when the context dies; only the pointer is dropped here. + Attrs->clear(); + Attrs = nullptr; } void Decl::addAttr(Attr *A) { @@ -1075,8 +1075,8 @@ void Decl::addAttr(Attr *A) { } const AttrVec &Decl::getAttrs() const { - assert(HasAttrs && "No attrs to get!"); - return getASTContext().getDeclAttrs(this); + assert(Attrs && "No attrs to get!"); + return *Attrs; } Decl *Decl::castFromDeclContext (const DeclContext *D) { diff --git a/llvm/include/llvm/Support/Allocator.h b/llvm/include/llvm/Support/Allocator.h index d36d0ff58e490..c4b1761f3385f 100644 --- a/llvm/include/llvm/Support/Allocator.h +++ b/llvm/include/llvm/Support/Allocator.h @@ -420,6 +420,9 @@ template <typename T> class SpecificBumpPtrAllocator { return *this; } + /// Total memory held by this allocator, including slack in the slabs. + size_t getTotalMemory() const { return Allocator.getTotalMemory(); } + /// Call the destructor of each allocated object and deallocate all but the /// current slab and reset the current pointer to the beginning of it, freeing /// all memory allocated so far. _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
