https://github.com/steffenlarsen created https://github.com/llvm/llvm-project/pull/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 >From a0af2dabaf4a5a63a8b4d04a5711b42ac0d39420 Mon Sep 17 00:00:00 2001 From: Steffen Holst Larsen <[email protected]> Date: Wed, 26 Aug 2026 02:52:22 -0500 Subject: [PATCH] [Clang][AST] Make Decl reference attributes directly 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 while also making it easier to reason about the lifetime of 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. Assisted-by: Claude Opus 5 Signed-off-by: Steffen Holst Larsen <[email protected]> --- clang/include/clang/AST/ASTContext.h | 15 +++++------ clang/include/clang/AST/DeclBase.h | 36 ++++++++++++++------------- clang/lib/AST/ASTContext.cpp | 27 +++----------------- clang/lib/AST/DeclBase.cpp | 22 ++++++++-------- llvm/include/llvm/Support/Allocator.h | 3 +++ 5 files changed, 42 insertions(+), 61 deletions(-) diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h index 013bc68a00a06..c737c3d2d44d0 100644 --- a/clang/include/clang/AST/ASTContext.h +++ b/clang/include/clang/AST/ASTContext.h @@ -521,11 +521,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; + /// 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 @@ -1164,11 +1164,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 9d233be282dbb..11f50c71e41e8 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 335d58ff2c9c8..5d82793495a7e 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -984,12 +984,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(); - CtorClosureDefaultArgs.clear(); for (const auto &Value : ModuleInitializers) @@ -1532,23 +1526,8 @@ DiagnosticsEngine &ASTContext::getDiagnostics() const { return SourceMgr.getDiagnostics(); } -AttrVec& ASTContext::getDeclAttrs(const Decl *D) { - AttrVec *&Result = DeclAttrs[D]; - if (!Result) { - void *Mem = Allocate(sizeof(AttrVec)); - Result = new (Mem) AttrVec; - } - - 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); - } +AttrVec *ASTContext::allocateAttrVec() { + return new (DeclAttrAllocator.Allocate()) AttrVec; } ArrayRef<CXXDefaultArgExpr *> @@ -13605,7 +13584,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 70f61fa57a682..c7e9e32975a32 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 bb0ca118e2015..0e06c6f1bc327 100644 --- a/llvm/include/llvm/Support/Allocator.h +++ b/llvm/include/llvm/Support/Allocator.h @@ -418,6 +418,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
