[clang] f2695a1 - [C++20] [Modules] Avoid writing untouched DeclUpdates from GMF in
Chuanqi Xu via cfe-commits
cfe-commits at lists.llvm.org
Wed Apr 17 20:01:51 PDT 2024
Author: Chuanqi Xu
Date: 2024-04-18T11:00:28+08:00
New Revision: f2695a1c2f561da42b973187a254b6eeb7da76a9
URL: https://github.com/llvm/llvm-project/commit/f2695a1c2f561da42b973187a254b6eeb7da76a9
DIFF: https://github.com/llvm/llvm-project/commit/f2695a1c2f561da42b973187a254b6eeb7da76a9.diff
LOG: [C++20] [Modules] Avoid writing untouched DeclUpdates from GMF in
Reduced BMI
Mitigate https://github.com/llvm/llvm-project/issues/61447
The root cause of the above problem is that when we write a declaration,
we need to lookup all the redeclarations in the imported modules. Then
it will be pretty slow if there are too many redeclarations in different
modules. This patch doesn't solve the porblem.
What the patchs mitigated is, when we writing a named module, we shouldn't
write the declarations from GMF if it is unreferenced **in current
module unit**. The difference here is that, if the declaration is used
in the imported modules, we used to emit it as an update. But we
definitely want to avoid that after this patch.
For that reproducer in
https://github.com/llvm/llvm-project/issues/61447, it used to take 2.5s
to compile and now it only takes 0.49s to compile, which is a big win.
Added:
clang/test/Modules/reduced-bmi-empty-module-purview.cppm
Modified:
clang/include/clang/AST/ASTMutationListener.h
clang/include/clang/AST/Decl.h
clang/include/clang/Serialization/ASTWriter.h
clang/lib/AST/ASTContext.cpp
clang/lib/AST/Decl.cpp
clang/lib/Frontend/MultiplexConsumer.cpp
clang/lib/Sema/SemaModule.cpp
clang/lib/Serialization/ASTWriter.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/AST/ASTMutationListener.h b/clang/include/clang/AST/ASTMutationListener.h
index 8879f9f3229ff3..2c4ec2ce67f36b 100644
--- a/clang/include/clang/AST/ASTMutationListener.h
+++ b/clang/include/clang/AST/ASTMutationListener.h
@@ -27,6 +27,7 @@ namespace clang {
class FunctionTemplateDecl;
class Module;
class NamedDecl;
+ class NamespaceDecl;
class ObjCCategoryDecl;
class ObjCContainerDecl;
class ObjCInterfaceDecl;
@@ -35,6 +36,7 @@ namespace clang {
class QualType;
class RecordDecl;
class TagDecl;
+ class TranslationUnitDecl;
class ValueDecl;
class VarDecl;
class VarTemplateDecl;
@@ -147,6 +149,31 @@ 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
+ ///
+ /// \param Number The mangling number that was added to the Decl
+ virtual void AddedManglingNumber(const Decl *D, unsigned Number) {}
+
+ /// An static local number was added to a Decl
+ ///
+ /// \param D The decl that got a static local number
+ ///
+ /// \param Number The static local number that was added to the Decl
+ virtual void AddedStaticLocalNumbers(const Decl *D, unsigned Number) {}
+
+ /// An anonymous namespace was added the translation unit decl
+ ///
+ /// \param TU The translation unit decl that got a new anonymous namespace
+ ///
+ /// \param AnonNamespace The anonymous namespace that was added
+ virtual void AddedAnonymousNamespace(const TranslationUnitDecl *TU,
+ NamespaceDecl *AnonNamespace) {}
+
// NOTE: If new methods are added they should also be added to
// MultiplexASTMutationListener.
};
diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 01af50ca694fdd..91449ebcceec37 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -120,7 +120,7 @@ class TranslationUnitDecl : public Decl,
ASTContext &getASTContext() const { return Ctx; }
NamespaceDecl *getAnonymousNamespace() const { return AnonymousNamespace; }
- void setAnonymousNamespace(NamespaceDecl *D) { AnonymousNamespace = D; }
+ void setAnonymousNamespace(NamespaceDecl *D);
static TranslationUnitDecl *Create(ASTContext &C);
diff --git a/clang/include/clang/Serialization/ASTWriter.h b/clang/include/clang/Serialization/ASTWriter.h
index 443f7703104700..892d7363e06dd7 100644
--- a/clang/include/clang/Serialization/ASTWriter.h
+++ b/clang/include/clang/Serialization/ASTWriter.h
@@ -399,6 +399,11 @@ 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.
+ /// Only meaningful for reduced BMI.
+ DeclUpdateMap DeclUpdatesFromGMF;
+
using FirstLatestDeclMap = llvm::DenseMap<Decl *, Decl *>;
/// Map of first declarations from a chained PCH that point to the
@@ -866,6 +871,11 @@ 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 *,
+ NamespaceDecl *AnonNamespace) override;
};
/// AST and semantic-analysis consumer that generates a
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 6ce233704a5885..bcae6d705b780c 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -12245,8 +12245,13 @@ QualType ASTContext::getRealTypeForBitwidth(unsigned DestWidth,
}
void ASTContext::setManglingNumber(const NamedDecl *ND, unsigned Number) {
- if (Number > 1)
- MangleNumbers[ND] = Number;
+ if (Number <= 1)
+ return;
+
+ MangleNumbers[ND] = Number;
+
+ if (Listener)
+ Listener->AddedManglingNumber(ND, Number);
}
unsigned ASTContext::getManglingNumber(const NamedDecl *ND,
@@ -12265,8 +12270,13 @@ unsigned ASTContext::getManglingNumber(const NamedDecl *ND,
}
void ASTContext::setStaticLocalNumber(const VarDecl *VD, unsigned Number) {
- if (Number > 1)
- StaticLocalNumbers[VD] = Number;
+ if (Number <= 1)
+ return;
+
+ StaticLocalNumbers[VD] = Number;
+
+ if (Listener)
+ Listener->AddedStaticLocalNumbers(VD, Number);
}
unsigned ASTContext::getStaticLocalNumber(const VarDecl *VD) const {
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 33b6f8611f2162..63388d79bbf217 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -5274,6 +5274,13 @@ TranslationUnitDecl *TranslationUnitDecl::Create(ASTContext &C) {
return new (C, (DeclContext *)nullptr) TranslationUnitDecl(C);
}
+void TranslationUnitDecl::setAnonymousNamespace(NamespaceDecl *D) {
+ AnonymousNamespace = D;
+
+ if (ASTMutationListener *Listener = Ctx.getASTMutationListener())
+ Listener->AddedAnonymousNamespace(this, D);
+}
+
void PragmaCommentDecl::anchor() {}
PragmaCommentDecl *PragmaCommentDecl::Create(const ASTContext &C,
diff --git a/clang/lib/Frontend/MultiplexConsumer.cpp b/clang/lib/Frontend/MultiplexConsumer.cpp
index 737877329c9ce7..744ea70cc24def 100644
--- a/clang/lib/Frontend/MultiplexConsumer.cpp
+++ b/clang/lib/Frontend/MultiplexConsumer.cpp
@@ -20,6 +20,9 @@ using namespace clang;
namespace clang {
+class NamespaceDecl;
+class TranslationUnitDecl;
+
MultiplexASTDeserializationListener::MultiplexASTDeserializationListener(
const std::vector<ASTDeserializationListener*>& L)
: Listeners(L) {
@@ -115,6 +118,11 @@ 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 *,
+ NamespaceDecl *AnonNamespace) override;
private:
std::vector<ASTMutationListener*> Listeners;
@@ -238,6 +246,27 @@ 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)
+ L->AddedManglingNumber(D, Number);
+}
+void MultiplexASTMutationListener::AddedStaticLocalNumbers(const Decl *D,
+ unsigned Number) {
+ for (auto *L : Listeners)
+ L->AddedStaticLocalNumbers(D, Number);
+}
+void MultiplexASTMutationListener::AddedAnonymousNamespace(
+ const TranslationUnitDecl *TU, NamespaceDecl *AnonNamespace) {
+ for (auto *L : Listeners)
+ L->AddedAnonymousNamespace(TU, AnonNamespace);
+}
+
} // end namespace clang
MultiplexConsumer::MultiplexConsumer(
diff --git a/clang/lib/Sema/SemaModule.cpp b/clang/lib/Sema/SemaModule.cpp
index 2ddf9d70263a09..67658c93ed3baf 100644
--- a/clang/lib/Sema/SemaModule.cpp
+++ b/clang/lib/Sema/SemaModule.cpp
@@ -12,6 +12,7 @@
//===----------------------------------------------------------------------===//
#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/ASTMutationListener.h"
#include "clang/Lex/HeaderSearch.h"
#include "clang/Lex/Preprocessor.h"
#include "clang/Sema/SemaInternal.h"
@@ -475,6 +476,9 @@ 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 b2a078b6d80f46..c3cf242391b743 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -5026,27 +5026,6 @@ ASTFileSignature ASTWriter::WriteASTCore(Sema &SemaRef, StringRef isysroot,
GetDeclRef(D);
}
- // If the translation unit has an anonymous namespace, and we don't already
- // have an update block for it, write it as an update block.
- // FIXME: Why do we not do this if there's already an update block?
- if (NamespaceDecl *NS = TU->getAnonymousNamespace()) {
- ASTWriter::UpdateRecord &Record = DeclUpdates[TU];
- if (Record.empty())
- Record.push_back(DeclUpdate(UPD_CXX_ADDED_ANONYMOUS_NAMESPACE, NS));
- }
-
- // 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.
- for (const auto &Number : Context.MangleNumbers)
- if (!Number.first->isFromASTFile())
- DeclUpdates[Number.first].push_back(DeclUpdate(UPD_MANGLING_NUMBER,
- Number.second));
- for (const auto &Number : Context.StaticLocalNumbers)
- if (!Number.first->isFromASTFile())
- DeclUpdates[Number.first].push_back(DeclUpdate(UPD_STATIC_LOCAL_NUMBER,
- Number.second));
-
// Make sure visible decls, added to DeclContexts previously loaded from
// an AST file, are registered for serialization. Likewise for template
// specializations added to imported templates.
@@ -5315,6 +5294,41 @@ ASTFileSignature ASTWriter::WriteASTCore(Sema &SemaRef, 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.
+void ASTWriter::AddedManglingNumber(const Decl *D, unsigned Number) {
+ if (D->isFromASTFile())
+ return;
+
+ DeclUpdates[D].push_back(DeclUpdate(UPD_MANGLING_NUMBER, Number));
+}
+void ASTWriter::AddedStaticLocalNumbers(const Decl *D, unsigned Number) {
+ if (D->isFromASTFile())
+ return;
+
+ DeclUpdates[D].push_back(DeclUpdate(UPD_STATIC_LOCAL_NUMBER, Number));
+}
+
+void ASTWriter::AddedAnonymousNamespace(const TranslationUnitDecl *TU,
+ NamespaceDecl *AnonNamespace) {
+ // If the translation unit has an anonymous namespace, and we don't already
+ // have an update block for it, write it as an update block.
+ // FIXME: Why do we not do this if there's already an update block?
+ if (NamespaceDecl *NS = TU->getAnonymousNamespace()) {
+ ASTWriter::UpdateRecord &Record = DeclUpdates[TU];
+ if (Record.empty())
+ Record.push_back(DeclUpdate(UPD_CXX_ADDED_ANONYMOUS_NAMESPACE, NS));
+ }
+}
+
void ASTWriter::WriteDeclAndTypes(ASTContext &Context) {
// Keep writing types, declarations, and declaration update records
// until we've emitted all of them.
@@ -5860,6 +5874,14 @@ DeclID ASTWriter::GetDeclRef(const Decl *D) {
return 0;
}
+ // 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);
+ }
+
// If D comes from an AST file, its declaration ID is already known and
// fixed.
if (D->isFromASTFile())
diff --git a/clang/test/Modules/reduced-bmi-empty-module-purview.cppm b/clang/test/Modules/reduced-bmi-empty-module-purview.cppm
new file mode 100644
index 00000000000000..54a1e9b77e6e64
--- /dev/null
+++ b/clang/test/Modules/reduced-bmi-empty-module-purview.cppm
@@ -0,0 +1,96 @@
+// Test that we won't write additional information into the Reduced BMI if the
+// module purview is empty.
+//
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/M.cppm -emit-reduced-module-interface -o %t/M.pcm
+// RUN: %clang_cc1 -std=c++20 %t/A.cppm -emit-reduced-module-interface -o %t/A.pcm \
+// RUN: -fmodule-file=M=%t/M.pcm
+// RUN: llvm-bcanalyzer --dump --disable-histogram --show-binary-blobs %t/A.pcm > %t/A.dump
+// RUN: cat %t/A.dump | FileCheck %t/A.cppm
+//
+// RUN: %clang_cc1 -std=c++20 %t/A1.cppm -emit-reduced-module-interface -o %t/A1.pcm \
+// RUN: -fmodule-file=M=%t/M.pcm
+// RUN: llvm-bcanalyzer --dump --disable-histogram --show-binary-blobs %t/A1.pcm > %t/A1.dump
+// RUN: cat %t/A1.dump | FileCheck %t/A1.cppm
+
+//--- foo.h
+namespace ns {
+template <class C>
+class A {
+
+};
+
+extern template class A<short>;
+
+inline A<int> a() { return A<int>(); }
+template <class T>
+A<T> _av_ = A<T>();
+
+auto _av_1 = _av_<int>;
+auto _av_2 = _av_<double>;
+
+template <>
+class A<void> {
+
+};
+
+void func(A<int>, ...) {
+
+}
+
+}
+
+struct S {
+ union {
+ unsigned int V;
+ struct {
+ int v1;
+ int v2;
+ ns::A<int> a1;
+ } WESQ;
+ };
+
+ union {
+ double d;
+ struct {
+ int v1;
+ unsigned v2;
+ ns::A<unsigned> a1;
+ } Another;
+ };
+};
+
+//--- M.cppm
+module;
+#include "foo.h"
+export module M;
+export namespace nv {
+ using ns::A;
+ using ns::a;
+ using ns::_av_;
+
+ using ns::func;
+}
+using ::S;
+
+//--- A.cppm
+module;
+#include "foo.h"
+export module A;
+import M;
+
+// CHECK-NOT: <DECL_CXX_RECORD
+// CHECK-NOT: <DECL_UPDATE_OFFSETS
+
+//--- A1.cppm
+module;
+import M;
+#include "foo.h"
+export module A;
+
+// CHECK-NOT: <DECL_CXX_RECORD
+// CHECK-NOT: <DECL_UPDATE_OFFSETS
+
More information about the cfe-commits
mailing list