[PATCH] D29951: Load lazily the template specialization in multi-module setups.
v.g.vassilev added a comment. For the record: relanded in r306903. https://reviews.llvm.org/D29951 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29951: Load lazily the template specialization in multi-module setups.
v.g.vassilev added a comment. Reverted in r305460 because broke libcxx modules builds. https://reviews.llvm.org/D29951 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29951: Load lazily the template specialization in multi-module setups.
v.g.vassilev closed this revision. v.g.vassilev added a comment. r305120. https://reviews.llvm.org/D29951 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29951: Load lazily the template specialization in multi-module setups.
rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. Looks great, thanks! https://reviews.llvm.org/D29951 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29951: Load lazily the template specialization in multi-module setups.
v.g.vassilev updated this revision to Diff 102045.
v.g.vassilev marked 2 inline comments as done.
v.g.vassilev added a comment.
Address comments.
https://reviews.llvm.org/D29951
Files:
include/clang/Serialization/ASTReader.h
lib/Serialization/ASTReaderDecl.cpp
Index: lib/Serialization/ASTReaderDecl.cpp
===
--- lib/Serialization/ASTReaderDecl.cpp
+++ lib/Serialization/ASTReaderDecl.cpp
@@ -216,6 +216,30 @@
TypedefNameForLinkage(nullptr), HasPendingBody(false),
IsDeclMarkedUsed(false) {}
+template static
+void AddLazySpecializations(T *D,
+SmallVectorImpl& IDs) {
+ if (IDs.empty())
+return;
+
+ // FIXME: We should avoid this pattern of getting the ASTContext.
+ ASTContext = D->getASTContext();
+
+ auto * = D->getCommonPtr()->LazySpecializations;
+
+ if (auto = LazySpecializations) {
+IDs.insert(IDs.end(), Old + 1, Old + 1 + Old[0]);
+std::sort(IDs.begin(), IDs.end());
+IDs.erase(std::unique(IDs.begin(), IDs.end()), IDs.end());
+ }
+
+ auto *Result = new (C) serialization::DeclID[1 + IDs.size()];
+ *Result = IDs.size();
+ std::copy(IDs.begin(), IDs.end(), Result + 1);
+
+ LazySpecializations = Result;
+}
+
template
static Decl *getMostRecentDeclImpl(Redeclarable *D);
static Decl *getMostRecentDeclImpl(...);
@@ -244,7 +268,7 @@
void ReadFunctionDefinition(FunctionDecl *FD);
void Visit(Decl *D);
-void UpdateDecl(Decl *D);
+void UpdateDecl(Decl *D, llvm::SmallVectorImpl&);
static void setNextObjCCategory(ObjCCategoryDecl *Cat,
ObjCCategoryDecl *Next) {
@@ -1951,35 +1975,15 @@
return Redecl;
}
-static DeclID *newDeclIDList(ASTContext , DeclID *Old,
- SmallVectorImpl ) {
- assert(!IDs.empty() && "no IDs to add to list");
- if (Old) {
-IDs.insert(IDs.end(), Old + 1, Old + 1 + Old[0]);
-std::sort(IDs.begin(), IDs.end());
-IDs.erase(std::unique(IDs.begin(), IDs.end()), IDs.end());
- }
-
- auto *Result = new (Context) DeclID[1 + IDs.size()];
- *Result = IDs.size();
- std::copy(IDs.begin(), IDs.end(), Result + 1);
- return Result;
-}
-
void ASTDeclReader::VisitClassTemplateDecl(ClassTemplateDecl *D) {
RedeclarableResult Redecl = VisitRedeclarableTemplateDecl(D);
if (ThisDeclID == Redecl.getFirstID()) {
// This ClassTemplateDecl owns a CommonPtr; read it to keep track of all of
// the specializations.
SmallVector SpecIDs;
ReadDeclIDList(SpecIDs);
-
-if (!SpecIDs.empty()) {
- auto *CommonPtr = D->getCommonPtr();
- CommonPtr->LazySpecializations = newDeclIDList(
- Reader.getContext(), CommonPtr->LazySpecializations, SpecIDs);
-}
+ASTDeclReader::AddLazySpecializations(D, SpecIDs);
}
if (D->getTemplatedDecl()->TemplateOrInstantiation) {
@@ -2006,12 +2010,7 @@
// the specializations.
SmallVector SpecIDs;
ReadDeclIDList(SpecIDs);
-
-if (!SpecIDs.empty()) {
- auto *CommonPtr = D->getCommonPtr();
- CommonPtr->LazySpecializations = newDeclIDList(
- Reader.getContext(), CommonPtr->LazySpecializations, SpecIDs);
-}
+ASTDeclReader::AddLazySpecializations(D, SpecIDs);
}
}
@@ -2117,12 +2116,7 @@
// This FunctionTemplateDecl owns a CommonPtr; read it.
SmallVector SpecIDs;
ReadDeclIDList(SpecIDs);
-
-if (!SpecIDs.empty()) {
- auto *CommonPtr = D->getCommonPtr();
- CommonPtr->LazySpecializations = newDeclIDList(
- Reader.getContext(), CommonPtr->LazySpecializations, SpecIDs);
-}
+ASTDeclReader::AddLazySpecializations(D, SpecIDs);
}
}
@@ -3666,6 +3660,9 @@
Decl *D = Record.D;
ProcessingUpdatesRAIIObj ProcessingUpdates(*this);
DeclUpdateOffsetsMap::iterator UpdI = DeclUpdateOffsets.find(ID);
+
+ llvm::SmallVector PendingLazySpecializationIDs;
+
if (UpdI != DeclUpdateOffsets.end()) {
auto UpdateOffsets = std::move(UpdI->second);
DeclUpdateOffsets.erase(UpdI);
@@ -3690,7 +3687,7 @@
ASTDeclReader Reader(*this, Record, RecordLocation(F, Offset), ID,
SourceLocation());
- Reader.UpdateDecl(D);
+ Reader.UpdateDecl(D, PendingLazySpecializationIDs);
// We might have made this declaration interesting. If so, remember that
// we need to hand it off to the consumer.
@@ -3702,6 +3699,17 @@
}
}
}
+ // Add the lazy specializations to the template.
+ assert((PendingLazySpecializationIDs.empty() || isa(D) ||
+ isa(D) || isa(D)) &&
+ "Must not have pending specializations");
+ if (auto *CTD = dyn_cast(D))
+ASTDeclReader::AddLazySpecializations(CTD, PendingLazySpecializationIDs);
+ else if (auto *FTD =
[PATCH] D29951: Load lazily the template specialization in multi-module setups.
v.g.vassilev updated this revision to Diff 98445.
v.g.vassilev marked 2 inline comments as done.
v.g.vassilev added a comment.
Reduce the amount of calls to AddLazySpecializations. Remove assert.
In order to create a reasonable test I need to use
`-error-on-deserialized-decl` and I hit a bug:
https://bugs.llvm.org/show_bug.cgi?id=32988
Richard, could you help me out with the test? Maybe we could trigger this
avoiding the abovementioned bug...
https://reviews.llvm.org/D29951
Files:
include/clang/Serialization/ASTReader.h
lib/Serialization/ASTReaderDecl.cpp
test/Modules/Inputs/lazy-template-specializations/A.h
test/Modules/Inputs/lazy-template-specializations/B.h
test/Modules/Inputs/lazy-template-specializations/module.modulemap
test/Modules/lazy-template-specializations.cpp
Index: test/Modules/lazy-template-specializations.cpp
===
--- /dev/null
+++ test/Modules/lazy-template-specializations.cpp
@@ -0,0 +1,10 @@
+// RUN: rm -fr %t
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t \
+// RUN: -I %S/Inputs/lazy-template-specializations -std=c++11 \
+// RUN: -error-on-deserialized-decl S -Rmodule-build %s
+
+
+#include "A.h"
+
+namespace N {
+}
Index: test/Modules/Inputs/lazy-template-specializations/module.modulemap
===
--- /dev/null
+++ test/Modules/Inputs/lazy-template-specializations/module.modulemap
@@ -0,0 +1,9 @@
+module A {
+ header "A.h"
+ export *
+}
+
+module B {
+ header "B.h"
+ export *
+}
Index: test/Modules/Inputs/lazy-template-specializations/B.h
===
--- /dev/null
+++ test/Modules/Inputs/lazy-template-specializations/B.h
@@ -0,0 +1,4 @@
+namespace N {
+ template struct S { int a; };
+ template <> struct S { double b; };
+}
Index: test/Modules/Inputs/lazy-template-specializations/A.h
===
--- /dev/null
+++ test/Modules/Inputs/lazy-template-specializations/A.h
@@ -0,0 +1,3 @@
+#include "B.h"
+int i;
+
Index: lib/Serialization/ASTReaderDecl.cpp
===
--- lib/Serialization/ASTReaderDecl.cpp
+++ lib/Serialization/ASTReaderDecl.cpp
@@ -1951,35 +1951,15 @@
return Redecl;
}
-static DeclID *newDeclIDList(ASTContext , DeclID *Old,
- SmallVectorImpl ) {
- assert(!IDs.empty() && "no IDs to add to list");
- if (Old) {
-IDs.insert(IDs.end(), Old + 1, Old + 1 + Old[0]);
-std::sort(IDs.begin(), IDs.end());
-IDs.erase(std::unique(IDs.begin(), IDs.end()), IDs.end());
- }
-
- auto *Result = new (Context) DeclID[1 + IDs.size()];
- *Result = IDs.size();
- std::copy(IDs.begin(), IDs.end(), Result + 1);
- return Result;
-}
-
void ASTDeclReader::VisitClassTemplateDecl(ClassTemplateDecl *D) {
RedeclarableResult Redecl = VisitRedeclarableTemplateDecl(D);
if (ThisDeclID == Redecl.getFirstID()) {
// This ClassTemplateDecl owns a CommonPtr; read it to keep track of all of
// the specializations.
SmallVector SpecIDs;
ReadDeclIDList(SpecIDs);
-
-if (!SpecIDs.empty()) {
- auto *CommonPtr = D->getCommonPtr();
- CommonPtr->LazySpecializations = newDeclIDList(
- Reader.getContext(), CommonPtr->LazySpecializations, SpecIDs);
-}
+Reader.AddLazySpecializations(D, SpecIDs);
}
if (D->getTemplatedDecl()->TemplateOrInstantiation) {
@@ -2006,12 +1986,7 @@
// the specializations.
SmallVector SpecIDs;
ReadDeclIDList(SpecIDs);
-
-if (!SpecIDs.empty()) {
- auto *CommonPtr = D->getCommonPtr();
- CommonPtr->LazySpecializations = newDeclIDList(
- Reader.getContext(), CommonPtr->LazySpecializations, SpecIDs);
-}
+Reader.AddLazySpecializations(D, SpecIDs);
}
}
@@ -2117,12 +2092,7 @@
// This FunctionTemplateDecl owns a CommonPtr; read it.
SmallVector SpecIDs;
ReadDeclIDList(SpecIDs);
-
-if (!SpecIDs.empty()) {
- auto *CommonPtr = D->getCommonPtr();
- CommonPtr->LazySpecializations = newDeclIDList(
- Reader.getContext(), CommonPtr->LazySpecializations, SpecIDs);
-}
+Reader.AddLazySpecializations(D, SpecIDs);
}
}
@@ -3697,6 +3667,17 @@
}
}
}
+ // Add the lazy specializations to the template.
+ assert((PendingLazySpecializationIDs.empty() || isa(D) ||
+ isa(D) || isa(D)) &&
+ "Must not have pending specializations");
+ if (auto *CTD = dyn_cast(D))
+AddLazySpecializations(CTD, PendingLazySpecializationIDs);
+ else if (auto *FTD = dyn_cast(D))
+AddLazySpecializations(FTD, PendingLazySpecializationIDs);
+ else if (auto *VTD = dyn_cast(D))
+AddLazySpecializations(VTD, PendingLazySpecializationIDs);
+
[PATCH] D29951: Load lazily the template specialization in multi-module setups.
rsmith added inline comments.
Comment at: lib/Serialization/ASTReaderDecl.cpp:213-215
+ assert(isa(D) ||
+ isa(D) &&
+ "Decl doesn't have specializations.");
Can this ever fail at runtime? I'd expect the below code to not compile if `D`
isn't one of these types.
Comment at: lib/Serialization/ASTReaderDecl.cpp:3911-3924
+case UPD_CXX_ADDED_TEMPLATE_SPECIALIZATION: {
+ // It will be added to the template's lazy specialization set when
loaded.
+ SmallVector SpecID;
+ SpecID.push_back(ReadDeclID());
+ if (auto *CTD = dyn_cast(D))
+AddLazySpecializations(CTD, SpecID);
+ else if (auto *FTD = dyn_cast(D))
This will end up being quadratic time and using quadratic storage if a module
adds N specializations to a template; instead, please move the SmallVector out
of the loop and call `AddLazySpecializations` once after reading all the update
records for the declaration. (It's probably best to move it all the way out to
`loadDeclUpdateRecords`, in case we have a large number of modules each adding
some specializations -- eg, if module A imports std::vector and declares A,
module B declares B and uses vector, module C declares C and uses vector,
..., we again should avoid quadratic behavior.)
https://reviews.llvm.org/D29951
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29951: Load lazily the template specialization in multi-module setups.
v.g.vassilev updated this revision to Diff 98333.
v.g.vassilev marked an inline comment as done.
v.g.vassilev added a comment.
Reduce code duplication. Use llvm_unreachable.
https://reviews.llvm.org/D29951
Files:
lib/Serialization/ASTReaderDecl.cpp
Index: lib/Serialization/ASTReaderDecl.cpp
===
--- lib/Serialization/ASTReaderDecl.cpp
+++ lib/Serialization/ASTReaderDecl.cpp
@@ -206,6 +206,30 @@
DeclContext *DC);
FindExistingResult findExisting(NamedDecl *D);
+template
+void AddLazySpecializations(T *D,
+SmallVectorImpl& IDs) {
+ assert(!IDs.empty() && "no IDs to add to list");
+ assert(isa(D) ||
+ isa(D) &&
+ "Decl doesn't have specializations.");
+
+ auto *CommonPtr = D->getCommonPtr();
+ ASTContext = Reader.getContext();
+
+ if (auto = CommonPtr->LazySpecializations) {
+IDs.insert(IDs.end(), Old + 1, Old + 1 + Old[0]);
+std::sort(IDs.begin(), IDs.end());
+IDs.erase(std::unique(IDs.begin(), IDs.end()), IDs.end());
+ }
+
+ auto *Result = new (C) DeclID[1 + IDs.size()];
+ *Result = IDs.size();
+ std::copy(IDs.begin(), IDs.end(), Result + 1);
+
+ CommonPtr->LazySpecializations = Result;
+}
+
public:
ASTDeclReader(ASTReader , ASTRecordReader ,
ASTReader::RecordLocation Loc,
@@ -1951,21 +1975,6 @@
return Redecl;
}
-static DeclID *newDeclIDList(ASTContext , DeclID *Old,
- SmallVectorImpl ) {
- assert(!IDs.empty() && "no IDs to add to list");
- if (Old) {
-IDs.insert(IDs.end(), Old + 1, Old + 1 + Old[0]);
-std::sort(IDs.begin(), IDs.end());
-IDs.erase(std::unique(IDs.begin(), IDs.end()), IDs.end());
- }
-
- auto *Result = new (Context) DeclID[1 + IDs.size()];
- *Result = IDs.size();
- std::copy(IDs.begin(), IDs.end(), Result + 1);
- return Result;
-}
-
void ASTDeclReader::VisitClassTemplateDecl(ClassTemplateDecl *D) {
RedeclarableResult Redecl = VisitRedeclarableTemplateDecl(D);
@@ -1975,11 +1984,8 @@
SmallVector SpecIDs;
ReadDeclIDList(SpecIDs);
-if (!SpecIDs.empty()) {
- auto *CommonPtr = D->getCommonPtr();
- CommonPtr->LazySpecializations = newDeclIDList(
- Reader.getContext(), CommonPtr->LazySpecializations, SpecIDs);
-}
+if (!SpecIDs.empty())
+ AddLazySpecializations(D, SpecIDs);
}
if (D->getTemplatedDecl()->TemplateOrInstantiation) {
@@ -2007,11 +2013,8 @@
SmallVector SpecIDs;
ReadDeclIDList(SpecIDs);
-if (!SpecIDs.empty()) {
- auto *CommonPtr = D->getCommonPtr();
- CommonPtr->LazySpecializations = newDeclIDList(
- Reader.getContext(), CommonPtr->LazySpecializations, SpecIDs);
-}
+if (!SpecIDs.empty())
+ AddLazySpecializations(D, SpecIDs);
}
}
@@ -2118,11 +2121,8 @@
SmallVector SpecIDs;
ReadDeclIDList(SpecIDs);
-if (!SpecIDs.empty()) {
- auto *CommonPtr = D->getCommonPtr();
- CommonPtr->LazySpecializations = newDeclIDList(
- Reader.getContext(), CommonPtr->LazySpecializations, SpecIDs);
-}
+if (!SpecIDs.empty())
+ AddLazySpecializations(D, SpecIDs);
}
}
@@ -3908,9 +3908,19 @@
break;
}
-case UPD_CXX_ADDED_TEMPLATE_SPECIALIZATION:
- // It will be added to the template's specializations set when loaded.
- (void)Record.readDecl();
+case UPD_CXX_ADDED_TEMPLATE_SPECIALIZATION: {
+ // It will be added to the template's lazy specialization set when loaded.
+ SmallVector SpecID;
+ SpecID.push_back(ReadDeclID());
+ if (auto *CTD = dyn_cast(D))
+AddLazySpecializations(CTD, SpecID);
+ else if (auto *FTD = dyn_cast(D))
+AddLazySpecializations(FTD, SpecID);
+ else if (auto *VTD = dyn_cast(D))
+AddLazySpecializations(VTD, SpecID);
+ else // TypeAliasTemplateDecl
+llvm_unreachable("TypeAliasTemplateDecl doesn't have specs!");
+}
break;
case UPD_CXX_ADDED_ANONYMOUS_NAMESPACE: {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29951: Load lazily the template specialization in multi-module setups.
aprantl added inline comments. Comment at: lib/Serialization/ASTReaderDecl.cpp:3814 + } else // TypeAliasTemplateDecl +assert(0 && "TypeAliasTemplateDecl doesn't have specs!"); +} llvm_unreachable() Repository: rL LLVM https://reviews.llvm.org/D29951 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29951: Load lazily the template specialization in multi-module setups.
v.g.vassilev created this revision.
Currently, we load all template specialization if we have more than one module
attached and we touch anything around the template definition.
This patch registers the template specializations as a lazily-loadable
entities. This reduces the amount of deserializations by 1%.
Repository:
rL LLVM
https://reviews.llvm.org/D29951
Files:
lib/Serialization/ASTReaderDecl.cpp
Index: lib/Serialization/ASTReaderDecl.cpp
===
--- lib/Serialization/ASTReaderDecl.cpp
+++ lib/Serialization/ASTReaderDecl.cpp
@@ -3788,9 +3788,31 @@
break;
}
-case UPD_CXX_ADDED_TEMPLATE_SPECIALIZATION:
- // It will be added to the template's specializations set when loaded.
- (void)Record.readDecl();
+case UPD_CXX_ADDED_TEMPLATE_SPECIALIZATION: {
+ // It will be added to the template's lazy specialization set when
loaded.
+ // FIXME: reduce the copy paste.
+ SmallVector SpecID;
+ if (auto *CTD = dyn_cast(D)) {
+SpecID.push_back(ReadDeclID());
+auto *CommonPtr = CTD->getCommonPtr();
+CommonPtr->LazySpecializations = newDeclIDList(
+Reader.getContext(), CommonPtr->LazySpecializations, SpecID);
+
+ } else if (auto *FTD = dyn_cast(D)) {
+SpecID.push_back(ReadDeclID());
+auto *CommonPtr = FTD->getCommonPtr();
+CommonPtr->LazySpecializations = newDeclIDList(
+Reader.getContext(), CommonPtr->LazySpecializations, SpecID);
+
+ } else if (auto *VTD = dyn_cast(D)) {
+SpecID.push_back(ReadDeclID());
+auto *CommonPtr = VTD->getCommonPtr();
+CommonPtr->LazySpecializations = newDeclIDList(
+Reader.getContext(), CommonPtr->LazySpecializations, SpecID);
+
+ } else // TypeAliasTemplateDecl
+assert(0 && "TypeAliasTemplateDecl doesn't have specs!");
+}
break;
case UPD_CXX_ADDED_ANONYMOUS_NAMESPACE: {
Index: lib/Serialization/ASTReaderDecl.cpp
===
--- lib/Serialization/ASTReaderDecl.cpp
+++ lib/Serialization/ASTReaderDecl.cpp
@@ -3788,9 +3788,31 @@
break;
}
-case UPD_CXX_ADDED_TEMPLATE_SPECIALIZATION:
- // It will be added to the template's specializations set when loaded.
- (void)Record.readDecl();
+case UPD_CXX_ADDED_TEMPLATE_SPECIALIZATION: {
+ // It will be added to the template's lazy specialization set when loaded.
+ // FIXME: reduce the copy paste.
+ SmallVector SpecID;
+ if (auto *CTD = dyn_cast(D)) {
+SpecID.push_back(ReadDeclID());
+auto *CommonPtr = CTD->getCommonPtr();
+CommonPtr->LazySpecializations = newDeclIDList(
+Reader.getContext(), CommonPtr->LazySpecializations, SpecID);
+
+ } else if (auto *FTD = dyn_cast(D)) {
+SpecID.push_back(ReadDeclID());
+auto *CommonPtr = FTD->getCommonPtr();
+CommonPtr->LazySpecializations = newDeclIDList(
+Reader.getContext(), CommonPtr->LazySpecializations, SpecID);
+
+ } else if (auto *VTD = dyn_cast(D)) {
+SpecID.push_back(ReadDeclID());
+auto *CommonPtr = VTD->getCommonPtr();
+CommonPtr->LazySpecializations = newDeclIDList(
+Reader.getContext(), CommonPtr->LazySpecializations, SpecID);
+
+ } else // TypeAliasTemplateDecl
+assert(0 && "TypeAliasTemplateDecl doesn't have specs!");
+}
break;
case UPD_CXX_ADDED_ANONYMOUS_NAMESPACE: {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
