[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
ChuanqiXu9 wrote: > This is a great way to start a new year ;) > > The phab link is https://reviews.llvm.org/D41416. > > In general I was wondering could we simplify the implementation by loading > the specialization hash table upon module load. That should be relatively > cheap as we will read 2 integers per specialization. > > Perhaps we should put both patches together and that'd allow us to test them > if they are on par with https://reviews.llvm.org/D41416 which we use > downstream. > > Thanks for working on this! Hi Vassilev, for testing purpose I sent https://github.com/ChuanqiXu9/llvm-project/tree/LoadSpecializationUpdatesLazily. I didn't create stacked review since I feel a standalone branch may be sufficient. > In general I was wondering could we simplify the implementation by loading > the specialization hash table upon module load. That should be relatively > cheap as we will read 2 integers per specialization. IIUC, it looks like what I do in https://github.com/ChuanqiXu9/llvm-project/commit/7f027f0b6551a8e421034e96bd0a4c953c473df6#diff-c61a3cce4bfa099b5af032fa83cbf1563f0af4bf58dc112b39571d74b6b681c1R3487-R3499. But I don't want to do that with this patch. Since we can avoid load the hash table if the template decl is not loaded. https://github.com/llvm/llvm-project/pull/76774 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
@@ -150,6 +150,11 @@ class ExternalASTSource : public
RefCountedBase {
virtual bool
FindExternalVisibleDeclsByName(const DeclContext *DC, DeclarationName Name);
+ /// Load all the external specialzations for the Decl and the corresponding
+ /// template arguments.
+ virtual void LoadExternalSpecs(const Decl *D,
ChuanqiXu9 wrote:
I feel `Load` may be a better name. Since from the signature it doesn't find
anything. And if we want consistency, I suggest to rename
`FindExternalVisibleDeclsByName ` to `LoadExternalVisibleDeclsByName`.
https://github.com/llvm/llvm-project/pull/76774
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
@@ -527,6 +527,10 @@ class ASTWriter : public ASTDeserializationListener, bool isLookupResultExternal(StoredDeclsList &Result, DeclContext *DC); bool isLookupResultEntirelyExternal(StoredDeclsList &Result, DeclContext *DC); + uint64_t + WriteSpecsLookupTable(NamedDecl *D, ChuanqiXu9 wrote: Got it. Will do in the next circle. https://github.com/llvm/llvm-project/pull/76774 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
@@ -2431,10 +2434,14 @@ void
ASTDeclReader::VisitClassTemplateDecl(ClassTemplateDecl *D) {
mergeRedeclarableTemplate(D, Redecl);
if (ThisDeclID == Redecl.getFirstID()) {
-// This ClassTemplateDecl owns a CommonPtr; read it to keep track of all of
-// the specializations.
+// This ClassTemplateDecl owns a CommonPtr; read it to keep track of all
+// of the specializations.
SmallVector SpecIDs;
readDeclIDList(SpecIDs);
+
+if (Record.readInt())
+ ReadDeclsSpecs(*Loc.F, D, Loc.F->DeclsCursor);
ChuanqiXu9 wrote:
Then it won't fall here. It is the job of the latter patch
(https://github.com/ChuanqiXu9/llvm-project/commit/7f027f0b6551a8e421034e96bd0a4c953c473df6)
https://github.com/llvm/llvm-project/pull/76774
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
@@ -1249,3 +1249,5 @@ void ODRHash::AddQualType(QualType T) {
void ODRHash::AddBoolean(bool Value) {
Bools.push_back(Value);
}
+
+void ODRHash::AddInteger(unsigned Value) { ID.AddInteger(Value); }
ChuanqiXu9 wrote:
Interesting. I didn't recognize this. If this is true, we need to decide if we
can leave a FIXME here or we must fix it to proceed.
https://github.com/llvm/llvm-project/pull/76774
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
@@ -1249,3 +1249,5 @@ void ODRHash::AddQualType(QualType T) {
void ODRHash::AddBoolean(bool Value) {
Bools.push_back(Value);
}
+
+void ODRHash::AddInteger(unsigned Value) { ID.AddInteger(Value); }
hahnjo wrote:
The review related to `ODRHash` is this one: https://reviews.llvm.org/D153003
In short, my understanding is that `ODRHash` gives the following guarantee: If
the hashes are different, there is guaranteed to be a ODR violation. In the
other direction, if two hashes are the same, the declarations have to be
compared in more detail, ie there may or may not be an ODR violation.
For the specializations, we need the opposite: If two template arguments are
semantically the same (*), they *must* hash to the same value or otherwise we
will not find the correct bucket. On the other hand, two different
specialization arguments may have the same hash, that's fine for the map data
structure.
Now the additional caveat (*) is that "semantically the same" is not the same
congruence as "no ODR violation". In https://reviews.llvm.org/D153003 we
discuss `using` declarations, but IIRC it's also possible to construct
problematic cases with (nested) namespaces, top-level `::` prefixes, and
template template parameters. Taken together, my conclusion from the discussion
above is that `ODRHash` is simply not the right method to find template
specialization parameters in a map.
https://github.com/llvm/llvm-project/pull/76774
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
@@ -1249,3 +1249,5 @@ void ODRHash::AddQualType(QualType T) {
void ODRHash::AddBoolean(bool Value) {
Bools.push_back(Value);
}
+
+void ODRHash::AddInteger(unsigned Value) { ID.AddInteger(Value); }
ChuanqiXu9 wrote:
Great analysis. Fair enough, let's find a method to proceed.
https://github.com/llvm/llvm-project/pull/76774
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
@@ -1249,3 +1249,5 @@ void ODRHash::AddQualType(QualType T) {
void ODRHash::AddBoolean(bool Value) {
Bools.push_back(Value);
}
+
+void ODRHash::AddInteger(unsigned Value) { ID.AddInteger(Value); }
ChuanqiXu9 wrote:
I tried to add a test case to show the problem in
https://github.com/llvm/llvm-project/commit/9b808a4beb8e6c8255b412fdd6f5a3e20cbcf270.
But the current patch works well for that. While I agree the ODRHash may be
too aggressive for the problem we're solving, I don't want to write things that
can't be well tested. I am wondering if we can proceed by leaving a FIXME here
if we can't find good test in time? Or maybe we can add an option
`-fload-specialization-lazily`, then we can regress smoothly if there are any
problems.
@hahnjo @vgvassilev
https://github.com/llvm/llvm-project/pull/76774
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
@@ -1249,3 +1249,5 @@ void ODRHash::AddQualType(QualType T) {
void ODRHash::AddBoolean(bool Value) {
Bools.push_back(Value);
}
+
+void ODRHash::AddInteger(unsigned Value) { ID.AddInteger(Value); }
ChuanqiXu9 wrote:
The secret why ODRHash can handle this may live in
https://github.com/llvm/llvm-project/blob/fe1364f1e7ac0c4d0f9a4b15189485782241190d/clang/lib/AST/ODRHash.cpp#L868-L910.
We've already done the work to strip the types.
https://github.com/llvm/llvm-project/pull/76774
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
@@ -1249,3 +1249,5 @@ void ODRHash::AddQualType(QualType T) {
void ODRHash::AddBoolean(bool Value) {
Bools.push_back(Value);
}
+
+void ODRHash::AddInteger(unsigned Value) { ID.AddInteger(Value); }
hahnjo wrote:
That test does not exercise an alias argument to a template template argument.
IIRC the code you code is only active for typenames. Also see the test in
https://reviews.llvm.org/D153003 that exercises different spellings of the
semantically equivalent type `NS::A` inside a namespace.
https://github.com/llvm/llvm-project/pull/76774
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
@@ -1249,3 +1249,5 @@ void ODRHash::AddQualType(QualType T) {
void ODRHash::AddBoolean(bool Value) {
Bools.push_back(Value);
}
+
+void ODRHash::AddInteger(unsigned Value) { ID.AddInteger(Value); }
ChuanqiXu9 wrote:
I had this
https://github.com/llvm/llvm-project/commit/9b808a4beb8e6c8255b412fdd6f5a3e20cbcf270#diff-e88c8434346144fdf335c14c17a112759af7a4e81957efcaca635a08b9f13a29R110-R116.
Do you mean I need to put that into a namespace?
I tried to create alias arguments for template template arguments. But the
tests can't pass before the current patch.
https://github.com/llvm/llvm-project/pull/76774
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
@@ -1249,3 +1249,5 @@ void ODRHash::AddQualType(QualType T) {
void ODRHash::AddBoolean(bool Value) {
Bools.push_back(Value);
}
+
+void ODRHash::AddInteger(unsigned Value) { ID.AddInteger(Value); }
ChuanqiXu9 wrote:
It looks like the qualified related problems in ODRHash (at least some of them)
are fixed in https://reviews.llvm.org/D156210
https://github.com/llvm/llvm-project/pull/76774
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
vgvassilev wrote: > > This is a great way to start a new year ;) > > The phab link is https://reviews.llvm.org/D41416. > > In general I was wondering could we simplify the implementation by loading > > the specialization hash table upon module load. That should be relatively > > cheap as we will read 2 integers per specialization. > > Perhaps we should put both patches together and that'd allow us to test > > them if they are on par with https://reviews.llvm.org/D41416 which we use > > downstream. > > Thanks for working on this! > > Hi Vassilev, for testing purpose I sent > https://github.com/ChuanqiXu9/llvm-project/tree/LoadSpecializationUpdatesLazily. > I didn't create stacked review since I feel a standalone branch may be > sufficient. > @ChuanqiXu9, I'd prefer to review both patches at the same time. Otherwise we risk of missing some important details. https://github.com/llvm/llvm-project/pull/76774 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
ChuanqiXu9 wrote: > > > This is a great way to start a new year ;) > > > The phab link is https://reviews.llvm.org/D41416. > > > In general I was wondering could we simplify the implementation by > > > loading the specialization hash table upon module load. That should be > > > relatively cheap as we will read 2 integers per specialization. > > > Perhaps we should put both patches together and that'd allow us to test > > > them if they are on par with https://reviews.llvm.org/D41416 which we use > > > downstream. > > > Thanks for working on this! > > > > > > Hi Vassilev, for testing purpose I sent > > https://github.com/ChuanqiXu9/llvm-project/tree/LoadSpecializationUpdatesLazily. > > I didn't create stacked review since I feel a standalone branch may be > > sufficient. > > @ChuanqiXu9, I'd prefer to review both patches at the same time. Otherwise we > risk of missing some important details. Got it. I can try to create a stacked review. But from I know about the status quo stacked review now, it will require us to lost the current contexnt... And it will still be pretty valuable if you can test this with your internal workloads, then may be we can find something pretty important in the high level before going into the details. I've tested this in our local workloads, and it looks good and the performance improvements remains. But I know our uses about modules may be not so complex like yours. https://github.com/llvm/llvm-project/pull/76774 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
@@ -1249,3 +1249,5 @@ void ODRHash::AddQualType(QualType T) {
void ODRHash::AddBoolean(bool Value) {
Bools.push_back(Value);
}
+
+void ODRHash::AddInteger(unsigned Value) { ID.AddInteger(Value); }
vgvassilev wrote:
I guess the comment we are discussing is here:
https://reviews.llvm.org/D154324#4524368 by @zygoloid:
"
...
For [D41416](https://reviews.llvm.org/D41416), ODR hashing may not be the best
mechanism to hash the template arguments, unfortunately. ODR hashing is (or
perhaps, should be) about determining whether two things are spelled the same
way and have the same meaning (as required by the C++ ODR), whereas I think
what you're looking for is whether they have the same meaning regardless of
spelling. Maybe we can get away with reusing ODR hashing anyway, on the basis
that any canonical, non-dependent template argument should have the same
(invented) spelling in every translation unit, but I'm not certain that's true
in all cases. There may still be cases where the canonical type includes some
aspect of "whatever we saw first", in which case the ODR hash can differ across
translation units for non-dependent, canonical template arguments that are
spelled differently but have the same meaning, though I can't think of one
off-hand.
```
https://github.com/llvm/llvm-project/pull/76774
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
https://github.com/vgvassilev edited https://github.com/llvm/llvm-project/pull/76774 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
https://github.com/vgvassilev edited https://github.com/llvm/llvm-project/pull/76774 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
@@ -1249,3 +1249,5 @@ void ODRHash::AddQualType(QualType T) {
void ODRHash::AddBoolean(bool Value) {
Bools.push_back(Value);
}
+
+void ODRHash::AddInteger(unsigned Value) { ID.AddInteger(Value); }
ChuanqiXu9 wrote:
Yeah, I just saw it. My concern for reinventing a new hash mechanism is how can
we make sure it is correct. It may be not hard to invent a new hasher. But I am
just worrying it may not be well tested. I prefer to make it step by step.
https://github.com/llvm/llvm-project/pull/76774
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
vgvassilev wrote: > > > > This is a great way to start a new year ;) > > > > The phab link is https://reviews.llvm.org/D41416. > > > > In general I was wondering could we simplify the implementation by > > > > loading the specialization hash table upon module load. That should be > > > > relatively cheap as we will read 2 integers per specialization. > > > > Perhaps we should put both patches together and that'd allow us to test > > > > them if they are on par with https://reviews.llvm.org/D41416 which we > > > > use downstream. > > > > Thanks for working on this! > > > > > > > > > Hi Vassilev, for testing purpose I sent > > > https://github.com/ChuanqiXu9/llvm-project/tree/LoadSpecializationUpdatesLazily. > > > I didn't create stacked review since I feel a standalone branch may be > > > sufficient. > > > > > > @ChuanqiXu9, I'd prefer to review both patches at the same time. Otherwise > > we risk of missing some important details. > > Got it. I can try to create a stacked review. But from I know about the > status quo stacked review now, it will require us to lost the current > contexnt... > > And it will still be pretty valuable if you can test this with your internal > workloads, then may be we can find something pretty important in the high > level before going into the details. I've tested this in our local workloads, > and it looks good and the performance improvements remains. But I know our > uses about modules may be not so complex like yours. I would just push the second commit here. It should be good enough. https://github.com/llvm/llvm-project/pull/76774 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
@@ -1249,3 +1249,5 @@ void ODRHash::AddQualType(QualType T) {
void ODRHash::AddBoolean(bool Value) {
Bools.push_back(Value);
}
+
+void ODRHash::AddInteger(unsigned Value) { ID.AddInteger(Value); }
vgvassilev wrote:
If the example of @hahnjo works, perhaps a FIXME referring to this discussion
should be sufficient and we can revisit the issue once we have an example that
breaks.
https://github.com/llvm/llvm-project/pull/76774
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
https://github.com/ChuanqiXu9 updated
https://github.com/llvm/llvm-project/pull/76774
>From 50fd47f2bfda527807f8cc5e46425050246868aa Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Wed, 3 Jan 2024 11:33:17 +0800
Subject: [PATCH] Load Specializations Lazily
---
clang/include/clang/AST/DeclTemplate.h| 35 ++--
clang/include/clang/AST/ExternalASTSource.h | 6 +
clang/include/clang/AST/ODRHash.h | 3 +
.../clang/Sema/MultiplexExternalSemaSource.h | 6 +
.../include/clang/Serialization/ASTBitCodes.h | 3 +
clang/include/clang/Serialization/ASTReader.h | 28 +++
clang/include/clang/Serialization/ASTWriter.h | 6 +
clang/lib/AST/DeclTemplate.cpp| 67 +---
clang/lib/AST/ExternalASTSource.cpp | 5 +
clang/lib/AST/ODRHash.cpp | 2 +
.../lib/Sema/MultiplexExternalSemaSource.cpp | 6 +
clang/lib/Serialization/ASTReader.cpp | 125 +-
clang/lib/Serialization/ASTReaderDecl.cpp | 35 +++-
clang/lib/Serialization/ASTReaderInternals.h | 80 +
clang/lib/Serialization/ASTWriter.cpp | 144 +++-
clang/lib/Serialization/ASTWriterDecl.cpp | 78 ++---
clang/test/Modules/odr_hash.cpp | 4 +-
clang/unittests/Serialization/CMakeLists.txt | 1 +
.../Serialization/LoadSpecLazily.cpp | 159 ++
19 files changed, 728 insertions(+), 65 deletions(-)
create mode 100644 clang/unittests/Serialization/LoadSpecLazily.cpp
diff --git a/clang/include/clang/AST/DeclTemplate.h
b/clang/include/clang/AST/DeclTemplate.h
index 832ad2de6b08a8..4699dd17bc182c 100644
--- a/clang/include/clang/AST/DeclTemplate.h
+++ b/clang/include/clang/AST/DeclTemplate.h
@@ -525,8 +525,11 @@ class FunctionTemplateSpecializationInfo final
return Function.getInt();
}
+ void loadExternalRedecls();
+
public:
friend TrailingObjects;
+ friend class ASTReader;
static FunctionTemplateSpecializationInfo *
Create(ASTContext &C, FunctionDecl *FD, FunctionTemplateDecl *Template,
@@ -789,13 +792,15 @@ class RedeclarableTemplateDecl : public TemplateDecl,
return SpecIterator(isEnd ? Specs.end() : Specs.begin());
}
- void loadLazySpecializationsImpl() const;
+ void loadExternalSpecializations() const;
template
typename SpecEntryTraits::DeclType*
findSpecializationImpl(llvm::FoldingSetVector &Specs,
void *&InsertPos, ProfileArguments &&...ProfileArgs);
+ void loadLazySpecializationsWithArgs(ArrayRef
TemplateArgs);
+
template
void addSpecializationImpl(llvm::FoldingSetVector &Specs,
EntryType *Entry, void *InsertPos);
@@ -814,9 +819,13 @@ class RedeclarableTemplateDecl : public TemplateDecl,
/// If non-null, points to an array of specializations (including
/// partial specializations) known only by their external declaration IDs.
///
+/// These specializations needs to be loaded at once in
+/// loadExternalSpecializations to complete the redecl chain or be
preparing
+/// for template resolution.
+///
/// The first value in the array is the number of specializations/partial
/// specializations that follow.
-uint32_t *LazySpecializations = nullptr;
+uint32_t *ExternalSpecializations = nullptr;
/// The set of "injected" template arguments used within this
/// template.
@@ -850,6 +859,8 @@ class RedeclarableTemplateDecl : public TemplateDecl,
friend class ASTDeclWriter;
friend class ASTReader;
template friend class RedeclarableTemplate;
+ friend class ClassTemplateSpecializationDecl;
+ friend class VarTemplateSpecializationDecl;
/// Retrieves the canonical declaration of this template.
RedeclarableTemplateDecl *getCanonicalDecl() override {
@@ -977,6 +988,7 @@ SpecEntryTraits {
class FunctionTemplateDecl : public RedeclarableTemplateDecl {
protected:
friend class FunctionDecl;
+ friend class FunctionTemplateSpecializationInfo;
/// Data that is common to all of the declarations of a given
/// function template.
@@ -1012,13 +1024,13 @@ class FunctionTemplateDecl : public
RedeclarableTemplateDecl {
void addSpecialization(FunctionTemplateSpecializationInfo* Info,
void *InsertPos);
+ /// Load any lazily-loaded specializations from the external source.
+ void LoadLazySpecializations() const;
+
public:
friend class ASTDeclReader;
friend class ASTDeclWriter;
- /// Load any lazily-loaded specializations from the external source.
- void LoadLazySpecializations() const;
-
/// Get the underlying function declaration of the template.
FunctionDecl *getTemplatedDecl() const {
return static_cast(TemplatedDecl);
@@ -1839,6 +1851,8 @@ class ClassTemplateSpecializationDecl
LLVM_PREFERRED_TYPE(TemplateSpecializationKind)
unsigned SpecializationKind : 3;
+ void loadExternalRedecls();
+
protected:
ClassTemplateSpecializationDecl(ASTCo
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
ChuanqiXu9 wrote: I failed to use spr to create stacked review... So I just create the stacked PR manually: https://github.com/llvm/llvm-project/pull/77417 https://github.com/llvm/llvm-project/pull/76774 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
@@ -1249,3 +1249,5 @@ void ODRHash::AddQualType(QualType T) {
void ODRHash::AddBoolean(bool Value) {
Bools.push_back(Value);
}
+
+void ODRHash::AddInteger(unsigned Value) { ID.AddInteger(Value); }
ChuanqiXu9 wrote:
Done
https://github.com/llvm/llvm-project/pull/76774
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
@@ -527,6 +527,10 @@ class ASTWriter : public ASTDeserializationListener, bool isLookupResultExternal(StoredDeclsList &Result, DeclContext *DC); bool isLookupResultEntirelyExternal(StoredDeclsList &Result, DeclContext *DC); + uint64_t + WriteSpecsLookupTable(NamedDecl *D, ChuanqiXu9 wrote: Done https://github.com/llvm/llvm-project/pull/76774 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
ChuanqiXu9 wrote: > I failed to use spr to create stacked review... So I just create the stacked > PR manually: #77417. Luckily the context are remained. I heard the current > context may be lost if we change to use spr now. https://github.com/llvm/llvm-project/pull/76774 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
https://github.com/vgvassilev updated
https://github.com/llvm/llvm-project/pull/76774
>From 50fd47f2bfda527807f8cc5e46425050246868aa Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Wed, 3 Jan 2024 11:33:17 +0800
Subject: [PATCH 1/2] Load Specializations Lazily
---
clang/include/clang/AST/DeclTemplate.h| 35 ++--
clang/include/clang/AST/ExternalASTSource.h | 6 +
clang/include/clang/AST/ODRHash.h | 3 +
.../clang/Sema/MultiplexExternalSemaSource.h | 6 +
.../include/clang/Serialization/ASTBitCodes.h | 3 +
clang/include/clang/Serialization/ASTReader.h | 28 +++
clang/include/clang/Serialization/ASTWriter.h | 6 +
clang/lib/AST/DeclTemplate.cpp| 67 +---
clang/lib/AST/ExternalASTSource.cpp | 5 +
clang/lib/AST/ODRHash.cpp | 2 +
.../lib/Sema/MultiplexExternalSemaSource.cpp | 6 +
clang/lib/Serialization/ASTReader.cpp | 125 +-
clang/lib/Serialization/ASTReaderDecl.cpp | 35 +++-
clang/lib/Serialization/ASTReaderInternals.h | 80 +
clang/lib/Serialization/ASTWriter.cpp | 144 +++-
clang/lib/Serialization/ASTWriterDecl.cpp | 78 ++---
clang/test/Modules/odr_hash.cpp | 4 +-
clang/unittests/Serialization/CMakeLists.txt | 1 +
.../Serialization/LoadSpecLazily.cpp | 159 ++
19 files changed, 728 insertions(+), 65 deletions(-)
create mode 100644 clang/unittests/Serialization/LoadSpecLazily.cpp
diff --git a/clang/include/clang/AST/DeclTemplate.h
b/clang/include/clang/AST/DeclTemplate.h
index 832ad2de6b08a8..4699dd17bc182c 100644
--- a/clang/include/clang/AST/DeclTemplate.h
+++ b/clang/include/clang/AST/DeclTemplate.h
@@ -525,8 +525,11 @@ class FunctionTemplateSpecializationInfo final
return Function.getInt();
}
+ void loadExternalRedecls();
+
public:
friend TrailingObjects;
+ friend class ASTReader;
static FunctionTemplateSpecializationInfo *
Create(ASTContext &C, FunctionDecl *FD, FunctionTemplateDecl *Template,
@@ -789,13 +792,15 @@ class RedeclarableTemplateDecl : public TemplateDecl,
return SpecIterator(isEnd ? Specs.end() : Specs.begin());
}
- void loadLazySpecializationsImpl() const;
+ void loadExternalSpecializations() const;
template
typename SpecEntryTraits::DeclType*
findSpecializationImpl(llvm::FoldingSetVector &Specs,
void *&InsertPos, ProfileArguments &&...ProfileArgs);
+ void loadLazySpecializationsWithArgs(ArrayRef
TemplateArgs);
+
template
void addSpecializationImpl(llvm::FoldingSetVector &Specs,
EntryType *Entry, void *InsertPos);
@@ -814,9 +819,13 @@ class RedeclarableTemplateDecl : public TemplateDecl,
/// If non-null, points to an array of specializations (including
/// partial specializations) known only by their external declaration IDs.
///
+/// These specializations needs to be loaded at once in
+/// loadExternalSpecializations to complete the redecl chain or be
preparing
+/// for template resolution.
+///
/// The first value in the array is the number of specializations/partial
/// specializations that follow.
-uint32_t *LazySpecializations = nullptr;
+uint32_t *ExternalSpecializations = nullptr;
/// The set of "injected" template arguments used within this
/// template.
@@ -850,6 +859,8 @@ class RedeclarableTemplateDecl : public TemplateDecl,
friend class ASTDeclWriter;
friend class ASTReader;
template friend class RedeclarableTemplate;
+ friend class ClassTemplateSpecializationDecl;
+ friend class VarTemplateSpecializationDecl;
/// Retrieves the canonical declaration of this template.
RedeclarableTemplateDecl *getCanonicalDecl() override {
@@ -977,6 +988,7 @@ SpecEntryTraits {
class FunctionTemplateDecl : public RedeclarableTemplateDecl {
protected:
friend class FunctionDecl;
+ friend class FunctionTemplateSpecializationInfo;
/// Data that is common to all of the declarations of a given
/// function template.
@@ -1012,13 +1024,13 @@ class FunctionTemplateDecl : public
RedeclarableTemplateDecl {
void addSpecialization(FunctionTemplateSpecializationInfo* Info,
void *InsertPos);
+ /// Load any lazily-loaded specializations from the external source.
+ void LoadLazySpecializations() const;
+
public:
friend class ASTDeclReader;
friend class ASTDeclWriter;
- /// Load any lazily-loaded specializations from the external source.
- void LoadLazySpecializations() const;
-
/// Get the underlying function declaration of the template.
FunctionDecl *getTemplatedDecl() const {
return static_cast(TemplatedDecl);
@@ -1839,6 +1851,8 @@ class ClassTemplateSpecializationDecl
LLVM_PREFERRED_TYPE(TemplateSpecializationKind)
unsigned SpecializationKind : 3;
+ void loadExternalRedecls();
+
protected:
ClassTemplateSpecializationDecl(A
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
https://github.com/ChuanqiXu9 created
https://github.com/llvm/llvm-project/pull/76774
The idea comes from @vgvassilev and @vgvassilev had patch for it on phab.
Unfortunately phab is closed and I forgot the Dxxx number of that patch. But I
remember the last comment from @vgvassilev is that we should use
MultiOnDiskHashTable for it. So I followed that and rewrite the whole from the
scratch in the new year.
### Background
Currently all the specializations of a template (including instantiation,
specialization and partial specializations) will be loaded at once if we want
to instantiate another instance for the template, or find instantiation for the
template, or just want to complete the redecl chain.
This means basically we need to load every specializations for the template
once the template declaration got loaded. This is bad since when we load a
specialization, we need to load all of its template arguments. Then we have to
deserialize a lot of unnecessary declarations.
For example,
```
// M.cppm
export module M;
export template
class A {};
export class ShouldNotBeLoaded {};
export class Temp {
A AS;
};
// use.cpp
import M;
A a;
```
We should a specialization ` A` in `M.cppm` and we
instantiate the template `A` in `use.cpp`. Then we will deserialize
`ShouldNotBeLoaded` surprisingly when compiling `use.cpp`. And this patch tries
to avoid that.
Given that the templates are heavily used in C++, this is a pain point for the
performance.
### What this patch did
This patch adds MultiOnDiskHashTable for specializations in the ASTReader. Then
we will only deserialize the specializations with the same template arguments.
We made that by using ODRHash for the template arguments as the key of the hash
table.
The partial specializations are not added to the MultiOnDiskHashTable. Since we
can't know if a partial specialization is needed before deciding the template
declaration for a instantiation request. There may be space for further
optimizations, but let's do that in the future.
To review this patch, I think `ASTReaderDecl::AddLazySpecializations` may be a
good entry point.
### What this patch not did
This patch doesn't solve the problem completely. Since we will add `update`
specializations if there are new specializations in a different module:
https://github.com/llvm/llvm-project/blob/8ae73fea3a2cbb072bf3e577dc49deb25b56e760/clang/lib/Serialization/ASTWriterDecl.cpp#L251-L269
That said, we can't handle this case now:
```
// M.cppm
export module M;
export template
class A {};
// N.cppm
export module N;
export import A;
export class ShouldNotBeLoaded {};
export class Temp {
A AS;
};
// use.cpp
import N;
A a;
```
Now `ShouldNotBeLoaded` will still be loaded.
But the current patch is already relatively big. So I want to split it in the
next patch. I think the current patch is already self contained.
>From f4191661961428a0f534f527774ac3d5159c5103 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Tue, 2 Jan 2024 10:43:03 +0800
Subject: [PATCH] Load Specializations Lazily
---
clang/include/clang/AST/DeclTemplate.h| 51 --
clang/include/clang/AST/ExternalASTSource.h | 5 +
clang/include/clang/AST/ODRHash.h | 3 +
.../clang/Sema/MultiplexExternalSemaSource.h | 6 +
.../include/clang/Serialization/ASTBitCodes.h | 3 +
clang/include/clang/Serialization/ASTReader.h | 19 +++
clang/include/clang/Serialization/ASTWriter.h | 6 +
clang/lib/AST/DeclTemplate.cpp| 66 +---
clang/lib/AST/ExternalASTSource.cpp | 5 +
clang/lib/AST/ODRHash.cpp | 2 +
.../lib/Sema/MultiplexExternalSemaSource.cpp | 6 +
clang/lib/Serialization/ASTReader.cpp | 109 +++-
clang/lib/Serialization/ASTReaderDecl.cpp | 33 +++-
clang/lib/Serialization/ASTReaderInternals.h | 80 +
clang/lib/Serialization/ASTWriter.cpp | 149 +++-
clang/lib/Serialization/ASTWriterDecl.cpp | 75 ++---
clang/test/Modules/odr_hash.cpp | 4 +-
.../Modules/static-member-in-templates.cppm | 52 ++
clang/unittests/Serialization/CMakeLists.txt | 1 +
.../Serialization/LoadSpecLazily.cpp | 159 ++
20 files changed, 769 insertions(+), 65 deletions(-)
create mode 100644 clang/test/Modules/static-member-in-templates.cppm
create mode 100644 clang/unittests/Serialization/LoadSpecLazily.cpp
diff --git a/clang/include/clang/AST/DeclTemplate.h
b/clang/include/clang/AST/DeclTemplate.h
index 832ad2de6b08a8..ab380f55c038ee 100644
--- a/clang/include/clang/AST/DeclTemplate.h
+++ b/clang/include/clang/AST/DeclTemplate.h
@@ -30,6 +30,7 @@
#include "llvm/ADT/FoldingSet.h"
#include "llvm/ADT/PointerIntPair.h"
#include "llvm/ADT/PointerUnion.h"
+#include "llvm/ADT/SmallPtrSet.h"
#include "llvm/ADT/iterator.h"
#include "llvm/ADT/iterator_range.h"
#include "llvm/Support/Casting.h"
@@ -525,8 +526,11 @@ class FunctionTemplateSpeci
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
llvmbot wrote: @llvm/pr-subscribers-clang-modules Author: Chuanqi Xu (ChuanqiXu9) Changes The idea comes from @vgvassilev and @vgvassilev had patch for it on phab. Unfortunately phab is closed and I forgot the Dxxx number of that patch. But I remember the last comment from @vgvassilev is that we should use MultiOnDiskHashTable for it. So I followed that and rewrite the whole from the scratch in the new year. ### Background Currently all the specializations of a template (including instantiation, specialization and partial specializations) will be loaded at once if we want to instantiate another instance for the template, or find instantiation for the template, or just want to complete the redecl chain. This means basically we need to load every specializations for the template once the template declaration got loaded. This is bad since when we load a specialization, we need to load all of its template arguments. Then we have to deserialize a lot of unnecessary declarations. For example, ``` // M.cppm export module M; export templateclass A {}; export class ShouldNotBeLoaded {}; export class Temp { A AS; }; // use.cpp import M; A a; ``` We should a specialization ` A ` in `M.cppm` and we instantiate the template `A` in `use.cpp`. Then we will deserialize `ShouldNotBeLoaded` surprisingly when compiling `use.cpp`. And this patch tries to avoid that. Given that the templates are heavily used in C++, this is a pain point for the performance. ### What this patch did This patch adds MultiOnDiskHashTable for specializations in the ASTReader. Then we will only deserialize the specializations with the same template arguments. We made that by using ODRHash for the template arguments as the key of the hash table. The partial specializations are not added to the MultiOnDiskHashTable. Since we can't know if a partial specialization is needed before deciding the template declaration for a instantiation request. There may be space for further optimizations, but let's do that in the future. To review this patch, I think `ASTReaderDecl::AddLazySpecializations` may be a good entry point. ### What this patch not did This patch doesn't solve the problem completely. Since we will add `update` specializations if there are new specializations in a different module: https://github.com/llvm/llvm-project/blob/8ae73fea3a2cbb072bf3e577dc49deb25b56e760/clang/lib/Serialization/ASTWriterDecl.cpp#L251-L269 That said, we can't handle this case now: ``` // M.cppm export module M; export template class A {}; // N.cppm export module N; export import A; export class ShouldNotBeLoaded {}; export class Temp { A AS; }; // use.cpp import N; A a; ``` Now `ShouldNotBeLoaded` will still be loaded. But the current patch is already relatively big. So I want to split it in the next patch. I think the current patch is already self contained. --- Patch is 53.18 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/76774.diff 20 Files Affected: - (modified) clang/include/clang/AST/DeclTemplate.h (+41-10) - (modified) clang/include/clang/AST/ExternalASTSource.h (+5) - (modified) clang/include/clang/AST/ODRHash.h (+3) - (modified) clang/include/clang/Sema/MultiplexExternalSemaSource.h (+6) - (modified) clang/include/clang/Serialization/ASTBitCodes.h (+3) - (modified) clang/include/clang/Serialization/ASTReader.h (+19) - (modified) clang/include/clang/Serialization/ASTWriter.h (+6) - (modified) clang/lib/AST/DeclTemplate.cpp (+45-21) - (modified) clang/lib/AST/ExternalASTSource.cpp (+5) - (modified) clang/lib/AST/ODRHash.cpp (+2) - (modified) clang/lib/Sema/MultiplexExternalSemaSource.cpp (+6) - (modified) clang/lib/Serialization/ASTReader.cpp (+103-6) - (modified) clang/lib/Serialization/ASTReaderDecl.cpp (+28-5) - (modified) clang/lib/Serialization/ASTReaderInternals.h (+80) - (modified) clang/lib/Serialization/ASTWriter.cpp (+148-1) - (modified) clang/lib/Serialization/ASTWriterDecl.cpp (+55-20) - (modified) clang/test/Modules/odr_hash.cpp (+2-2) - (added) clang/test/Modules/static-member-in-templates.cppm (+52) - (modified) clang/unittests/Serialization/CMakeLists.txt (+1) - (added) clang/unittests/Serialization/LoadSpecLazily.cpp (+159) ``diff diff --git a/clang/include/clang/AST/DeclTemplate.h b/clang/include/clang/AST/DeclTemplate.h index 832ad2de6b08a8..ab380f55c038ee 100644 --- a/clang/include/clang/AST/DeclTemplate.h +++ b/clang/include/clang/AST/DeclTemplate.h @@ -30,6 +30,7 @@ #include "llvm/ADT/FoldingSet.h" #include "llvm/ADT/PointerIntPair.h" #include "llvm/ADT/PointerUnion.h" +#include "llvm/ADT/SmallPtrSet.h" #include "llvm/ADT/iterator.h" #include "llvm/ADT/iterator_range.h" #include "llvm/Support/Casting.h" @@ -525,8 +526,11 @@ class FunctionTemplateSpecializationInfo final return Function.getInt(); } +
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/76774 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
https://github.com/ChuanqiXu9 updated
https://github.com/llvm/llvm-project/pull/76774
>From af6f8ca9b739c532a489881245fac1413ec84a07 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Wed, 3 Jan 2024 11:33:17 +0800
Subject: [PATCH] Load Specializations Lazily
---
clang/include/clang/AST/DeclTemplate.h| 37 ++--
clang/include/clang/AST/ExternalASTSource.h | 5 +
clang/include/clang/AST/ODRHash.h | 3 +
.../clang/Sema/MultiplexExternalSemaSource.h | 6 +
.../include/clang/Serialization/ASTBitCodes.h | 3 +
clang/include/clang/Serialization/ASTReader.h | 19 +++
clang/include/clang/Serialization/ASTWriter.h | 6 +
clang/lib/AST/DeclTemplate.cpp| 66 +---
clang/lib/AST/ExternalASTSource.cpp | 5 +
clang/lib/AST/ODRHash.cpp | 2 +
.../lib/Sema/MultiplexExternalSemaSource.cpp | 6 +
clang/lib/Serialization/ASTReader.cpp | 109 +++-
clang/lib/Serialization/ASTReaderDecl.cpp | 34 +++-
clang/lib/Serialization/ASTReaderInternals.h | 80 +
clang/lib/Serialization/ASTWriter.cpp | 149 +++-
clang/lib/Serialization/ASTWriterDecl.cpp | 75 ++---
clang/test/Modules/odr_hash.cpp | 4 +-
clang/unittests/Serialization/CMakeLists.txt | 1 +
.../Serialization/LoadSpecLazily.cpp | 159 ++
19 files changed, 704 insertions(+), 65 deletions(-)
create mode 100644 clang/unittests/Serialization/LoadSpecLazily.cpp
diff --git a/clang/include/clang/AST/DeclTemplate.h
b/clang/include/clang/AST/DeclTemplate.h
index 832ad2de6b08a8..3e0a89a92b020d 100644
--- a/clang/include/clang/AST/DeclTemplate.h
+++ b/clang/include/clang/AST/DeclTemplate.h
@@ -525,8 +525,11 @@ class FunctionTemplateSpecializationInfo final
return Function.getInt();
}
+ void loadExternalRedecls();
+
public:
friend TrailingObjects;
+ friend class ASTReader;
static FunctionTemplateSpecializationInfo *
Create(ASTContext &C, FunctionDecl *FD, FunctionTemplateDecl *Template,
@@ -789,13 +792,15 @@ class RedeclarableTemplateDecl : public TemplateDecl,
return SpecIterator(isEnd ? Specs.end() : Specs.begin());
}
- void loadLazySpecializationsImpl() const;
+ void loadExternalSpecializations() const;
template
typename SpecEntryTraits::DeclType*
findSpecializationImpl(llvm::FoldingSetVector &Specs,
void *&InsertPos, ProfileArguments &&...ProfileArgs);
+ void loadLazySpecializationsWithArgs(ArrayRef
TemplateArgs);
+
template
void addSpecializationImpl(llvm::FoldingSetVector &Specs,
EntryType *Entry, void *InsertPos);
@@ -814,9 +819,13 @@ class RedeclarableTemplateDecl : public TemplateDecl,
/// If non-null, points to an array of specializations (including
/// partial specializations) known only by their external declaration IDs.
///
+/// These specializations needs to be loaded at once in
+/// loadExternalSpecializations to complete the redecl chain or be
preparing
+/// for template resolution.
+///
/// The first value in the array is the number of specializations/partial
/// specializations that follow.
-uint32_t *LazySpecializations = nullptr;
+uint32_t *ExternalSpecializations = nullptr;
/// The set of "injected" template arguments used within this
/// template.
@@ -850,6 +859,8 @@ class RedeclarableTemplateDecl : public TemplateDecl,
friend class ASTDeclWriter;
friend class ASTReader;
template friend class RedeclarableTemplate;
+ friend class ClassTemplateSpecializationDecl;
+ friend class VarTemplateSpecializationDecl;
/// Retrieves the canonical declaration of this template.
RedeclarableTemplateDecl *getCanonicalDecl() override {
@@ -977,6 +988,7 @@ SpecEntryTraits {
class FunctionTemplateDecl : public RedeclarableTemplateDecl {
protected:
friend class FunctionDecl;
+ friend class FunctionTemplateSpecializationInfo;
/// Data that is common to all of the declarations of a given
/// function template.
@@ -1012,13 +1024,13 @@ class FunctionTemplateDecl : public
RedeclarableTemplateDecl {
void addSpecialization(FunctionTemplateSpecializationInfo* Info,
void *InsertPos);
+ /// Load any lazily-loaded specializations from the external source.
+ void LoadLazySpecializations() const;
+
public:
friend class ASTDeclReader;
friend class ASTDeclWriter;
- /// Load any lazily-loaded specializations from the external source.
- void LoadLazySpecializations() const;
-
/// Get the underlying function declaration of the template.
FunctionDecl *getTemplatedDecl() const {
return static_cast(TemplatedDecl);
@@ -1839,6 +1851,8 @@ class ClassTemplateSpecializationDecl
LLVM_PREFERRED_TYPE(TemplateSpecializationKind)
unsigned SpecializationKind : 3;
+ void loadExternalRedecls();
+
protected:
ClassTemplateSpecializationDecl(ASTCont
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff 7a3b0cbb143d02b70b2bfae5cd40e9867c124748
af6f8ca9b739c532a489881245fac1413ec84a07 --
clang/unittests/Serialization/LoadSpecLazily.cpp
clang/include/clang/AST/DeclTemplate.h
clang/include/clang/AST/ExternalASTSource.h clang/include/clang/AST/ODRHash.h
clang/include/clang/Sema/MultiplexExternalSemaSource.h
clang/include/clang/Serialization/ASTBitCodes.h
clang/include/clang/Serialization/ASTReader.h
clang/include/clang/Serialization/ASTWriter.h clang/lib/AST/DeclTemplate.cpp
clang/lib/AST/ExternalASTSource.cpp clang/lib/AST/ODRHash.cpp
clang/lib/Sema/MultiplexExternalSemaSource.cpp
clang/lib/Serialization/ASTReader.cpp clang/lib/Serialization/ASTReaderDecl.cpp
clang/lib/Serialization/ASTReaderInternals.h
clang/lib/Serialization/ASTWriter.cpp clang/lib/Serialization/ASTWriterDecl.cpp
clang/test/Modules/odr_hash.cpp
``
View the diff from clang-format here.
``diff
diff --git a/clang/include/clang/AST/DeclTemplate.h
b/clang/include/clang/AST/DeclTemplate.h
index 3e0a89a92b..4699dd17bc 100644
--- a/clang/include/clang/AST/DeclTemplate.h
+++ b/clang/include/clang/AST/DeclTemplate.h
@@ -2253,7 +2253,6 @@ public:
/// Declaration of a class template.
class ClassTemplateDecl : public RedeclarableTemplateDecl {
protected:
-
/// Data that is common to all of the declarations of a given
/// class template.
struct Common : CommonBase {
@@ -3035,7 +3034,6 @@ public:
/// Declaration of a variable template.
class VarTemplateDecl : public RedeclarableTemplateDecl {
protected:
-
/// Data that is common to all of the declarations of a given
/// variable template.
struct Common : CommonBase {
``
https://github.com/llvm/llvm-project/pull/76774
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
https://github.com/ChuanqiXu9 updated
https://github.com/llvm/llvm-project/pull/76774
>From 79cefc9f0f006acd788b6ac4e240c17d9deadf13 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Wed, 3 Jan 2024 11:33:17 +0800
Subject: [PATCH] Load Specializations Lazily
---
clang/include/clang/AST/DeclTemplate.h| 35 ++--
clang/include/clang/AST/ExternalASTSource.h | 5 +
clang/include/clang/AST/ODRHash.h | 3 +
.../clang/Sema/MultiplexExternalSemaSource.h | 6 +
.../include/clang/Serialization/ASTBitCodes.h | 3 +
clang/include/clang/Serialization/ASTReader.h | 19 +++
clang/include/clang/Serialization/ASTWriter.h | 6 +
clang/lib/AST/DeclTemplate.cpp| 66 +---
clang/lib/AST/ExternalASTSource.cpp | 5 +
clang/lib/AST/ODRHash.cpp | 2 +
.../lib/Sema/MultiplexExternalSemaSource.cpp | 6 +
clang/lib/Serialization/ASTReader.cpp | 109 +++-
clang/lib/Serialization/ASTReaderDecl.cpp | 34 +++-
clang/lib/Serialization/ASTReaderInternals.h | 80 +
clang/lib/Serialization/ASTWriter.cpp | 149 +++-
clang/lib/Serialization/ASTWriterDecl.cpp | 75 ++---
clang/test/Modules/odr_hash.cpp | 4 +-
clang/unittests/Serialization/CMakeLists.txt | 1 +
.../Serialization/LoadSpecLazily.cpp | 159 ++
19 files changed, 702 insertions(+), 65 deletions(-)
create mode 100644 clang/unittests/Serialization/LoadSpecLazily.cpp
diff --git a/clang/include/clang/AST/DeclTemplate.h
b/clang/include/clang/AST/DeclTemplate.h
index 832ad2de6b08a8..4699dd17bc182c 100644
--- a/clang/include/clang/AST/DeclTemplate.h
+++ b/clang/include/clang/AST/DeclTemplate.h
@@ -525,8 +525,11 @@ class FunctionTemplateSpecializationInfo final
return Function.getInt();
}
+ void loadExternalRedecls();
+
public:
friend TrailingObjects;
+ friend class ASTReader;
static FunctionTemplateSpecializationInfo *
Create(ASTContext &C, FunctionDecl *FD, FunctionTemplateDecl *Template,
@@ -789,13 +792,15 @@ class RedeclarableTemplateDecl : public TemplateDecl,
return SpecIterator(isEnd ? Specs.end() : Specs.begin());
}
- void loadLazySpecializationsImpl() const;
+ void loadExternalSpecializations() const;
template
typename SpecEntryTraits::DeclType*
findSpecializationImpl(llvm::FoldingSetVector &Specs,
void *&InsertPos, ProfileArguments &&...ProfileArgs);
+ void loadLazySpecializationsWithArgs(ArrayRef
TemplateArgs);
+
template
void addSpecializationImpl(llvm::FoldingSetVector &Specs,
EntryType *Entry, void *InsertPos);
@@ -814,9 +819,13 @@ class RedeclarableTemplateDecl : public TemplateDecl,
/// If non-null, points to an array of specializations (including
/// partial specializations) known only by their external declaration IDs.
///
+/// These specializations needs to be loaded at once in
+/// loadExternalSpecializations to complete the redecl chain or be
preparing
+/// for template resolution.
+///
/// The first value in the array is the number of specializations/partial
/// specializations that follow.
-uint32_t *LazySpecializations = nullptr;
+uint32_t *ExternalSpecializations = nullptr;
/// The set of "injected" template arguments used within this
/// template.
@@ -850,6 +859,8 @@ class RedeclarableTemplateDecl : public TemplateDecl,
friend class ASTDeclWriter;
friend class ASTReader;
template friend class RedeclarableTemplate;
+ friend class ClassTemplateSpecializationDecl;
+ friend class VarTemplateSpecializationDecl;
/// Retrieves the canonical declaration of this template.
RedeclarableTemplateDecl *getCanonicalDecl() override {
@@ -977,6 +988,7 @@ SpecEntryTraits {
class FunctionTemplateDecl : public RedeclarableTemplateDecl {
protected:
friend class FunctionDecl;
+ friend class FunctionTemplateSpecializationInfo;
/// Data that is common to all of the declarations of a given
/// function template.
@@ -1012,13 +1024,13 @@ class FunctionTemplateDecl : public
RedeclarableTemplateDecl {
void addSpecialization(FunctionTemplateSpecializationInfo* Info,
void *InsertPos);
+ /// Load any lazily-loaded specializations from the external source.
+ void LoadLazySpecializations() const;
+
public:
friend class ASTDeclReader;
friend class ASTDeclWriter;
- /// Load any lazily-loaded specializations from the external source.
- void LoadLazySpecializations() const;
-
/// Get the underlying function declaration of the template.
FunctionDecl *getTemplatedDecl() const {
return static_cast(TemplatedDecl);
@@ -1839,6 +1851,8 @@ class ClassTemplateSpecializationDecl
LLVM_PREFERRED_TYPE(TemplateSpecializationKind)
unsigned SpecializationKind : 3;
+ void loadExternalRedecls();
+
protected:
ClassTemplateSpecializationDecl(ASTCont
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
https://github.com/vgvassilev edited https://github.com/llvm/llvm-project/pull/76774 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
https://github.com/vgvassilev commented: This is a great way to start a new year ;) The phab link is https://reviews.llvm.org/D41416. In general I was wondering could we simplify the implementation by loading the specialization hash table upon module load. That should be relatively cheap as we will read 2 integers per specialization. Perhaps we should put both patches together and that'd allow us to test them if they are on par with https://reviews.llvm.org/D41416 which we use downstream. Thanks for working on this! https://github.com/llvm/llvm-project/pull/76774 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
@@ -1249,3 +1249,5 @@ void ODRHash::AddQualType(QualType T) {
void ODRHash::AddBoolean(bool Value) {
Bools.push_back(Value);
}
+
+void ODRHash::AddInteger(unsigned Value) { ID.AddInteger(Value); }
vgvassilev wrote:
I remember @hahnjo and @zygoloid discussing that the odr-hasher is probably not
the best way to has template arguments because the hasher would not take into
account semantic aspects of template arguments. For example, a fully qualified
template argument would not compare the same to a non-qualified one. We might
need to implement our own folding set logic.
@hahnjo, could you help me out dig that discussion.
https://github.com/llvm/llvm-project/pull/76774
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
@@ -2431,10 +2434,14 @@ void
ASTDeclReader::VisitClassTemplateDecl(ClassTemplateDecl *D) {
mergeRedeclarableTemplate(D, Redecl);
if (ThisDeclID == Redecl.getFirstID()) {
-// This ClassTemplateDecl owns a CommonPtr; read it to keep track of all of
-// the specializations.
+// This ClassTemplateDecl owns a CommonPtr; read it to keep track of all
+// of the specializations.
SmallVector SpecIDs;
readDeclIDList(SpecIDs);
+
+if (Record.readInt())
+ ReadDeclsSpecs(*Loc.F, D, Loc.F->DeclsCursor);
vgvassilev wrote:
What if the TemplateDecl came from a different module file and this module file
contains only specializations?
https://github.com/llvm/llvm-project/pull/76774
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
@@ -150,6 +150,11 @@ class ExternalASTSource : public
RefCountedBase {
virtual bool
FindExternalVisibleDeclsByName(const DeclContext *DC, DeclarationName Name);
+ /// Load all the external specialzations for the Decl and the corresponding
+ /// template arguments.
+ virtual void LoadExternalSpecs(const Decl *D,
vgvassilev wrote:
```suggestion
virtual void FindExternalSpecialization(const Decl *D,
```
sounds more consistent to the surroundings here.
https://github.com/llvm/llvm-project/pull/76774
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
@@ -527,6 +527,10 @@ class ASTWriter : public ASTDeserializationListener, bool isLookupResultExternal(StoredDeclsList &Result, DeclContext *DC); bool isLookupResultEntirelyExternal(StoredDeclsList &Result, DeclContext *DC); + uint64_t + WriteSpecsLookupTable(NamedDecl *D, vgvassilev wrote: Generally `spec` would read as specification not specialization. Maybe we should use the full word. https://github.com/llvm/llvm-project/pull/76774 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
