[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

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.

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

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

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 

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.

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

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;
 

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

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

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;

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

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

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,

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.

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

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

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.