hans created this revision.
hans added reviewers: thakis, rnk, rsmith.

With MSVC, PCH files are created along with an object file that needs to be 
linked into the final library or executable. That object file contains the code 
generated when building the headers. In particular, it will include definitions 
of inline dllexport functions, and because they are emitted in this object 
file, other files using the PCH do not need to emit them. See the bug for an 
example.

This patch makes clang-cl match MSVC's behaviour in this regard, causing 
significant compile-time savings when building dlls using precompiled headers.

For example, in a 64-bit optimized shared library build of Chromium with PCH, 
it reduces the binary size and compile time of stroke_opacity_custom.obj from 
9315564 bytes to 3659629 bytes and 14.6 to 6.63 s. The wall-clock time of 
building blink_core.dll goes from 38m41s to 22m33s. ("user" time goes from 
1979m to 1142m).


https://reviews.llvm.org/D48426

Files:
  include/clang/AST/ASTContext.h
  include/clang/AST/ExternalASTSource.h
  include/clang/Driver/CC1Options.td
  include/clang/Frontend/FrontendOptions.h
  include/clang/Sema/MultiplexExternalSemaSource.h
  include/clang/Serialization/ASTBitCodes.h
  include/clang/Serialization/ASTReader.h
  include/clang/Serialization/ASTWriter.h
  include/clang/Serialization/Module.h
  lib/AST/ASTContext.cpp
  lib/Driver/Driver.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInstance.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Frontend/FrontendActions.cpp
  lib/Sema/MultiplexExternalSemaSource.cpp
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTWriter.cpp
  lib/Serialization/GeneratePCH.cpp
  test/CodeGen/pch-dllexport.cpp
  test/Driver/cl-pch.cpp

Index: test/Driver/cl-pch.cpp
===================================================================
--- test/Driver/cl-pch.cpp
+++ test/Driver/cl-pch.cpp
@@ -7,13 +7,15 @@
 // 1. Build .pch file.
 // CHECK-YC: cc1
 // CHECK-YC: -emit-pch
+// CHECK-YC: -building-pch-with-obj
 // CHECK-YC: -o
 // CHECK-YC: pchfile.pch
 // CHECK-YC: -x
 // CHECK-YC: "c++-header"
 // 2. Use .pch file.
 // CHECK-YC: cc1
 // CHECK-YC: -emit-obj
+// CHECK-YC: -building-pch-with-obj
 // CHECK-YC: -include-pch
 // CHECK-YC: pchfile.pch
 
@@ -24,11 +26,13 @@
 // 1. Build .pch file.
 // CHECK-YCO: cc1
 // CHECK-YCO: -emit-pch
+// CHECK-YCO: -building-pch-with-obj
 // CHECK-YCO: -o
 // CHECK-YCO: pchfile.pch
 // 2. Use .pch file.
 // CHECK-YCO: cc1
 // CHECK-YCO: -emit-obj
+// CHECK-YCO: -building-pch-with-obj
 // CHECK-YCO: -include-pch
 // CHECK-YCO: pchfile.pch
 // CHECK-YCO: -o
@@ -46,6 +50,7 @@
 // RUN:   | FileCheck -check-prefix=CHECK-YU %s
 // Use .pch file, but don't build it.
 // CHECK-YU-NOT: -emit-pch
+// CHECK-YU-NOT: -building-pch-with-obj
 // CHECK-YU: cc1
 // CHECK-YU: -emit-obj
 // CHECK-YU: -include-pch
@@ -63,6 +68,7 @@
 // 1. Build .pch file.
 // CHECK-YC-YU: cc1
 // CHECK-YC-YU: -emit-pch
+// CHECK-YC-YU: -building-pch-with-obj
 // CHECK-YC-YU: -o
 // CHECK-YC-YU: pchfile.pch
 // 2. Use .pch file.
Index: test/CodeGen/pch-dllexport.cpp
===================================================================
--- /dev/null
+++ test/CodeGen/pch-dllexport.cpp
@@ -0,0 +1,25 @@
+// Build PCH without object file, then use it.
+// RUN: %clang_cc1 -triple i686-pc-win32 -fms-extensions -emit-pch -o %t %s
+// RUN: %clang_cc1 -triple i686-pc-win32 -fms-extensions -emit-obj -emit-llvm -include-pch %t -o - %s | FileCheck -check-prefix=PCH %s
+
+// Build PCH with object file, then use it.
+// RUN: %clang_cc1 -triple i686-pc-win32 -fms-extensions -emit-pch -building-pch-with-obj -o %t %s
+// RUN: %clang_cc1 -triple i686-pc-win32 -fms-extensions -emit-obj -emit-llvm -include-pch %t -building-pch-with-obj -o - %s | FileCheck -check-prefix=OBJ %s
+// RUN: %clang_cc1 -triple i686-pc-win32 -fms-extensions -emit-obj -emit-llvm -include-pch %t -o - %s | FileCheck -check-prefix=PCHWITHOBJ %s
+
+#ifndef IN_HEADER
+#define IN_HEADER
+
+inline void __declspec(dllexport) foo() {}
+// OBJ: define weak_odr dso_local dllexport void @"?foo@@YAXXZ"
+// PCH: define weak_odr dso_local dllexport void @"?foo@@YAXXZ"
+// PCHWITHOBJ-NOT: define weak_odr dso_local dllexport void @"?foo@@YAXXZ"
+
+struct __declspec(dllexport) S {
+  void bar() {}
+// OBJ: define weak_odr dso_local dllexport x86_thiscallcc void @"?bar@S@@QAEXXZ"
+// PCH: define weak_odr dso_local dllexport x86_thiscallcc void @"?bar@S@@QAEXXZ"
+// PCHWITHOBJ-NOT: define weak_odr dso_local dllexport x86_thiscallcc void @"?bar@S@@QAEXXZ"
+};
+
+#endif
Index: lib/Serialization/GeneratePCH.cpp
===================================================================
--- lib/Serialization/GeneratePCH.cpp
+++ lib/Serialization/GeneratePCH.cpp
@@ -25,11 +25,11 @@
     const Preprocessor &PP, StringRef OutputFile, StringRef isysroot,
     std::shared_ptr<PCHBuffer> Buffer,
     ArrayRef<std::shared_ptr<ModuleFileExtension>> Extensions,
-    bool AllowASTWithErrors, bool IncludeTimestamps)
+    bool AllowASTWithErrors, bool IncludeTimestamps, bool PCHHasObjectFile)
     : PP(PP), OutputFile(OutputFile), isysroot(isysroot.str()),
       SemaPtr(nullptr), Buffer(std::move(Buffer)), Stream(this->Buffer->Data),
       Writer(Stream, this->Buffer->Data, PP.getPCMCache(), Extensions,
-             IncludeTimestamps),
+             IncludeTimestamps, PCHHasObjectFile),
       AllowASTWithErrors(AllowASTWithErrors) {
   this->Buffer->IsComplete = false;
 }
Index: lib/Serialization/ASTWriter.cpp
===================================================================
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -1458,6 +1458,7 @@
   MetadataAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 16)); // Clang min.
   MetadataAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // Relocatable
   MetadataAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // Timestamps
+  MetadataAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // PCHHasObjectFile
   MetadataAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // Errors
   MetadataAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); // SVN branch/tag
   unsigned MetadataAbbrevCode = Stream.EmitAbbrev(std::move(MetadataAbbrev));
@@ -1467,7 +1468,7 @@
     RecordData::value_type Record[] = {METADATA, VERSION_MAJOR, VERSION_MINOR,
                                        CLANG_VERSION_MAJOR, CLANG_VERSION_MINOR,
                                        !isysroot.empty(), IncludeTimestamps,
-                                       ASTHasCompilerErrors};
+                                       PCHHasObjectFile, ASTHasCompilerErrors};
     Stream.EmitRecordWithBlob(MetadataAbbrevCode, Record,
                               getClangFullRepositoryVersion());
   }
@@ -4556,9 +4557,9 @@
 ASTWriter::ASTWriter(llvm::BitstreamWriter &Stream,
                      SmallVectorImpl<char> &Buffer, MemoryBufferCache &PCMCache,
                      ArrayRef<std::shared_ptr<ModuleFileExtension>> Extensions,
-                     bool IncludeTimestamps)
+                     bool IncludeTimestamps, bool PCHHasObjectFile)
     : Stream(Stream), Buffer(Buffer), PCMCache(PCMCache),
-      IncludeTimestamps(IncludeTimestamps) {
+      IncludeTimestamps(IncludeTimestamps), PCHHasObjectFile(PCHHasObjectFile) {
   for (const auto &Ext : Extensions) {
     if (auto Writer = Ext->createExtensionWriter(*this))
       ModuleFileExtensionWriters.push_back(std::move(Writer));
Index: lib/Serialization/ASTReader.cpp
===================================================================
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -2482,7 +2482,7 @@
         return VersionMismatch;
       }
 
-      bool hasErrors = Record[6];
+      bool hasErrors = Record[7];
       if (hasErrors && !DisableValidation && !AllowASTWithCompilerErrors) {
         Diag(diag::err_pch_with_compiler_errors);
         return HadErrors;
@@ -2500,6 +2500,8 @@
 
       F.HasTimestamps = Record[5];
 
+      F.PCHHasObjectFile = Record[6];
+
       const std::string &CurBranch = getClangFullRepositoryVersion();
       StringRef ASTBranch = Blob;
       if (StringRef(CurBranch) != ASTBranch && !DisableValidation) {
@@ -8448,6 +8450,11 @@
   return getSubmodule(ID);
 }
 
+bool ASTReader::DeclIsFromPCHWithObjectFile(const Decl *D) {
+  ModuleFile *MF = getOwningModuleFile(D);
+  return MF && MF->PCHHasObjectFile;
+}
+
 ModuleFile *ASTReader::getLocalModuleFile(ModuleFile &F, unsigned ID) {
   if (ID & 1) {
     // It's a module, look it up by submodule ID.
Index: lib/Sema/MultiplexExternalSemaSource.cpp
===================================================================
--- lib/Sema/MultiplexExternalSemaSource.cpp
+++ lib/Sema/MultiplexExternalSemaSource.cpp
@@ -171,6 +171,13 @@
   return nullptr;
 }
 
+bool MultiplexExternalSemaSource::DeclIsFromPCHWithObjectFile(const Decl *D) {
+  for (auto *S : Sources)
+    if (S->DeclIsFromPCHWithObjectFile(D))
+      return true;
+  return false;
+}
+
 bool MultiplexExternalSemaSource::layoutRecordType(const RecordDecl *Record,
                                                    uint64_t &Size, 
                                                    uint64_t &Alignment,
Index: lib/Frontend/FrontendActions.cpp
===================================================================
--- lib/Frontend/FrontendActions.cpp
+++ lib/Frontend/FrontendActions.cpp
@@ -112,14 +112,15 @@
   if (!CI.getFrontendOpts().RelocatablePCH)
     Sysroot.clear();
 
+  const auto &FrontendOpts = CI.getFrontendOpts();
   auto Buffer = std::make_shared<PCHBuffer>();
   std::vector<std::unique_ptr<ASTConsumer>> Consumers;
   Consumers.push_back(llvm::make_unique<PCHGenerator>(
                         CI.getPreprocessor(), OutputFile, Sysroot,
-                        Buffer, CI.getFrontendOpts().ModuleFileExtensions,
-      /*AllowASTWithErrors*/CI.getPreprocessorOpts().AllowPCHWithCompilerErrors,
-                        /*IncludeTimestamps*/
-                          +CI.getFrontendOpts().IncludeTimestamps));
+                        Buffer, FrontendOpts.ModuleFileExtensions,
+                        CI.getPreprocessorOpts().AllowPCHWithCompilerErrors,
+                        FrontendOpts.IncludeTimestamps,
+                        FrontendOpts.BuildingPCHWithObjectFile));
   Consumers.push_back(CI.getPCHContainerWriter().CreatePCHContainerGenerator(
       CI, InFile, OutputFile, std::move(OS), Buffer));
 
Index: lib/Frontend/CompilerInvocation.cpp
===================================================================
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -1528,6 +1528,7 @@
   Opts.ModulesEmbedFiles = Args.getAllArgValues(OPT_fmodules_embed_file_EQ);
   Opts.ModulesEmbedAllFiles = Args.hasArg(OPT_fmodules_embed_all_files);
   Opts.IncludeTimestamps = !Args.hasArg(OPT_fno_pch_timestamp);
+  Opts.BuildingPCHWithObjectFile = Args.hasArg(OPT_building_pch_with_obj);
 
   Opts.CodeCompleteOpts.IncludeMacros
     = Args.hasArg(OPT_code_completion_macros);
Index: lib/Frontend/CompilerInstance.cpp
===================================================================
--- lib/Frontend/CompilerInstance.cpp
+++ lib/Frontend/CompilerInstance.cpp
@@ -487,6 +487,7 @@
                                  PP.getIdentifierTable(), PP.getSelectorTable(),
                                  PP.getBuiltinInfo());
   Context->InitBuiltinTypes(getTarget(), getAuxTarget());
+  Context->BuildingPCHWithObjectFile = getFrontendOpts().BuildingPCHWithObjectFile;
   setASTContext(Context);
 }
 
Index: lib/Driver/ToolChains/Clang.cpp
===================================================================
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -1084,6 +1084,10 @@
     CmdArgs.push_back(Args.MakeArgString(Twine("-find-pch-source=") +
                                          Inputs[0].second->getValue()));
   }
+  if (YcIndex != -1 && JA.getKind() >= Action::PrecompileJobClass &&
+      JA.getKind() <= Action::AssembleJobClass) {
+    CmdArgs.push_back(Args.MakeArgString("-building-pch-with-obj"));
+  }
 
   bool RenderedImplicitInclude = false;
   int AI = -1;
Index: lib/Driver/Driver.cpp
===================================================================
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -3088,6 +3088,8 @@
         // The driver currently exits after the first failed command.  This
         // relies on that behavior, to make sure if the pch generation fails,
         // the main compilation won't run.
+        // FIXME: If the main compilation fails, the PCH generation should
+        // probably not be considered successful either.
       }
     }
 
Index: lib/AST/ASTContext.cpp
===================================================================
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -9553,6 +9553,16 @@
   else
     return false;
 
+  if (D->isFromASTFile() && !BuildingPCHWithObjectFile) {
+    assert(getExternalSource() && "It's from an AST file; must be a source.");
+    // On Windows, PCH files are built together with an object file. If this
+    // declaration comes from such a PCH and the decl needs to be emitted, it
+    // would already have been emitted into that object file, so it doesn't need
+    // to be emitted here.
+    if (getExternalSource()->DeclIsFromPCHWithObjectFile(D))
+      return false;
+  }
+
   // If this is a member of a class template, we do not need to emit it.
   if (D->getDeclContext()->isDependentContext())
     return false;
@@ -9573,7 +9583,7 @@
     // Constructors and destructors are required.
     if (FD->hasAttr<ConstructorAttr>() || FD->hasAttr<DestructorAttr>())
       return true;
-    
+
     // The key function for a class is required.  This rule only comes
     // into play when inline functions can be key functions, though.
     if (getTargetInfo().getCXXABI().canKeyFunctionBeInline()) {
@@ -9594,7 +9604,7 @@
     // Implicit template instantiations can also be deferred in C++.
     return !isDiscardableGVALinkage(Linkage);
   }
-  
+
   const auto *VD = cast<VarDecl>(D);
   assert(VD->isFileVarDecl() && "Expected file scoped var");
 
Index: include/clang/Serialization/Module.h
===================================================================
--- include/clang/Serialization/Module.h
+++ include/clang/Serialization/Module.h
@@ -157,6 +157,9 @@
   /// Whether timestamps are included in this module file.
   bool HasTimestamps = false;
 
+  /// Whether the PCH has a corresponding object file.
+  bool PCHHasObjectFile = false;
+
   /// The file entry for the module file.
   const FileEntry *File = nullptr;
 
Index: include/clang/Serialization/ASTWriter.h
===================================================================
--- include/clang/Serialization/ASTWriter.h
+++ include/clang/Serialization/ASTWriter.h
@@ -155,6 +155,9 @@
   /// file is up to date, but not otherwise.
   bool IncludeTimestamps;
 
+  /// Indicates whether the PCH has a corresponding object file.
+  bool PCHHasObjectFile;
+
   /// Indicates when the AST writing is actively performing
   /// serialization, rather than just queueing updates.
   bool WritingAST = false;
@@ -544,7 +547,7 @@
   ASTWriter(llvm::BitstreamWriter &Stream, SmallVectorImpl<char> &Buffer,
             MemoryBufferCache &PCMCache,
             ArrayRef<std::shared_ptr<ModuleFileExtension>> Extensions,
-            bool IncludeTimestamps = true);
+            bool IncludeTimestamps = true, bool PCHHasObjectFile = false);
   ~ASTWriter() override;
 
   const LangOptions &getLangOpts() const;
@@ -980,7 +983,8 @@
   PCHGenerator(const Preprocessor &PP, StringRef OutputFile, StringRef isysroot,
                std::shared_ptr<PCHBuffer> Buffer,
                ArrayRef<std::shared_ptr<ModuleFileExtension>> Extensions,
-               bool AllowASTWithErrors = false, bool IncludeTimestamps = true);
+               bool AllowASTWithErrors = false, bool IncludeTimestamps = true,
+               bool PCHHasObjectFile = false);
   ~PCHGenerator() override;
 
   void InitializeSema(Sema &S) override { SemaPtr = &S; }
Index: include/clang/Serialization/ASTReader.h
===================================================================
--- include/clang/Serialization/ASTReader.h
+++ include/clang/Serialization/ASTReader.h
@@ -2071,6 +2071,8 @@
   /// Note: overrides method in ExternalASTSource
   Module *getModule(unsigned ID) override;
 
+  bool DeclIsFromPCHWithObjectFile(const Decl *D) override;
+
   /// Retrieve the module file with a given local ID within the specified
   /// ModuleFile.
   ModuleFile *getLocalModuleFile(ModuleFile &M, unsigned ID);
Index: include/clang/Serialization/ASTBitCodes.h
===================================================================
--- include/clang/Serialization/ASTBitCodes.h
+++ include/clang/Serialization/ASTBitCodes.h
@@ -42,7 +42,7 @@
     /// Version 4 of AST files also requires that the version control branch and
     /// revision match exactly, since there is no backward compatibility of
     /// AST files at this time.
-    const unsigned VERSION_MAJOR = 6;
+    const unsigned VERSION_MAJOR = 7; // XXX: Could we avoid this somehow?
 
     /// AST file minor version number supported by this version of
     /// Clang.
Index: include/clang/Sema/MultiplexExternalSemaSource.h
===================================================================
--- include/clang/Sema/MultiplexExternalSemaSource.h
+++ include/clang/Sema/MultiplexExternalSemaSource.h
@@ -152,6 +152,8 @@
   /// Retrieve the module that corresponds to the given module ID.
   Module *getModule(unsigned ID) override;
 
+  bool DeclIsFromPCHWithObjectFile(const Decl *D) override;
+
   /// Perform layout on the given record.
   ///
   /// This routine allows the external AST source to provide an specific 
Index: include/clang/Frontend/FrontendOptions.h
===================================================================
--- include/clang/Frontend/FrontendOptions.h
+++ include/clang/Frontend/FrontendOptions.h
@@ -306,6 +306,10 @@
   /// Whether timestamps should be written to the produced PCH file.
   unsigned IncludeTimestamps : 1;
 
+  /// Whether this compilation is part of building a PCH with corresponding
+  /// object file. (Either we're building the PCH itself, or the obj file.)
+  unsigned BuildingPCHWithObjectFile : 1;
+
   CodeCompleteOptions CodeCompleteOpts;
 
   enum {
@@ -451,7 +455,8 @@
         SkipFunctionBodies(false), UseGlobalModuleIndex(true),
         GenerateGlobalModuleIndex(true), ASTDumpDecls(false),
         ASTDumpLookups(false), BuildingImplicitModule(false),
-        ModulesEmbedAllFiles(false), IncludeTimestamps(true) {}
+        ModulesEmbedAllFiles(false), IncludeTimestamps(true),
+        BuildingPCHWithObjectFile(false) {}
 
   /// getInputKindForExtension - Return the appropriate input kind for a file
   /// extension. For example, "c" would return InputKind::C.
Index: include/clang/Driver/CC1Options.td
===================================================================
--- include/clang/Driver/CC1Options.td
+++ include/clang/Driver/CC1Options.td
@@ -609,7 +609,9 @@
            "to this flag.">;
 def fno_pch_timestamp : Flag<["-"], "fno-pch-timestamp">,
   HelpText<"Disable inclusion of timestamp in precompiled headers">;
-  
+def building_pch_with_obj : Flag<["-"], "building-pch-with-obj">,
+  HelpText<"This compilation is part of building a PCH with corresponding object file.">;
+
 def aligned_alloc_unavailable : Flag<["-"], "faligned-alloc-unavailable">,
   HelpText<"Aligned allocation/deallocation functions are unavailable">;
 
Index: include/clang/AST/ExternalASTSource.h
===================================================================
--- include/clang/AST/ExternalASTSource.h
+++ include/clang/AST/ExternalASTSource.h
@@ -163,6 +163,10 @@
   /// Retrieve the module that corresponds to the given module ID.
   virtual Module *getModule(unsigned ID) { return nullptr; }
 
+  /// Determine whether D comes from a PCH which was built with a corresponding
+  /// object file.
+  virtual bool DeclIsFromPCHWithObjectFile(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.
Index: include/clang/AST/ASTContext.h
===================================================================
--- include/clang/AST/ASTContext.h
+++ include/clang/AST/ASTContext.h
@@ -2882,6 +2882,9 @@
   };
 
   llvm::StringMap<SectionInfo> SectionInfos;
+
+  // XXX: I don't like adding this to ASTContext, but I ran out of ideas for how ASTContext::DeclMustBeEmitted() would know about it otherwise.
+  bool BuildingPCHWithObjectFile = false;
 };
 
 /// Utility function for constructing a nullary selector.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to