https://github.com/joker-eph updated https://github.com/llvm/llvm-project/pull/223047
>From faa2ae401e324057d0690db5a5f104676bb3262f Mon Sep 17 00:00:00 2001 From: Mehdi Amini <[email protected]> Date: Thu, 10 Sep 2026 05:51:53 -0700 Subject: [PATCH 1/2] Cache stable DeclContext relationships Cache the owning AST context and stable C++ primary contexts after their first traversal. Leave incomplete C++ records uncached. Resolve mutable primary identities dynamically for namespaces, translation units, C tags, and Objective-C: redeclaration linking, definition demotion, and duplicate-definition merging can change their identities. Add regression tests for these transitions. The following measurements predate the cache-invalidation fixes above. CTMark O0 (3 samples, CPU 6): 29.439800 s -> 29.239400 s (-0.681%). Impact on significant TUs in MLIR build time: - `mlir/lib/RegisterAllDialects.cpp`: 2.8987% fewer retired instructions. - `mlir/lib/Dialect/LLVMIR/IR/NVVMDialect.cpp`: 1.4486% fewer retired instructions. Assisted-by: Codex --- clang/include/clang/AST/DeclBase.h | 26 +++++++++- clang/lib/AST/DeclBase.cpp | 50 +++++++++++++++---- clang/unittests/AST/DeclTest.cpp | 78 ++++++++++++++++++++++++++++++ 3 files changed, 143 insertions(+), 11 deletions(-) diff --git a/clang/include/clang/AST/DeclBase.h b/clang/include/clang/AST/DeclBase.h index 9d233be282dbb..1a2b1c72a6159 100644 --- a/clang/include/clang/AST/DeclBase.h +++ b/clang/include/clang/AST/DeclBase.h @@ -33,6 +33,7 @@ #include "llvm/Support/PrettyStackTrace.h" #include "llvm/Support/VersionTuple.h" #include <algorithm> +#include <atomic> #include <cassert> #include <cstddef> #include <iterator> @@ -2101,6 +2102,17 @@ class DeclContext { /// another pointer. mutable Decl *LastDecl = nullptr; + /// A primary context whose identity cannot change. Incomplete C++ records + /// and contexts with mutable primary identities remain uncached. + /// Relaxed atomics permit cache population during concurrent read-only AST + /// traversal; they do not synchronize mutations to the AST itself. + mutable std::atomic<DeclContext *> CachedPrimaryContext = nullptr; + + /// The owning AST context, which remains the same for this context's + /// lifetime. Null until first queried; relaxed access has the same contract + /// as above. + mutable std::atomic<ASTContext *> CachedASTContext = nullptr; + /// Build up a chain of declarations. /// /// \returns the first/last pair of declarations. @@ -2153,7 +2165,9 @@ class DeclContext { } ASTContext &getParentASTContext() const { - return cast<Decl>(this)->getASTContext(); + if (ASTContext *Cached = CachedASTContext.load(std::memory_order_relaxed)) + return *Cached; + return getParentASTContextSlow(); } bool isClosure() const { return getDeclKind() == Decl::Block; } @@ -2288,7 +2302,12 @@ class DeclContext { /// a different set of declarations. This routine returns the /// "primary" DeclContext structure, which will contain the /// information needed to perform name lookup into this context. - DeclContext *getPrimaryContext(); + DeclContext *getPrimaryContext() { + if (DeclContext *Cached = + CachedPrimaryContext.load(std::memory_order_relaxed)) + return Cached; + return getPrimaryContextSlow(); + } const DeclContext *getPrimaryContext() const { return const_cast<DeclContext*>(this)->getPrimaryContext(); } @@ -2813,6 +2832,9 @@ class DeclContext { StoredDeclsMap *CreateStoredDeclsMap(ASTContext &C) const; + ASTContext &getParentASTContextSlow() const; + DeclContext *getPrimaryContextSlow(); + void loadLazyLocalLexicalLookups(); void buildLookupImpl(DeclContext *DCtx, bool Internal); void makeDeclVisibleInContextWithFlags(NamedDecl *D, bool Internal, diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp index 70f61fa57a682..59ddc64cf30e0 100644 --- a/clang/lib/AST/DeclBase.cpp +++ b/clang/lib/AST/DeclBase.cpp @@ -1488,7 +1488,20 @@ DeclContext *DeclContext::getNonTransparentContext() { return DC; } -DeclContext *DeclContext::getPrimaryContext() { +ASTContext &DeclContext::getParentASTContextSlow() const { + const DeclContext *DC = this; + while (!DC->isTranslationUnit()) { + DC = DC->getParent(); + assert(DC && "This decl context is not contained in a translation unit!"); + } + + ASTContext &Context = cast<TranslationUnitDecl>(DC)->getASTContext(); + CachedASTContext.store(&Context, std::memory_order_relaxed); + return Context; +} + +DeclContext *DeclContext::getPrimaryContextSlow() { + DeclContext *Primary; switch (getDeclKind()) { case Decl::ExternCContext: case Decl::LinkageSpec: @@ -1502,7 +1515,8 @@ DeclContext *DeclContext::getPrimaryContext() { case Decl::RequiresExprBody: case Decl::CXXExpansionStmt: // There is only one DeclContext for these entities. - return this; + Primary = this; + break; case Decl::HLSLBuffer: // Each buffer, even with the same name, is a distinct construct. @@ -1511,17 +1525,22 @@ DeclContext *DeclContext::getPrimaryContext() { // As long as buffers have unique resource bindings the names don't matter. // The names get exposed via the CPU-side reflection API which // supports querying bindings, so we cannot remove them. - return this; + Primary = this; + break; case Decl::TranslationUnit: + // Redeclaration chains can change during AST merging. These queries are + // already constant-time, so do not cache their results. return static_cast<TranslationUnitDecl *>(this)->getFirstDecl(); case Decl::Namespace: return static_cast<NamespaceDecl *>(this)->getFirstDecl(); case Decl::ObjCMethod: - return this; + Primary = this; + break; case Decl::ObjCInterface: + // Duplicate-definition comparison temporarily changes the definition. if (auto *OID = dyn_cast<ObjCInterfaceDecl>(this)) if (auto *Def = OID->getDefinition()) return Def; @@ -1534,27 +1553,40 @@ DeclContext *DeclContext::getPrimaryContext() { return this; case Decl::ObjCCategory: - return this; + Primary = this; + break; case Decl::ObjCImplementation: case Decl::ObjCCategoryImpl: - return this; + Primary = this; + break; // If this is a tag type that has a definition or is currently // being defined, that definition is our primary context. case Decl::ClassTemplatePartialSpecialization: case Decl::ClassTemplateSpecialization: - case Decl::CXXRecord: - return cast<CXXRecordDecl>(this)->getDefinitionOrSelf(); + case Decl::CXXRecord: { + CXXRecordDecl *Definition = cast<CXXRecordDecl>(this)->getDefinition(); + if (!Definition) + return this; + Primary = Definition; + break; + } case Decl::Record: case Decl::Enum: + // Unlike C++ definition data, these definitions can be demoted during + // module merging, changing which declaration is the primary context. return cast<TagDecl>(this)->getDefinitionOrSelf(); default: assert(getDeclKind() >= Decl::firstFunction && getDeclKind() <= Decl::lastFunction && "Unknown DeclContext kind"); - return this; + Primary = this; + break; } + + CachedPrimaryContext.store(Primary, std::memory_order_relaxed); + return Primary; } template <typename T> diff --git a/clang/unittests/AST/DeclTest.cpp b/clang/unittests/AST/DeclTest.cpp index 195b8ab4c4e66..4048c34edf142 100644 --- a/clang/unittests/AST/DeclTest.cpp +++ b/clang/unittests/AST/DeclTest.cpp @@ -37,6 +37,84 @@ using namespace clang::ast_matchers; using namespace clang::tooling; using namespace clang; +TEST(Decl, PrimaryContextAfterNamespaceRedeclaration) { + auto AST = tooling::buildASTFromCode(""); + ASTContext &Ctx = AST->getASTContext(); + auto *TU = Ctx.getTranslationUnitDecl(); + auto *First = NamespaceDecl::Create(Ctx, TU, false, {}, {}, + &Ctx.Idents.get("N"), nullptr, false); + auto *Second = NamespaceDecl::Create(Ctx, TU, false, {}, {}, + &Ctx.Idents.get("N"), nullptr, false); + // A primary-context query must not freeze the redeclaration chain. + EXPECT_EQ(Second, Second->getPrimaryContext()); + Second->setPreviousDecl(First); + EXPECT_EQ(First, Second->getPrimaryContext()); + EXPECT_EQ(&Ctx, &Second->getParentASTContext()); +} + +TEST(Decl, PrimaryContextAfterDefinitionDemotion) { + auto AST = tooling::buildASTFromCodeWithArgs("struct S; struct S {};", {}, + "input.c"); + ASTContext &Ctx = AST->getASTContext(); + auto *Forward = const_cast<RecordDecl *>(selectFirst<RecordDecl>( + "s", + match(recordDecl(hasName("S"), unless(isDefinition())).bind("s"), Ctx))); + ASSERT_NE(nullptr, Forward); + auto *Definition = Forward->getDefinition(); + ASSERT_NE(nullptr, Definition); + EXPECT_EQ(Definition, Forward->getPrimaryContext()); + Definition->demoteThisDefinitionToDeclaration(); + EXPECT_EQ(Forward, Forward->getPrimaryContext()); +} + +TEST(Decl, PrimaryContextAfterObjCDuplicateDefinition) { + auto AST = tooling::buildASTFromCodeWithArgs("@protocol P @end", + {"-x", "objective-c"}); + ASTContext &Ctx = AST->getASTContext(); + auto *First = const_cast<ObjCProtocolDecl *>(selectFirst<ObjCProtocolDecl>( + "p", match(objcProtocolDecl(hasName("P")).bind("p"), Ctx))); + ASSERT_NE(nullptr, First); + auto *Duplicate = ObjCProtocolDecl::Create( + Ctx, Ctx.getTranslationUnitDecl(), &Ctx.Idents.get("P"), {}, {}, First); + Duplicate->startDuplicateDefinitionForComparison(); + EXPECT_EQ(Duplicate, Duplicate->getPrimaryContext()); + Duplicate->mergeDuplicateDefinitionWithCommon(First); + EXPECT_EQ(First, Duplicate->getPrimaryContext()); +} + +TEST(Decl, PrimaryContextAfterEnumDefinitionDemotion) { + auto AST = tooling::buildASTFromCodeWithArgs("enum E; enum E { Value };", {}, + "input.c"); + ASSERT_NE(nullptr, AST); + ASTContext &Ctx = AST->getASTContext(); + auto *Forward = const_cast<EnumDecl *>(selectFirst<EnumDecl>( + "e", + match(enumDecl(hasName("E"), unless(isDefinition())).bind("e"), Ctx))); + ASSERT_NE(nullptr, Forward); + auto *Definition = Forward->getDefinition(); + ASSERT_NE(nullptr, Definition); + EXPECT_EQ(Definition, Forward->getPrimaryContext()); + Definition->demoteThisDefinitionToDeclaration(); + EXPECT_EQ(Forward, Forward->getPrimaryContext()); +} + +TEST(Decl, PrimaryContextAfterObjCInterfaceDuplicateDefinition) { + auto AST = tooling::buildASTFromCodeWithArgs("@interface I @end", + {"-x", "objective-c"}); + ASSERT_NE(nullptr, AST); + ASTContext &Ctx = AST->getASTContext(); + auto *First = const_cast<ObjCInterfaceDecl *>(selectFirst<ObjCInterfaceDecl>( + "i", match(objcInterfaceDecl(hasName("I")).bind("i"), Ctx))); + ASSERT_NE(nullptr, First); + auto *Duplicate = + ObjCInterfaceDecl::Create(Ctx, Ctx.getTranslationUnitDecl(), {}, + &Ctx.Idents.get("I"), nullptr, First); + Duplicate->startDuplicateDefinitionForComparison(); + EXPECT_EQ(Duplicate, Duplicate->getPrimaryContext()); + Duplicate->mergeDuplicateDefinitionWithCommon(First); + EXPECT_EQ(First, Duplicate->getPrimaryContext()); +} + TEST(Decl, CleansUpAPValues) { MatchFinder Finder; std::unique_ptr<FrontendActionFactory> Factory( >From b8d9565e07e67d62103c445c5dced16db4eae8dc Mon Sep 17 00:00:00 2001 From: Mehdi Amini <[email protected]> Date: Sat, 12 Sep 2026 18:05:35 +0200 Subject: [PATCH 2/2] [Clang] Do not imply concurrent AST traversal support --- clang/include/clang/AST/DeclBase.h | 15 +++++---------- clang/lib/AST/DeclBase.cpp | 4 ++-- 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/clang/include/clang/AST/DeclBase.h b/clang/include/clang/AST/DeclBase.h index 1a2b1c72a6159..55dbda3d4d0c0 100644 --- a/clang/include/clang/AST/DeclBase.h +++ b/clang/include/clang/AST/DeclBase.h @@ -33,7 +33,6 @@ #include "llvm/Support/PrettyStackTrace.h" #include "llvm/Support/VersionTuple.h" #include <algorithm> -#include <atomic> #include <cassert> #include <cstddef> #include <iterator> @@ -2104,14 +2103,11 @@ class DeclContext { /// A primary context whose identity cannot change. Incomplete C++ records /// and contexts with mutable primary identities remain uncached. - /// Relaxed atomics permit cache population during concurrent read-only AST - /// traversal; they do not synchronize mutations to the AST itself. - mutable std::atomic<DeclContext *> CachedPrimaryContext = nullptr; + mutable DeclContext *CachedPrimaryContext = nullptr; /// The owning AST context, which remains the same for this context's - /// lifetime. Null until first queried; relaxed access has the same contract - /// as above. - mutable std::atomic<ASTContext *> CachedASTContext = nullptr; + /// lifetime. Null until first queried. + mutable ASTContext *CachedASTContext = nullptr; /// Build up a chain of declarations. /// @@ -2165,7 +2161,7 @@ class DeclContext { } ASTContext &getParentASTContext() const { - if (ASTContext *Cached = CachedASTContext.load(std::memory_order_relaxed)) + if (ASTContext *Cached = CachedASTContext) return *Cached; return getParentASTContextSlow(); } @@ -2303,8 +2299,7 @@ class DeclContext { /// "primary" DeclContext structure, which will contain the /// information needed to perform name lookup into this context. DeclContext *getPrimaryContext() { - if (DeclContext *Cached = - CachedPrimaryContext.load(std::memory_order_relaxed)) + if (DeclContext *Cached = CachedPrimaryContext) return Cached; return getPrimaryContextSlow(); } diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp index 59ddc64cf30e0..e54d4757f41f4 100644 --- a/clang/lib/AST/DeclBase.cpp +++ b/clang/lib/AST/DeclBase.cpp @@ -1496,7 +1496,7 @@ ASTContext &DeclContext::getParentASTContextSlow() const { } ASTContext &Context = cast<TranslationUnitDecl>(DC)->getASTContext(); - CachedASTContext.store(&Context, std::memory_order_relaxed); + CachedASTContext = &Context; return Context; } @@ -1585,7 +1585,7 @@ DeclContext *DeclContext::getPrimaryContextSlow() { break; } - CachedPrimaryContext.store(Primary, std::memory_order_relaxed); + CachedPrimaryContext = Primary; return Primary; } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
