[clang-cl] Don't emit dllexport inline functions etc. from pch files (PR37801)

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

Differential Revision: https://reviews.llvm.org/D48426

llvm-svn: 335466

17 files changed, 168 insertions, 13 deletions

diff --git a/clang/include/clang/AST/ExternalASTSource.h b/clang/include/clang/AST/ExternalASTSource.h
index 52de0fc..1e05a30 100644
--- a/clang/include/clang/AST/ExternalASTSource.h
+++ b/clang/include/clang/AST/ExternalASTSource.h
@@ -163,6 +163,10 @@ public:
   /// 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.
diff --git a/clang/include/clang/Basic/LangOptions.def b/clang/include/clang/Basic/LangOptions.def
index d7d66be..55f057a 100644
--- a/clang/include/clang/Basic/LangOptions.def
+++ b/clang/include/clang/Basic/LangOptions.def
@@ -154,6 +154,7 @@ COMPATIBLE_LANGOPT(ModulesTS  , 1, 0, "C++ Modules TS")
 BENIGN_ENUM_LANGOPT(CompilingModule, CompilingModuleKind, 2, CMK_None,
                     "compiling a module interface")
 BENIGN_LANGOPT(CompilingPCH, 1, 0, "building a pch")
+BENIGN_LANGOPT(BuildingPCHWithObjectFile, 1, 0, "building a pch which has a corresponding object file")
 COMPATIBLE_LANGOPT(ModulesDeclUse    , 1, 0, "require declaration of module uses")
 BENIGN_LANGOPT(ModulesSearchAll  , 1, 1, "searching even non-imported modules to find unresolved references")
 COMPATIBLE_LANGOPT(ModulesStrictDeclUse, 1, 0, "requiring declaration of module uses and all headers to be in modules")
diff --git a/clang/include/clang/Driver/CC1Options.td b/clang/include/clang/Driver/CC1Options.td
index 18d0483..d500cab 100644
--- a/clang/include/clang/Driver/CC1Options.td
+++ b/clang/include/clang/Driver/CC1Options.td
@@ -609,7 +609,9 @@ def find_pch_source_EQ : Joined<["-"], "find-pch-source=">,
            "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">;
 
diff --git a/clang/include/clang/Sema/MultiplexExternalSemaSource.h b/clang/include/clang/Sema/MultiplexExternalSemaSource.h
index 8514fff..4c242c8 100644
--- a/clang/include/clang/Sema/MultiplexExternalSemaSource.h
+++ b/clang/include/clang/Sema/MultiplexExternalSemaSource.h
@@ -152,6 +152,8 @@ public:
   /// 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 
diff --git a/clang/include/clang/Serialization/ASTBitCodes.h b/clang/include/clang/Serialization/ASTBitCodes.h
index e98a623..82bb1f5 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -42,7 +42,7 @@ namespace serialization {
     /// 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;
 
     /// AST file minor version number supported by this version of
     /// Clang.
diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h
index 7de511a..d09a9f4 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -2071,6 +2071,8 @@ public:
   /// 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);
diff --git a/clang/include/clang/Serialization/Module.h b/clang/include/clang/Serialization/Module.h
index 5f41f39..653981b 100644
--- a/clang/include/clang/Serialization/Module.h
+++ b/clang/include/clang/Serialization/Module.h
@@ -157,6 +157,9 @@ public:
   /// 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;
 
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 96b7b56..43577e8 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -9553,6 +9553,29 @@ bool ASTContext::DeclMustBeEmitted(const Decl *D) {
   else
     return false;
 
+  if (D->isFromASTFile() && !LangOpts.BuildingPCHWithObjectFile) {
+    assert(getExternalSource() && "It's from an AST file; must have a source.");
+    // On Windows, PCH files are built together with an object file. If this
+    // declaration comes from such a PCH and DeclMustBeEmitted would return
+    // true, it would have returned true and the decl would have been emitted
+    // into that object file, so it doesn't need to be emitted here.
+    // Note that decls are still emitted if they're referenced, as usual;
+    // DeclMustBeEmitted is used to decide whether a decl must be emitted even
+    // if it's not referenced.
+    //
+    // Explicit template instantiation definitions are tricky. If there was an
+    // explicit template instantiation decl in the PCH before, it will look like
+    // the definition comes from there, even if that was just the declaration.
+    // (Explicit instantiation defs of variable templates always get emitted.)
+    bool IsExpInstDef =
+        isa<FunctionDecl>(D) &&
+        cast<FunctionDecl>(D)->getTemplateSpecializationKind() ==
+            TSK_ExplicitInstantiationDefinition;
+
+    if (getExternalSource()->DeclIsFromPCHWithObjectFile(D) && !IsExpInstDef)
+      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 +9596,7 @@ bool ASTContext::DeclMustBeEmitted(const Decl *D) {
     // 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 +9617,7 @@ bool ASTContext::DeclMustBeEmitted(const Decl *D) {
     // 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");
 
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 14d94c0..7b890d80 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -3088,6 +3088,8 @@ void Driver::BuildActions(Compilation &C, DerivedArgList &Args,
         // 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.
       }
     }
 
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index bce2083..88e2f83 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -1084,6 +1084,10 @@ void Clang::AddPreprocessingOptions(Compilation &C, const JobAction &JA,
     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;
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index 60ed5fd..26414f1 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -2776,6 +2776,7 @@ static void ParseLangArgs(LangOptions &Opts, ArgList &Args, InputKind IK,
   }
 
   Opts.CompleteMemberPointers = Args.hasArg(OPT_fcomplete_member_pointers);
+  Opts.BuildingPCHWithObjectFile = Args.hasArg(OPT_building_pch_with_obj);
 }
 
 static bool isStrictlyPreprocessorAction(frontend::ActionKind Action) {
diff --git a/clang/lib/Frontend/FrontendActions.cpp b/clang/lib/Frontend/FrontendActions.cpp
index 602fee1..06db814 100644
--- a/clang/lib/Frontend/FrontendActions.cpp
+++ b/clang/lib/Frontend/FrontendActions.cpp
@@ -112,14 +112,14 @@ GeneratePCHAction::CreateASTConsumer(CompilerInstance &CI, StringRef InFile) {
   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));
   Consumers.push_back(CI.getPCHContainerWriter().CreatePCHContainerGenerator(
       CI, InFile, OutputFile, std::move(OS), Buffer));
 
diff --git a/clang/lib/Sema/MultiplexExternalSemaSource.cpp b/clang/lib/Sema/MultiplexExternalSemaSource.cpp
index 9370cf2..7e61ccb 100644
--- a/clang/lib/Sema/MultiplexExternalSemaSource.cpp
+++ b/clang/lib/Sema/MultiplexExternalSemaSource.cpp
@@ -171,6 +171,13 @@ Module *MultiplexExternalSemaSource::getModule(unsigned ID) {
   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,
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index 3dd233e..982c4cb 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -2482,7 +2482,7 @@ ASTReader::ReadControlBlock(ModuleFile &F,
         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 @@ ASTReader::ReadControlBlock(ModuleFile &F,
 
       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 @@ Module *ASTReader::getModule(unsigned ID) {
   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.
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 1e3a098..7d5b521 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -1458,16 +1458,23 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, ASTContext &Context,
   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));
   assert((!WritingModule || isysroot.empty()) &&
          "writing module as a relocatable PCH?");
   {
-    RecordData::value_type Record[] = {METADATA, VERSION_MAJOR, VERSION_MINOR,
-                                       CLANG_VERSION_MAJOR, CLANG_VERSION_MINOR,
-                                       !isysroot.empty(), IncludeTimestamps,
-                                       ASTHasCompilerErrors};
+    RecordData::value_type Record[] = {
+        METADATA,
+        VERSION_MAJOR,
+        VERSION_MINOR,
+        CLANG_VERSION_MAJOR,
+        CLANG_VERSION_MINOR,
+        !isysroot.empty(),
+        IncludeTimestamps,
+        Context.getLangOpts().BuildingPCHWithObjectFile,
+        ASTHasCompilerErrors};
     Stream.EmitRecordWithBlob(MetadataAbbrevCode, Record,
                               getClangFullRepositoryVersion());
   }
diff --git a/clang/test/CodeGen/pch-dllexport.cpp b/clang/test/CodeGen/pch-dllexport.cpp
new file mode 100644
index 0000000..9c6623b
--- /dev/null
+++ b/clang/test/CodeGen/pch-dllexport.cpp
@@ -0,0 +1,84 @@
+// 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
+
+// Check for vars separately to avoid having to reorder the check statements.
+// RUN: %clang_cc1 -triple i686-pc-win32 -fms-extensions -emit-obj -emit-llvm -include-pch %t -o - %s | FileCheck -check-prefix=PCHWITHOBJVARS %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 {{.*}}foo
+
+
+// This function is referenced, so gets emitted as usual.
+inline void __declspec(dllexport) baz() {}
+// OBJ: define weak_odr dso_local dllexport void @"?baz@@YAXXZ"
+// PCH: define weak_odr dso_local dllexport void @"?baz@@YAXXZ"
+// PCHWITHOBJ: define weak_odr dso_local dllexport void @"?baz@@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 {{.*}}bar
+};
+
+// This isn't dllexported, attribute((used)) or referenced, so not emitted.
+inline void quux() {}
+// OBJ-NOT: define {{.*}}quux
+// PCH-NOT: define {{.*}}quux
+// PCHWITHOBJ-NOT: define {{.*}}quux
+
+// Referenced non-dllexport function.
+inline void referencedNonExported() {}
+// OBJ: define {{.*}}referencedNonExported
+// PCH: define {{.*}}referencedNonExported
+// PCHWITHOBJ: define {{.*}}referencedNonExported
+
+template <typename T> void __declspec(dllexport) implicitInstantiation(T) {}
+
+template <typename T> inline void __declspec(dllexport) explicitSpecialization(T) {}
+
+template <typename T> void __declspec(dllexport) explicitInstantiationDef(T) {}
+
+template <typename T> void __declspec(dllexport) explicitInstantiationDefAfterDecl(T) {}
+extern template void explicitInstantiationDefAfterDecl<int>(int);
+
+template <typename T> T __declspec(dllexport) variableTemplate;
+extern template int variableTemplate<int>;
+
+#else
+
+void use() {
+  baz();
+  referencedNonExported();
+}
+
+// Templates can be tricky. None of the definitions below come from the PCH.
+
+void useTemplate() { implicitInstantiation(42); }
+// PCHWITHOBJ: define weak_odr dso_local dllexport void @"??$implicitInstantiation@H@@YAXH@Z"
+
+template<> inline void __declspec(dllexport) explicitSpecialization<int>(int) {}
+// PCHWITHOBJ: define weak_odr dso_local  dllexport void @"??$explicitSpecialization@H@@YAXH@Z"
+
+template void __declspec(dllexport) explicitInstantiationDef<int>(int);
+// PCHWITHOBJ: define weak_odr dso_local dllexport void @"??$explicitInstantiationDef@H@@YAXH@Z"
+
+template void __declspec(dllexport) explicitInstantiationDefAfterDecl<int>(int);
+// PCHWITHOBJ: define weak_odr dso_local dllexport void @"??$explicitInstantiationDefAfterDecl@H@@YAXH@Z"(i32)
+
+template int __declspec(dllexport) variableTemplate<int>;
+// PCHWITHOBJVARS: @"??$variableTemplate@H@@3HA" = weak_odr dso_local dllexport global
+
+#endif
diff --git a/clang/test/Driver/cl-pch.cpp b/clang/test/Driver/cl-pch.cpp
index 2503c97..bef5ce6 100644
--- a/clang/test/Driver/cl-pch.cpp
+++ b/clang/test/Driver/cl-pch.cpp
@@ -7,6 +7,7 @@
 // 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
@@ -14,6 +15,7 @@
 // 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.