llunak created this revision.
llunak added reviewers: rsmith, dblaikie, hans.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
This is a WIP patch that can significantly reduce build time by avoiding
needlessly repeatedly instantiating templates when using PCH.
As described in http://lists.llvm.org/pipermail/cfe-dev/2019-May/062371.html /
http://llunak.blogspot.com/2019/05/why-precompiled-headers-do-not-improve.html
, whenever a compilation uses PCH it instantiates every template that's been
instantiated in the PCH, which can add significant build time. This patch
allows avoiding that, if -building-pch-with-obj option is used, the templates
will be instantiated just once in the object file that should accompany the
PCH, and then it can be skipped during normal compilations.
Testcase: I've built an entire debug build of LibreOffice with this patch and
it works.
Pending problems/questions:
- The patch currently skips C++ constructors and destructors, because (at least
with the Itanium ABI) they can be emitted in several variants (complete vs base
ctor/dtor) and I don't know how to force emitting all variants. Ctors/dtors are
a significant portion of PCH template instantiations, so without this the
building time improvement is not as large as it could be.
- The patch currently does not handle function inlining well (=it works only
with -O0 builds). This optimization cannot be done with template functions that
should be inlined by the compilation, so the code needs to check whether a
function would possibly be inlined. The problem is that this depends on
settings in CodeGenOptions and Sema (understandably) doesn't have an access to
that. How can I access that in Sema? Does it make sense to try to handle this,
or is the deciding whether a function will get actually inlined too complex and
this should rather be skipped for CodeGen::NormalInlining?
- In rate circumstances it is possible to get undefined references because of
instantiations in the PCH's object file. Specifically this happens if the PCH
includes a header providing non-exported code from another module (=shared
library), in that case the PCH may refer to it. I think this is conceptually
not possible to handle in Clang, either the code should be cleaned up to not
provide non-exported code to other modules, or it's possible to use linker's
--gc-sections to drop such instantiations.
- In Sema::InstantiateFunctionDefinition() the code for extern templates still
instantiates a function if it has getContainedAutoType(), so my code should
probably also check that. But I'm not sure what that is (is that 'auto foo() {
return 1; }' ?) or why that would need an instance in every TU.
- Is there a guideline on when to use data members in class Decl vs functions?
The patch uses DeclIsPendingInstantiationFromPCHWithObjectFile() to check
whether a FunctionDecl comes from the PCH, which may get called quite often.
This could be optimized away by storing a flag in class Decl.
Repository:
rC Clang
https://reviews.llvm.org/D64284
Files:
clang/include/clang/AST/ExternalASTSource.h
clang/include/clang/Sema/MultiplexExternalSemaSource.h
clang/include/clang/Serialization/ASTReader.h
clang/lib/AST/ASTContext.cpp
clang/lib/Sema/MultiplexExternalSemaSource.cpp
clang/lib/Sema/Sema.cpp
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
clang/lib/Serialization/ASTReader.cpp
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -8293,14 +8293,14 @@
void ASTReader::ReadPendingInstantiations(
SmallVectorImpl> &Pending) {
- for (unsigned Idx = 0, N = PendingInstantiations.size(); Idx < N;) {
+ for (unsigned Idx = PendingInstantiationsReadCount, N = PendingInstantiations.size(); Idx < N;) {
ValueDecl *D = cast(GetDecl(PendingInstantiations[Idx++]));
SourceLocation Loc
= SourceLocation::getFromRawEncoding(PendingInstantiations[Idx++]);
Pending.push_back(std::make_pair(D, Loc));
}
- PendingInstantiations.clear();
+ PendingInstantiationsReadCount = PendingInstantiations.size();
}
void ASTReader::ReadLateParsedTemplates(
@@ -8522,6 +8522,17 @@
return MF && MF->PCHHasObjectFile;
}
+bool ASTReader::DeclIsPendingInstantiationFromPCHWithObjectFile(const Decl *D) {
+ if( !DeclIsFromPCHWithObjectFile(D))
+return false;
+ for (unsigned Idx = 0, N = PendingInstantiations.size(); Idx < N;) {
+if( D == cast(GetDecl(PendingInstantiations[Idx++])))
+return true;
+Idx++;
+ }
+ return false;
+}
+
ModuleFile *ASTReader::getLocalModuleFile(ModuleFile &F, unsigned ID) {
if (ID & 1) {
// It's a module, look it up by submodule ID.
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
