[clang] [C++20] [Modules] Apply DeclUpdate lazily in reduced BMI (PR #175498)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Jan 12 00:04:37 PST 2026
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-modules
Author: Chuanqi Xu (ChuanqiXu9)
<details>
<summary>Changes</summary>
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.
---
Full diff: https://github.com/llvm/llvm-project/pull/175498.diff
7 Files Affected:
- (modified) clang/include/clang/AST/ASTMutationListener.h (-3)
- (modified) clang/include/clang/Serialization/ASTWriter.h (+20-4)
- (modified) clang/lib/Frontend/MultiplexConsumer.cpp (-6)
- (modified) clang/lib/Sema/SemaModule.cpp (-3)
- (modified) clang/lib/Serialization/ASTWriter.cpp (+65-14)
- (added) clang/test/Modules/class-instantiate-no-change-02.cppm (+45)
- (added) clang/test/Modules/special-member-definitions.cppm (+96)
``````````diff
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(
``````````
</details>
https://github.com/llvm/llvm-project/pull/175498
More information about the cfe-commits
mailing list