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

[hans]

Summary

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

[thakis]

Looks like pretty straightforward plumbing. I can't think of a better place than ASTContext either. I think this is fine with your two XXXs just removed.

Exciting build time wins!

lib/AST/ASTContext.cpp

9557

nit: s/be/have/

[hans]

I think you revved the PCH version last. Did you have to do anything special, e.g. will it break vendors who ship PCH files as part of SDKs and such? Or are things generally set up to handle this?

[thakis]

PCHs aren't compatible with themselves if only the compiler revision changes, so I'm not sure changing that field should be worse than a regular compiler revision update (which happens at every commit). But I don't know what this field is for. I don't remember any trouble from me changing it though, and if it was bad to change it I'd hope there'd be a comment right above the field telling us why. (If I had run into problems, I would've expected me to add a comment like this.)

[hans]

In D48426#1139318, @thakis wrote:

PCHs aren't compatible with themselves if only the compiler revision changes, so I'm not sure changing that field should be worse than a regular compiler revision update (which happens at every commit). But I don't know what this field is for. I don't remember any trouble from me changing it though, and if it was bad to change it I'd hope there'd be a comment right above the field telling us why. (If I had run into problems, I would've expected me to add a comment like this.)

Sounds good, thanks.

[thakis]

lib/AST/ASTContext.cpp

9563

It'd be good to add a comment (and a test) explaining why / how _referenced_ inline functions still get emitted for inlining.

[rnk]

LangOpts.ModulesCodegen is very related in spirit to this, but I think we need a distinct option because that was designed to handle all inline functions (too much), not just dllexport inline functions. + @dblaikie

include/clang/AST/ASTContext.h

2886–2887 ↗

(On Diff #152267)

Does LangOpts.CompilingPCH do the right thing, or is that also true when we make a PCH with no object file? In any case, I'd probably make this a langopt. Even if it's functionally different from ModulesCodegen and ModulesDebugInfo, it's the same basic idea.

[dblaikie]

In D48426#1139823, @rnk wrote:

LangOpts.ModulesCodegen is very related in spirit to this, but I think we need a distinct option because that was designed to handle all inline functions (too much), not just dllexport inline functions. + @dblaikie

Does it have to be only the dllexported inline functions - or could this be used to home all inline functions? (I guess not - presumably it's not acceptable for the compiler to implicitly promote something to dllexport (& if it doesn't do that promotion, then the function wouldn't be visible from the use))

Is it valid to provide a definition for optimization purposes (LLVM's available_externally linkage) for these inline functions? Or is it required that, after a change to the header (modifying the implementation of one of these dllexported inline functions), the DLL they're exported could be rebuilt without rebuilding the clients & the change in behavior would be correctly applied?

test/CodeGen/pch-dllexport.cpp

23

This is a pretty specific "NOT" - maybe:

define {{.*}}@"?bar@...

would be better to ensure no definition is provided? (similarly above)

Also, should this file have some non-exported inline functions to check those do the right thing?

[hans]

In D48426#1140057, @dblaikie wrote:

In D48426#1139823, @rnk wrote:

LangOpts.ModulesCodegen is very related in spirit to this, but I think we need a distinct option because that was designed to handle all inline functions (too much), not just dllexport inline functions. + @dblaikie

Does it have to be only the dllexported inline functions - or could this be used to home all inline functions? (I guess not - presumably it's not acceptable for the compiler to implicitly promote something to dllexport (& if it doesn't do that promotion, then the function wouldn't be visible from the use))

Right, I don't think we could do that. But regular inline functions isn't as large a problem as the dllexport ones, which we have to emit even if they're not referenced, causing a huge amount of redundant codegen.

Is it valid to provide a definition for optimization purposes (LLVM's available_externally linkage) for these inline functions? Or is it required that, after a change to the header (modifying the implementation of one of these dllexported inline functions), the DLL they're exported could be rebuilt without rebuilding the clients & the change in behavior would be correctly applied?

Yes, we will provide such definitions (not just available_externally, but weak_odr) if the inline function is referenced, as usual. This is only homing unreferenced but "force emitted", i.e. DeclMustBeEmitted, functions.

include/clang/AST/ASTContext.h

2886–2887 ↗

(On Diff #152267)

Yes, CompilingPCH is always set when building PCHs, also when there's no object file so we can't use that.

Moving BuildingPCHWithObjectFile to LangOpts sounds good to me.

Actually that's interesting, because it means the flag will get written into the PCH will all the other LangOpts, and we could in theory look for that in ASTReader. But it seems the code isn't really set up to store the LangOpts coming out of an AST file, we only verify that they're compatible.

test/CodeGen/pch-dllexport.cpp

23

Updated the -NOT checks and added test.

[hans]

Addressing review comments.

[thakis]

test/CodeGen/pch-dllexport.cpp

41

There still isn't a non-dllexported-but-referenced inline function in here as far as I can see. Am I just missing it?

[hans]

I hit a snag while building some more Chromium targets. For class templates with explicit instantiation decls in the PCH file and explicit instantiation definitions in a .cc file, the function definition will be marked as coming from the PCH, even though it wasn't defined there. For example:

#ifndef IN_HEADER
#define IN_HEADER

template <typename T> struct Template { Template() {} };
extern template class Template<int>;

#else

template class __declspec(dllexport) Template<int>;

#endif

This isn't a problem for regular functions where we build a decl chain: the first declaration may come from a PCH, but the definition would be a separate Decl object which does not.

I'm trying to figure out how to handle this.

[hans]

Added special-casing for explicit template instantiations, and missing test case suggested by Nico.

Please take another look.

[thakis]

Still looks good, ship it! One more suggestion about additional test coverage (but maybe it's already there and I'm just missing it).

test/CodeGen/pch-dllexport.cpp

56

This has two interesting cases:

1. An explicit instantiation declaration.
2. An explicit instantiation definition.

You have a test for 2, but not for 1, as far as I can tell (?). For 1, the inline function from the pch-obj should be used (which probably already works; if not a FIXME is enough).

[hans]

test/CodeGen/pch-dllexport.cpp

56

It's really testing the "explicit instantiation def after explicit instantiation decl" case.

With just an explicit instantiation decl (1), there's not much to test, since that's like a promise that someone else is going to instantiate the template. Nothing needs to be emitted for that.

Maybe I'm not understanding your question?

[thakis]

test/CodeGen/pch-dllexport.cpp

56

Nothing needs to be emitted for that.

Right, I meant including a test that we really don't emit anything for that. But feel free to omit it if you don't think it adds much. I suggested it because we end up with TSK_ExplicitInstantiationDeclaration in that case, and you're checking the TSK in this change, and you have coverage for the other kinds.

include/clang/AST/ExternalASTSource.h

public:
   /// context is up to date.
   ///
   /// The default implementation of this function is a no-op.
   virtual void completeVisibleDeclsMap(const DeclContext *DC);
 
   /// 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.
   class ASTSourceDescriptor {
     StringRef PCHModuleName;
     StringRef Path;
     StringRef ASTFile;
     ASTFileSignature Signature;

include/clang/Basic/LangOptions.def

 BENIGN_LANGOPT(EmitAllDecls      , 1, 0, "emitting all declarations")
 LANGOPT(MathErrno         , 1, 1, "errno in math functions")
 BENIGN_LANGOPT(HeinousExtensions , 1, 0, "extensions that we really don't like and may be ripped out at any time")
 LANGOPT(Modules           , 1, 0, "modules extension to C")
 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")
 BENIGN_LANGOPT(ModulesErrorRecovery, 1, 1, "automatically importing modules as needed when performing error recovery")
 BENIGN_LANGOPT(ImplicitModules, 1, 1, "building modules that are not specified via -fmodule-file")
 COMPATIBLE_LANGOPT(ModulesLocalVisibility, 1, 0, "local submodule visibility")
 COMPATIBLE_LANGOPT(Optimize          , 1, 0, "__OPTIMIZE__ predefined macro")
 COMPATIBLE_LANGOPT(OptimizeSize      , 1, 0, "__OPTIMIZE_SIZE__ predefined macro")

include/clang/Driver/CC1Options.td

 def foverride_record_layout_EQ : Joined<["-"], "foverride-record-layout=">,
   HelpText<"Override record layouts with those in the given file">;
 def find_pch_source_EQ : Joined<["-"], "find-pch-source=">,
   HelpText<"When building a pch, try to find the input file in include "
            "directories, as if it had been included by the argument passed "
            "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">;
 
 //===----------------------------------------------------------------------===//
 // Language Options
 //===----------------------------------------------------------------------===//
 
 let Flags = [CC1Option, CC1AsOption, NoDriverOption] in {

include/clang/Sema/MultiplexExternalSemaSource.h

public:
 
   /// Print any statistics that have been gathered regarding
   /// the external AST source.
   void PrintStats() override;
 
   /// 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 
   /// layout for a record, overriding the layout that would normally be
   /// constructed. It is intended for clients who receive specific layout
   /// details rather than source code (such as LLDB). The client is expected
   /// to fill in the field offsets, base offsets, virtual base offsets, and
   /// complete object size.

include/clang/Serialization/ASTBitCodes.h

namespace serialization {
     /// Whenever the AST file format changes in a way that makes it
     /// incompatible with previous versions (such that a reader
     /// designed for the previous version could not support reading
     /// the new version), this number should be increased.
     ///
     /// 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.
     ///
     /// Whenever the AST format changes in a way that is still
     /// compatible with previous versions (such that a reader designed
     /// for the previous version could still support reading the new
     /// version by ignoring new kinds of subblocks), this number

include/clang/Serialization/ASTReader.h

public:
   ///
   Module *getSubmodule(serialization::SubmoduleID GlobalID);
 
   /// Retrieve the module that corresponds to the given module ID.
   ///
   /// 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);
 
   /// Get an ID for the given module file.
   unsigned getModuleFileID(ModuleFile *M);
 
   /// Return a descriptor for the corresponding module.

include/clang/Serialization/Module.h

public:
   std::string ModuleMapPath;
 
   /// Whether this precompiled header is a relocatable PCH file.
   bool RelocatablePCH = false;
 
   /// 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;
 
   /// The signature of the module file, which may be used instead of the size
   /// and modification time to identify this particular file.
   ASTFileSignature Signature;
 
   /// Whether this module has been directly imported by the

lib/AST/ASTContext.cpp

else if (isa<OMPThreadPrivateDecl>(D))
     return !D->getDeclContext()->isDependentContext();
   else if (isa<OMPDeclareReductionDecl>(D))
     return !D->getDeclContext()->isDependentContext();
   else if (isa<ImportDecl>(D))
     return true;
   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.
+    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;
 
   // Weak references don't produce any output by themselves.
   if (D->hasAttr<WeakRefAttr>())
     return false;
 
   // Aliases and used decls are required.
   if (D->hasAttr<AliasAttr>() || D->hasAttr<UsedAttr>())
     return true;
 
   if (const auto *FD = dyn_cast<FunctionDecl>(D)) {
     // Forward declarations aren't required.
     if (!FD->doesThisDeclarationHaveABody())
       return FD->doesDeclarationForceExternallyVisibleDefinition();
 
     // 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()) {
       if (const auto *MD = dyn_cast<CXXMethodDecl>(FD)) {
         const CXXRecordDecl *RD = MD->getParent();
         if (MD->isOutOfLine() && RD->isDynamicClass()) {
           const CXXMethodDecl *KeyFunc = getCurrentKeyFunction(RD);
           if (KeyFunc && KeyFunc->getCanonicalDecl() == MD->getCanonicalDecl())
             return true;
         }
       }
     }
 
     GVALinkage Linkage = GetGVALinkageForFunction(FD);
 
     // static, static inline, always_inline, and extern inline functions can
     // always be deferred.  Normal inline functions can be deferred in C99/C++.
     // 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");
 
   if (VD->isThisDeclarationADefinition() == VarDecl::DeclarationOnly &&
       !isMSStaticDataMemberInlineDefinition(VD))
     return false;
 
   // Variables that can be needed in other TUs are required.

lib/Driver/Driver.cpp

if (YcArg) {
             C.MakeAction<InputAction>(*PchInputArg, HeaderType);
         for (phases::ID Phase : PCHPL)
           ClangClPch = ConstructPhaseAction(C, Args, Phase, ClangClPch);
         assert(ClangClPch);
         Actions.push_back(ClangClPch);
         // 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.
       }
     }
 
     // Build the pipeline for this file.
     Action *Current = C.MakeAction<InputAction>(*InputArg, InputType);
 
     // Use the current host action in any of the offloading actions, if
     // required.

lib/Driver/ToolChains/Clang.cpp

void Clang::AddPreprocessingOptions(Compilation &C, const JobAction &JA,
   }
   if (isa<PrecompileJobAction>(JA) && YcIndex != -1) {
     Driver::InputList Inputs;
     D.BuildInputs(getToolChain(), C.getArgs(), Inputs);
     assert(Inputs.size() == 1 && "Need one input when building pch");
     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;
   for (const Arg *A : Args.filtered(options::OPT_clang_i_Group)) {
     ++AI;
 
     if (getToolChain().getDriver().IsCLMode() &&
         A->getOption().matches(options::OPT_include)) {

lib/Frontend/CompilerInvocation.cpp

if (!VerParts.first.startswith("0") &&
         Opts.setClangABICompat(LangOptions::ClangABI::Ver6);
     } else if (Ver != "latest") {
       Diags.Report(diag::err_drv_invalid_value)
           << A->getAsString(Args) << A->getValue();
     }
   }
 
   Opts.CompleteMemberPointers = Args.hasArg(OPT_fcomplete_member_pointers);
+  Opts.BuildingPCHWithObjectFile = Args.hasArg(OPT_building_pch_with_obj);
 }
 
 static bool isStrictlyPreprocessorAction(frontend::ActionKind Action) {
   switch (Action) {
   case frontend::ASTDeclList:
   case frontend::ASTDump:
   case frontend::ASTPrint:
   case frontend::ASTView:

lib/Frontend/FrontendActions.cpp

GeneratePCHAction::CreateASTConsumer(CompilerInstance &CI, StringRef InFile) {
   std::unique_ptr<raw_pwrite_stream> OS =
       CreateOutputFile(CI, InFile, /*ref*/ OutputFile);
   if (!OS)
     return nullptr;
 
   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));
 
   return llvm::make_unique<MultiplexConsumer>(std::move(Consumers));
 }
 
 bool GeneratePCHAction::ComputeASTConsumerArguments(CompilerInstance &CI,
                                                     std::string &Sysroot) {

lib/Sema/MultiplexExternalSemaSource.cpp

 
 Module *MultiplexExternalSemaSource::getModule(unsigned ID) {
   for (size_t i = 0; i < Sources.size(); ++i)
     if (auto M = Sources[i]->getModule(ID))
       return M;
   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,
                       llvm::DenseMap<const FieldDecl *, uint64_t> &FieldOffsets,
                   llvm::DenseMap<const CXXRecordDecl *, CharUnits> &BaseOffsets,
           llvm::DenseMap<const CXXRecordDecl *, CharUnits> &VirtualBaseOffsets){
   for(size_t i = 0; i < Sources.size(); ++i)
     if (Sources[i]->layoutRecordType(Record, Size, Alignment, FieldOffsets, 

lib/Serialization/ASTReader.cpp

while (true) {
     case METADATA: {
       if (Record[0] != VERSION_MAJOR && !DisableValidation) {
         if ((ClientLoadCapabilities & ARR_VersionMismatch) == 0)
           Diag(Record[0] < VERSION_MAJOR? diag::err_pch_version_too_old
                                         : diag::err_pch_version_too_new);
         return VersionMismatch;
       }
 
-      bool hasErrors = Record[6];
+      bool hasErrors = Record[7];
       if (hasErrors && !DisableValidation && !AllowASTWithCompilerErrors) {
         Diag(diag::err_pch_with_compiler_errors);
         return HadErrors;
       }
       if (hasErrors) {
         Diags.ErrorOccurred = true;
         Diags.UncompilableErrorOccurred = true;
         Diags.UnrecoverableErrorOccurred = true;
       }
 
       F.RelocatablePCH = Record[4];
       // Relative paths in a relocatable PCH are relative to our sysroot.
       if (F.RelocatablePCH)
         F.BaseDirectory = isysroot.empty() ? "/" : isysroot;
 
       F.HasTimestamps = Record[5];
 
+      F.PCHHasObjectFile = Record[6];
+
       const std::string &CurBranch = getClangFullRepositoryVersion();
       StringRef ASTBranch = Blob;
       if (StringRef(CurBranch) != ASTBranch && !DisableValidation) {
         if ((ClientLoadCapabilities & ARR_VersionMismatch) == 0)
           Diag(diag::err_pch_different_branch) << ASTBranch << CurBranch;
         return VersionMismatch;
       }
       break;
Module *ASTReader::getSubmodule(SubmoduleID GlobalID) {
 
   return SubmodulesLoaded[GlobalID - NUM_PREDEF_SUBMODULE_IDS];
 }
 
 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.
     auto I = GlobalSubmoduleMap.find(getGlobalSubmoduleID(F, ID >> 1));
     return I == GlobalSubmoduleMap.end() ? nullptr : I->second;
   } else {
     // It's a prefix (preamble, PCH, ...). Look it up by index.
     unsigned IndexFromEnd = ID >> 1;

lib/Serialization/ASTWriter.cpp

void ASTWriter::WriteControlBlock(Preprocessor &PP, ASTContext &Context,
   auto MetadataAbbrev = std::make_shared<BitCodeAbbrev>();
   MetadataAbbrev->Add(BitCodeAbbrevOp(METADATA));
   MetadataAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 16)); // Major
   MetadataAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 16)); // Minor
   MetadataAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 16)); // Clang maj.
   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,
     Stream.EmitRecordWithBlob(MetadataAbbrevCode, Record,
                               getClangFullRepositoryVersion());
   }
 
   if (WritingModule) {
     // Module name
     auto Abbrev = std::make_shared<BitCodeAbbrev>();
     Abbrev->Add(BitCodeAbbrevOp(MODULE_NAME));

test/CodeGen/pch-dllexport.cpp

+// 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 {{.*}}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
+
+#else
+
+void use() { baz(); }
+
+#endif

test/Driver/cl-pch.cpp

 // Note: %s and %S must be preceded by --, otherwise it may be interpreted as a
 // command-line option, e.g. on Mac where %s is commonly under /Users.
 
 // /Yc
 // RUN: %clang_cl -Werror /Ycpchfile.h /FIpchfile.h /c -### -- %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-YC %s
 // 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
 
 // /Yc /Fo
 // /Fo overrides the .obj output filename, but not the .pch filename
 // RUN: %clang_cl -Werror /Fomyobj.obj /Ycpchfile.h /FIpchfile.h /c -### -- %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-YCO %s
 // 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
 // CHECK-YCO: myobj.obj
 
 // /Yc /Y-
 // /Y- disables pch generation
 // RUN: %clang_cl -Werror /Y- /Ycpchfile.h /FIpchfile.h /c -### -- %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-YC-Y_ %s
 // CHECK-YC-Y_-NOT: -emit-pch
 // CHECK-YC-Y_-NOT: -include-pch
 
 // /Yu
 // RUN: %clang_cl -Werror /Yupchfile.h /FIpchfile.h /c -### -- %s 2>&1 \
 // 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
 // CHECK-YU: pchfile.pch
 
 // /Yu /Y-
 // RUN: %clang_cl -Werror /Y- /Yupchfile.h /FIpchfile.h /c -### -- %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-YU-Y_ %s
 // CHECK-YU-Y_-NOT: -emit-pch
 // CHECK-YU-Y_-NOT: -include-pch
 
 // /Yc /Yu -- /Yc overrides /Yc if they both refer to the same file
 // RUN: %clang_cl -Werror /Ycpchfile.h /Yupchfile.h /FIpchfile.h /c -### -- %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-YC-YU %s
 // 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.
 // CHECK-YC-YU: cc1
 // CHECK-YC-YU: -emit-obj
 // CHECK-YC-YU: -include-pch
 // CHECK-YC-YU: pchfile.pch