(WIP) share template instantiations from PCH in one .o if -building-pch-with-obj

[llunak]

Summary

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.

[llunak]

Due to some technical problems with this approach and lack of feedback, I'm scratching this one. A new approach is at https://reviews.llvm.org/D69585 .

clang/include/clang/AST/ExternalASTSource.h

   /// 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.

clang/include/clang/Sema/MultiplexExternalSemaSource.h

 
   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

clang/include/clang/Serialization/ASTReader.h

   /// 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

 
   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);

clang/lib/AST/ASTContext.cpp

     // 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))

clang/lib/Sema/MultiplexExternalSemaSource.cpp

   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,

clang/lib/Sema/Sema.cpp

       (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?");

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

+#include <iostream>
 //===--- SemaTemplateInstantiateDecl.cpp - C++ Template Decl Instantiation ===/
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.

       !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.

clang/lib/Serialization/ASTReader.cpp

 
 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(

   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.