https://github.com/ChuanqiXu9 created https://github.com/llvm/llvm-project/pull/175498
Previously we will apply all decl update unconditionally, but it might not be best. See the attached example 'class-instantiate-no-change-02.cppm' for example. Sometimes these BMIs are reducible. Would land after 22 cut. >From cd9ff8d5ee49af54b87d14bb057677e56b7ec150 Mon Sep 17 00:00:00 2001 From: Chuanqi Xu <[email protected]> Date: Mon, 12 Jan 2026 16:00:02 +0800 Subject: [PATCH] [C++20] [Modules] Apply DeclUpdate lazily in reduced BMI Previously we will apply all decl update unconditionally, but it might not be best. See the attached example 'class-instantiate-no-change-02.cppm' for example. Sometimes these BMIs are reducible. --- clang/include/clang/AST/ASTMutationListener.h | 3 - clang/include/clang/Serialization/ASTWriter.h | 24 ++++- clang/lib/Frontend/MultiplexConsumer.cpp | 6 -- clang/lib/Sema/SemaModule.cpp | 3 - clang/lib/Serialization/ASTWriter.cpp | 79 ++++++++++++--- .../class-instantiate-no-change-02.cppm | 45 +++++++++ .../Modules/special-member-definitions.cppm | 96 +++++++++++++++++++ 7 files changed, 226 insertions(+), 30 deletions(-) create mode 100644 clang/test/Modules/class-instantiate-no-change-02.cppm create mode 100644 clang/test/Modules/special-member-definitions.cppm diff --git a/clang/include/clang/AST/ASTMutationListener.h b/clang/include/clang/AST/ASTMutationListener.h index c8448a25c23a4..6beaf2967b00e 100644 --- a/clang/include/clang/AST/ASTMutationListener.h +++ b/clang/include/clang/AST/ASTMutationListener.h @@ -168,9 +168,6 @@ class ASTMutationListener { virtual void AddedAttributeToRecord(const Attr *Attr, const RecordDecl *Record) {} - /// The parser find the named module declaration. - virtual void EnteringModulePurview() {} - /// An mangling number was added to a Decl /// /// \param D The decl that got a mangling number diff --git a/clang/include/clang/Serialization/ASTWriter.h b/clang/include/clang/Serialization/ASTWriter.h index d3029373ed2f7..581c0e5b37837 100644 --- a/clang/include/clang/Serialization/ASTWriter.h +++ b/clang/include/clang/Serialization/ASTWriter.h @@ -428,10 +428,22 @@ class ASTWriter : public ASTDeserializationListener, /// record containing modifications to them. DeclUpdateMap DeclUpdates; - /// DeclUpdates added during parsing the GMF. We split these from - /// DeclUpdates since we want to add these updates in GMF on need. + /// DeclUpdates added during parsing the module unit. We split + /// these from DeclUpdates since we want to add these updates on need. /// Only meaningful for reduced BMI. - DeclUpdateMap DeclUpdatesFromGMF; + DeclUpdateMap DeclUpdatesLazy; + + /// Map from parents to the updated function definitions and variable + /// definitions. + /// We need this as we need to apply these updates if their parents are + /// touched. + llvm::DenseMap<const Decl *, llvm::SmallVector<const Decl *>> + AddedDefinitions; + + /// Convert non-lazy updates into lazy updates if we're in reduced BMI. + void prepareLazyUpdates(); + /// Apply lazy update if the update is touched during the writing process. + void getLazyUpdates(const Decl *D); /// Mapping from decl templates and its new specialization in the /// current TU. @@ -469,6 +481,11 @@ class ASTWriter : public ASTDeserializationListener, /// it. llvm::SmallSetVector<const DeclContext *, 16> UpdatedDeclContexts; + /// Same as UpdatedDeclContexts except that we only apply these updates + /// lazily. e.g., if these decl context are touched during the writing + /// process. Only meaningful in reduced BMI. + llvm::SmallSetVector<const DeclContext *, 16> UpdatedDeclContextsLazy; + /// Keeps track of declarations that we must emit, even though we're /// not guaranteed to be able to find them by walking the AST starting at the /// translation unit. @@ -976,7 +993,6 @@ class ASTWriter : public ASTDeserializationListener, void RedefinedHiddenDefinition(const NamedDecl *D, Module *M) override; void AddedAttributeToRecord(const Attr *Attr, const RecordDecl *Record) override; - void EnteringModulePurview() override; void AddedManglingNumber(const Decl *D, unsigned) override; void AddedStaticLocalNumbers(const Decl *D, unsigned) override; void AddedAnonymousNamespace(const TranslationUnitDecl *, diff --git a/clang/lib/Frontend/MultiplexConsumer.cpp b/clang/lib/Frontend/MultiplexConsumer.cpp index f5f8848798a35..a2a2ce1996ae7 100644 --- a/clang/lib/Frontend/MultiplexConsumer.cpp +++ b/clang/lib/Frontend/MultiplexConsumer.cpp @@ -125,7 +125,6 @@ class MultiplexASTMutationListener : public ASTMutationListener { void RedefinedHiddenDefinition(const NamedDecl *D, Module *M) override; void AddedAttributeToRecord(const Attr *Attr, const RecordDecl *Record) override; - void EnteringModulePurview() override; void AddedManglingNumber(const Decl *D, unsigned) override; void AddedStaticLocalNumbers(const Decl *D, unsigned) override; void AddedAnonymousNamespace(const TranslationUnitDecl *, @@ -258,11 +257,6 @@ void MultiplexASTMutationListener::AddedAttributeToRecord( L->AddedAttributeToRecord(Attr, Record); } -void MultiplexASTMutationListener::EnteringModulePurview() { - for (auto *L : Listeners) - L->EnteringModulePurview(); -} - void MultiplexASTMutationListener::AddedManglingNumber(const Decl *D, unsigned Number) { for (auto *L : Listeners) diff --git a/clang/lib/Sema/SemaModule.cpp b/clang/lib/Sema/SemaModule.cpp index 24275b97b7462..d43951648ffd2 100644 --- a/clang/lib/Sema/SemaModule.cpp +++ b/clang/lib/Sema/SemaModule.cpp @@ -482,9 +482,6 @@ Sema::ActOnModuleDecl(SourceLocation StartLoc, SourceLocation ModuleLoc, getASTContext().setCurrentNamedModule(Mod); - if (auto *Listener = getASTMutationListener()) - Listener->EnteringModulePurview(); - // We already potentially made an implicit import (in the case of a module // implementation unit importing its interface). Make this module visible // and return the import decl to be added to the current TU. diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 39104da10d0b7..847ed1da6c90d 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -5598,11 +5598,47 @@ void ASTWriter::computeNonAffectingInputFiles() { FileMgr.trackVFSUsage(false); } +void ASTWriter::prepareLazyUpdates() { + // In C++20 named modules with reduced BMI, we only apply the update + // if these updates are touched. + if (!GeneratingReducedBMI) + return; + + DeclUpdateMap DeclUpdatesTmp; + // Move updates to DeclUpdatesLazy but leave CXXAddedFunctionDefinition as is. + // Since added function definition is critical to the AST. If we don't take + // care of it, user might meet missing definition error at linking time. + // Here we leave all CXXAddedFunctionDefinition unconditionally to avoid + // potential issues. + // TODO: Try to refine the strategy to handle CXXAddedFunctionDefinition + // precisely. + for (auto &DeclUpdate : DeclUpdates) { + const Decl *D = DeclUpdate.first; + + for (auto &Update : DeclUpdate.second) { + DeclUpdateKind Kind = Update.getKind(); + + if (Kind == DeclUpdateKind::CXXAddedFunctionDefinition) + DeclUpdatesTmp[D].push_back( + ASTWriter::DeclUpdate(DeclUpdateKind::CXXAddedFunctionDefinition)); + else + DeclUpdatesLazy[D].push_back(Update); + } + } + DeclUpdates.swap(DeclUpdatesTmp); + + UpdatedDeclContextsLazy.swap(UpdatedDeclContexts); + // In reduced BMI, we don't have decls have to emit even if unreferenced. + DeclsToEmitEvenIfUnreferenced.clear(); +} + void ASTWriter::PrepareWritingSpecialDecls(Sema &SemaRef) { ASTContext &Context = SemaRef.Context; bool isModule = WritingModule != nullptr; + prepareLazyUpdates(); + // Set up predefined declaration IDs. auto RegisterPredefDecl = [&] (Decl *D, PredefinedDeclIDs ID) { if (D) { @@ -6231,13 +6267,6 @@ ASTFileSignature ASTWriter::WriteASTCore(Sema *SemaPtr, StringRef isysroot, return backpatchSignature(); } -void ASTWriter::EnteringModulePurview() { - // In C++20 named modules, all entities before entering the module purview - // lives in the GMF. - if (GeneratingReducedBMI) - DeclUpdatesFromGMF.swap(DeclUpdates); -} - // Add update records for all mangling numbers and static local numbers. // These aren't really update records, but this is a convenient way of // tagging this rare extra data onto the declarations. @@ -6947,13 +6976,7 @@ LocalDeclID ASTWriter::GetDeclRef(const Decl *D) { return LocalDeclID(); } - // If the DeclUpdate from the GMF gets touched, emit it. - if (auto *Iter = DeclUpdatesFromGMF.find(D); - Iter != DeclUpdatesFromGMF.end()) { - for (DeclUpdate &Update : Iter->second) - DeclUpdates[D].push_back(Update); - DeclUpdatesFromGMF.erase(Iter); - } + getLazyUpdates(D); // If D comes from an AST file, its declaration ID is already known and // fixed. @@ -7010,6 +7033,34 @@ bool ASTWriter::wasDeclEmitted(const Decl *D) const { return Emitted; } +void ASTWriter::getLazyUpdates(const Decl *D) { + if (!GeneratingReducedBMI) + return; + + auto ApplyLazyUpdate = [&](const Decl *D) { + if (auto *Iter = DeclUpdatesLazy.find(D); Iter != DeclUpdatesLazy.end()) { + for (DeclUpdate &Update : Iter->second) + DeclUpdates[D].push_back(Update); + DeclUpdatesLazy.erase(Iter); + } + }; + + ApplyLazyUpdate(D); + + // If the Decl in DeclUpdatesLazy gets touched, emit the update. + if (auto *DC = dyn_cast<DeclContext>(D); + DC && UpdatedDeclContextsLazy.count(DC)) { + UpdatedDeclContexts.insert(DC); + UpdatedDeclContextsLazy.remove(DC); + } + + if (auto Iter = AddedDefinitions.find(D); Iter != AddedDefinitions.end()) { + for (auto *Def : Iter->second) + ApplyLazyUpdate(Def); + AddedDefinitions.erase(Iter); + } +} + void ASTWriter::associateDeclWithFile(const Decl *D, LocalDeclID ID) { assert(ID.isValid()); assert(D); diff --git a/clang/test/Modules/class-instantiate-no-change-02.cppm b/clang/test/Modules/class-instantiate-no-change-02.cppm new file mode 100644 index 0000000000000..9188408af6ec8 --- /dev/null +++ b/clang/test/Modules/class-instantiate-no-change-02.cppm @@ -0,0 +1,45 @@ +// RUN: rm -rf %t +// RUN: mkdir -p %t +// RUN: split-file %s %t + +// RUN: %clang_cc1 -std=c++20 %t/impl.cppm -emit-reduced-module-interface -o %t/impl.pcm +// RUN: %clang_cc1 -std=c++20 %t/impl.v2.cppm -emit-reduced-module-interface -o %t/impl.v2.pcm +// RUN: %clang_cc1 -std=c++20 %t/m.cppm -emit-reduced-module-interface -o %t/m.pcm \ +// RUN: -fmodule-file=impl=%t/impl.pcm +// RUN: %clang_cc1 -std=c++20 %t/m.cppm -emit-reduced-module-interface -o %t/m.v2.pcm \ +// RUN: -fmodule-file=impl=%t/impl.v2.pcm + +// Since m only uses impl in the definition, the change in impl shouldn't affect m. +// RUN: diff %t/m.pcm %t/m.v2.pcm &> /dev/null + +//--- impl.cppm +export module impl; +export struct Impl { + int get() { + return 43; + } +}; + +//--- impl.v2.cppm +export module impl; +export struct Impl { + int get() { + return 43; + } +}; + +export struct ImplV2 { + int get() { + return 43; + } +}; + +//--- m.cppm +export module m; +import impl; + +export int interface() { + Impl impl; + return impl.get(); +}; + diff --git a/clang/test/Modules/special-member-definitions.cppm b/clang/test/Modules/special-member-definitions.cppm new file mode 100644 index 0000000000000..a01cf0c7bef6b --- /dev/null +++ b/clang/test/Modules/special-member-definitions.cppm @@ -0,0 +1,96 @@ +// RUN: rm -rf %t +// RUN: mkdir -p %t +// RUN: split-file %s %t +// +// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple -emit-reduced-module-interface %t/lock.cppm -o %t/lock.pcm +// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple -emit-reduced-module-interface %t/queue.cppm -o %t/queue.pcm -fprebuilt-module-path=%t +// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple -emit-reduced-module-interface %t/threadpool.cppm -o %t/threadpool.pcm -fprebuilt-module-path=%t +// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/user.cppm -o %t/user.pcm -fprebuilt-module-path=%t -emit-llvm -o - | FileCheck %t/user.cppm + +//--- lock.h +#pragma once + +namespace std { + +struct mutex { + void lock() {} + void unlock() {} +}; + +template <class T> +struct lock_guard { + lock_guard(T &m) : m(m) { + m.lock(); + } + ~lock_guard() { + m.unlock(); + } + + T &m; +}; +} + +//--- lock.cppm +module; +#include "lock.h" +export module lock; +namespace std { + export using ::std::mutex; + export using ::std::lock_guard; +} + +//--- queue.cppm +export module queue; +import lock; + +export template <class T> +class queue { +public: + void push(T value) { + std::lock_guard<std::mutex> lock(m_mutex); + } + T pop() { + std::lock_guard<std::mutex> lock(m_mutex); + return T{}; + } + +private: + std::mutex m_mutex; +}; + +//--- threadpool.cppm +export module threadpool; +import queue; + +export class threadpool { +public: + + void add_task(); + int pop_task(); +private: + queue<int> m_queue; +}; + +inline void threadpool::add_task() { + m_queue.push(42); +} + +inline int threadpool::pop_task() { + return m_queue.pop(); +} + +//--- user.cppm +export module user; +import threadpool; + +export class user { +public: + void run() { + m_threadpool.add_task(); + m_threadpool.pop_task(); + } +private: + threadpool m_threadpool; +}; + +// CHECK: define {{.*}}@_ZNSt10lock_guardISt5mutexED1Ev( _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
