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<std::pair<ValueDecl *, SourceLocation>> &Pending) {
- for (unsigned Idx = 0, N = PendingInstantiations.size(); Idx < N;) {
+ for (unsigned Idx = PendingInstantiationsReadCount, N = PendingInstantiations.size(); Idx < N;) {
ValueDecl *D = cast<ValueDecl>(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<ValueDecl>(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
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -1,3 +1,4 @@
+#include <iostream>
//===--- SemaTemplateInstantiateDecl.cpp - C++ Template Decl Instantiation ===/
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
@@ -4213,6 +4214,31 @@
!PatternDecl->getReturnType()->getContainedAutoType())
return;
+ if (Function->isFromASTFile() && getExternalSource()->DeclIsPendingInstantiationFromPCHWithObjectFile(Function)) {
+ assert(getSourceManager().isLoadedSourceLocation(PointOfInstantiation));
+
+ // Instantiation of this function may be possibly skipped, other translation units
+ // using the same PCH will instantiate it as well. Do not skip if the function
+ // may get inlined or evaluated.
+
+ // TODO is restricting to TSK_ImplicitInstantiation needed?
+ if(TSK == TSK_ImplicitInstantiation) {
+ // TODO !PatternDecl->getReturnType()->getContainedAutoType())
+
+ // TODO ctors/dtors are currently broken
+ bool Ignore = false;
+ if (dyn_cast<CXXConstructorDecl>(Function) || dyn_cast<CXXDestructorDecl>(Function))
+ Ignore = true;
+
+ // TODO check will-never-get-inlined
+ if(!Function->isConstexpr() && !Function->hasAttr<AlwaysInlineAttr>() && !Ignore) {
+ if(!getLangOpts().BuildingPCHWithObjectFile) {
+ return;
+ }
+ }
+ }
+ }
+
if (PatternDecl->isInlined()) {
// Function, and all later redeclarations of it (from imported modules,
// for instance), are now implicitly inline.
Index: clang/lib/Sema/Sema.cpp
===================================================================
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -724,8 +724,12 @@
(void)FD;
assert(FD->getMostRecentDecl()->isInlined() &&
"used object requires definition but isn't inline or internal?");
- // FIXME: This is ill-formed; we should reject.
- S.Diag(VD->getLocation(), diag::warn_undefined_inline) << VD;
+ // TODO check will-never-get-inlined
+ bool ignoreSkippedPCHTemplate = FD->isFromASTFile() && S.getExternalSource()->DeclIsPendingInstantiationFromPCHWithObjectFile(FD);
+ if( !ignoreSkippedPCHTemplate ) {
+ // FIXME: This is ill-formed; we should reject.
+ S.Diag(VD->getLocation(), diag::warn_undefined_inline) << VD;
+ }
} else {
assert(cast<VarDecl>(VD)->getMostRecentDecl()->isInline() &&
"used var requires definition but isn't inline or internal?");
Index: clang/lib/Sema/MultiplexExternalSemaSource.cpp
===================================================================
--- clang/lib/Sema/MultiplexExternalSemaSource.cpp
+++ clang/lib/Sema/MultiplexExternalSemaSource.cpp
@@ -177,6 +177,13 @@
return false;
}
+bool MultiplexExternalSemaSource::DeclIsPendingInstantiationFromPCHWithObjectFile(const Decl *D) {
+ for (auto *S : Sources)
+ if (S->DeclIsPendingInstantiationFromPCHWithObjectFile(D))
+ return true;
+ return false;
+}
+
bool MultiplexExternalSemaSource::layoutRecordType(const RecordDecl *Record,
uint64_t &Size,
uint64_t &Alignment,
Index: clang/lib/AST/ASTContext.cpp
===================================================================
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -9862,6 +9862,11 @@
// We never need to emit an uninstantiated function template.
if (FD->getTemplatedKind() == FunctionDecl::TK_FunctionTemplate)
return false;
+ // Force emitting shared template instantiations in the PCH's object file.
+ if (LangOpts.BuildingPCHWithObjectFile && FD->isFromASTFile()
+ && getExternalSource()->DeclIsPendingInstantiationFromPCHWithObjectFile(FD)) {
+ return true;
+ }
} else if (isa<PragmaCommentDecl>(D))
return true;
else if (isa<PragmaDetectMismatchDecl>(D))
Index: clang/include/clang/Serialization/ASTReader.h
===================================================================
--- clang/include/clang/Serialization/ASTReader.h
+++ clang/include/clang/Serialization/ASTReader.h
@@ -792,6 +792,11 @@
/// is the instantiation location.
SmallVector<uint64_t, 64> PendingInstantiations;
+ /// The position in PendingInstantiations for ReadPendingInstantiations().
+ ///
+ /// To make sure ReadPendingInstantiations() does not return the same item twice.
+ unsigned PendingInstantiationsReadCount = 0;
+
//@}
/// \name DiagnosticsEngine-relevant special data
@@ -2089,6 +2094,8 @@
bool DeclIsFromPCHWithObjectFile(const Decl *D) override;
+ bool DeclIsPendingInstantiationFromPCHWithObjectFile(const Decl *D) override;
+
/// Retrieve the module file with a given local ID within the specified
/// ModuleFile.
ModuleFile *getLocalModuleFile(ModuleFile &M, unsigned ID);
Index: clang/include/clang/Sema/MultiplexExternalSemaSource.h
===================================================================
--- clang/include/clang/Sema/MultiplexExternalSemaSource.h
+++ clang/include/clang/Sema/MultiplexExternalSemaSource.h
@@ -153,6 +153,8 @@
bool DeclIsFromPCHWithObjectFile(const Decl *D) override;
+ bool DeclIsPendingInstantiationFromPCHWithObjectFile(const Decl *D) override;
+
/// Perform layout on the given record.
///
/// This routine allows the external AST source to provide an specific
Index: clang/include/clang/AST/ExternalASTSource.h
===================================================================
--- clang/include/clang/AST/ExternalASTSource.h
+++ clang/include/clang/AST/ExternalASTSource.h
@@ -166,6 +166,10 @@
/// object file.
virtual bool DeclIsFromPCHWithObjectFile(const Decl *D) { return false; }
+ /// Determine whether D is a pending instantiation that comes from a PCH
+ /// which was built with a corresponding object file.
+ virtual bool DeclIsPendingInstantiationFromPCHWithObjectFile(const Decl *D) { return false; }
+
/// Abstracts clang modules and precompiled header files and holds
/// everything needed to generate debug info for an imported module
/// or PCH.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
