From 4490abcb586a10b3908daef9bf23643cf536eeeb Mon Sep 17 00:00:00 2001 From: Chuanqi Xu Date: Tue, 11 Nov 2025 16:19:26 +0800 Subject: [PATCH] [C++20] [Modules] Don't import initializer/pending implicit instantiations from other named module Close https://github.com/llvm/llvm-project/issues/166068 The cause of the problem is that we would import initializers and pending implicit instantiations from other named module. This is very bad and it may waste a lot of time. And we didn't observe it as the weak symbols can live together and the strong symbols would be removed by other mechanism. So we didn't observe the bad behavior for a long time. But it indeeds waste compilation time. --- clang/lib/Serialization/ASTReader.cpp | 23 ++++++++++------ clang/lib/Serialization/ASTWriter.cpp | 22 +++++++++------- clang/test/Modules/pr166068.cppm | 38 +++++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 18 deletions(-) create mode 100644 clang/test/Modules/pr166068.cppm diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index a04041c10b4ba..634bf991b2aee 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -4087,10 +4087,14 @@ llvm::Error ASTReader::ReadASTBlock(ModuleFile &F, std::errc::illegal_byte_sequence, "Invalid PENDING_IMPLICIT_INSTANTIATIONS block"); - for (unsigned I = 0, N = Record.size(); I != N; /* in loop */) { - PendingInstantiations.push_back( - {ReadDeclID(F, Record, I), - ReadSourceLocation(F, Record, I).getRawEncoding()}); + // For standard C++20 module, we will only reads the instantiations + // if it is the main file. + if (!F.StandardCXXModule || F.Kind == MK_MainFile) { + for (unsigned I = 0, N = Record.size(); I != N; /* in loop */) { + PendingInstantiations.push_back( + {ReadDeclID(F, Record, I), + ReadSourceLocation(F, Record, I).getRawEncoding()}); + } } break; @@ -6438,10 +6442,13 @@ llvm::Error ASTReader::ReadSubmoduleBlock(ModuleFile &F, case SUBMODULE_INITIALIZERS: { if (!ContextObj) break; - SmallVector Inits; - for (unsigned I = 0; I < Record.size(); /*in loop*/) - Inits.push_back(ReadDeclID(F, Record, I)); - ContextObj->addLazyModuleInitializers(CurrentModule, Inits); + // Standard C++ module has its own way to initialize variables. + if (!F.StandardCXXModule || F.Kind == MK_MainFile) { + SmallVector Inits; + for (unsigned I = 0; I < Record.size(); /*in loop*/) + Inits.push_back(ReadDeclID(F, Record, I)); + ContextObj->addLazyModuleInitializers(CurrentModule, Inits); + } break; } diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 821e7df1bce53..e4618d60a8acb 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -3247,7 +3247,7 @@ void ASTWriter::WriteSubmodules(Module *WritingModule, ASTContext *Context) { // Emit the reachable initializers. // The initializer may only be unreachable in reduced BMI. - if (Context) { + if (Context && !GeneratingReducedBMI) { RecordData Inits; for (Decl *D : Context->getModuleInitializers(Mod)) if (wasDeclEmitted(D)) @@ -5827,17 +5827,19 @@ void ASTWriter::WriteSpecialDeclRecords(Sema &SemaRef) { Stream.EmitRecord(UNUSED_LOCAL_TYPEDEF_NAME_CANDIDATES, UnusedLocalTypedefNameCandidates); - // Write the record containing pending implicit instantiations. - RecordData PendingInstantiations; - for (const auto &I : SemaRef.PendingInstantiations) { - if (!wasDeclEmitted(I.first)) - continue; + if (!GeneratingReducedBMI) { + // Write the record containing pending implicit instantiations. + RecordData PendingInstantiations; + for (const auto &I : SemaRef.PendingInstantiations) { + if (!wasDeclEmitted(I.first)) + continue; - AddDeclRef(I.first, PendingInstantiations); - AddSourceLocation(I.second, PendingInstantiations); + AddDeclRef(I.first, PendingInstantiations); + AddSourceLocation(I.second, PendingInstantiations); + } + if (!PendingInstantiations.empty()) + Stream.EmitRecord(PENDING_IMPLICIT_INSTANTIATIONS, PendingInstantiations); } - if (!PendingInstantiations.empty()) - Stream.EmitRecord(PENDING_IMPLICIT_INSTANTIATIONS, PendingInstantiations); // Write the record containing declaration references of Sema. RecordData SemaDeclRefs; diff --git a/clang/test/Modules/pr166068.cppm b/clang/test/Modules/pr166068.cppm new file mode 100644 index 0000000000000..b6944b591d264 --- /dev/null +++ b/clang/test/Modules/pr166068.cppm @@ -0,0 +1,38 @@ +// RUN: rm -rf %t +// RUN: mkdir -p %t +// RUN: split-file %s %t +// +// RUN: %clang_cc1 -std=c++20 %t/flyweight.cppm -emit-reduced-module-interface -o %t/flyweight.pcm +// RUN: %clang_cc1 -std=c++20 %t/account.cppm -emit-reduced-module-interface -o %t/account.pcm -fprebuilt-module-path=%t +// RUN: %clang_cc1 -std=c++20 %t/core.cppm -emit-reduced-module-interface -o %t/core.pcm -fprebuilt-module-path=%t +// RUN: %clang_cc1 -std=c++20 %t/core.cppm -fprebuilt-module-path=%t -emit-llvm -disable-llvm-passes -o - | FileCheck %t/core.cppm + +//--- flyweight.cppm +module; +template struct flyweight_core { + static bool init() { (void)__builtin_operator_new(2); return true; } + static bool static_initializer; +}; +template bool flyweight_core::static_initializer = init(); +export module flyweight; +export template void flyweight() { + (void)flyweight_core::static_initializer; +} + +//--- account.cppm +export module account; +import flyweight; +export void account() { + (void)::flyweight; +} + +//--- core.cppm +export module core; +import account; + +extern "C" void core() {} + +// Fine enough to check it won't crash. +// CHECK-NOT: init +// CHECK-NOT: static_initializer +// CHECK: define {{.*}}@core(