[clang] b198de6 - Merge some of the PCH object support with modular codegen
David Blaikie via cfe-commits
cfe-commits at lists.llvm.org
Wed Jul 22 12:46:22 PDT 2020
Author: David Blaikie
Date: 2020-07-22T12:46:12-07:00
New Revision: b198de67e0bab462217db50814b1434796fa7caf
URL: https://github.com/llvm/llvm-project/commit/b198de67e0bab462217db50814b1434796fa7caf
DIFF: https://github.com/llvm/llvm-project/commit/b198de67e0bab462217db50814b1434796fa7caf.diff
LOG: Merge some of the PCH object support with modular codegen
I was trying to pick this up a bit when reviewing D48426 (& perhaps D69778) - in any case, looks like D48426 added a module level flag that might not be needed.
The D48426 implementation worked by setting a module level flag, then code generating contents from the PCH a special case in ASTContext::DeclMustBeEmitted would be used to delay emitting the definition of these functions if they came from a Module with this flag.
This strategy is similar to the one initially implemented for modular codegen that was removed in D29901 in favor of the modular decls list and a bit on each decl to specify whether it's homed to a module.
One major difference between PCH object support and modular code generation, other than the specific list of decls that are homed, is the compilation model: MSVC PCH modules are built into the object file for some other source file (when compiling that source file /Yc is specified to say "this compilation is where the PCH is homed"), whereas modular code generation invokes a separate compilation for the PCH alone. So the current modular code generation test of to decide if a decl should be emitted "is the module where this decl is serialized the current main file" has to be extended (as Lubos did in D69778) to also test the command line flag -building-pch-with-obj.
Otherwise the whole thing is basically streamlined down to the modular code generation path.
This even offers one extra material improvement compared to the existing divergent implementation: Homed functions are not emitted into object files that use the pch. Instead at -O0 they are not emitted into the IR at all, and at -O1 they are emitted using available_externally (existing functionality implemented for modular code generation). The pch-codegen test has been updated to reflect this new behavior.
[If possible: I'd love it if we could not have the extra MSVC-style way of accessing dllexport-pch-homing, and just do it the modular codegen way, but I understand that it might be a limitation of existing build systems. @hans / @thakis: Do either of you know if it'd be practical to move to something more similar to .pcm handling, where the pch itself is passed to the compilation, rather than homed as a side effect of compiling some other source file?]
Reviewers: llunak, hans
Differential Revision: https://reviews.llvm.org/D83652
Added:
Modified:
clang/include/clang/AST/ExternalASTSource.h
clang/include/clang/Sema/MultiplexExternalSemaSource.h
clang/include/clang/Serialization/ASTReader.h
clang/include/clang/Serialization/ModuleFile.h
clang/lib/AST/ASTContext.cpp
clang/lib/Sema/MultiplexExternalSemaSource.cpp
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTReaderDecl.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/lib/Serialization/ASTWriterDecl.cpp
clang/test/CodeGen/pch-dllexport.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/AST/ExternalASTSource.h b/clang/include/clang/AST/ExternalASTSource.h
index def877b91816..caae0770931b 100644
--- a/clang/include/clang/AST/ExternalASTSource.h
+++ b/clang/include/clang/AST/ExternalASTSource.h
@@ -161,10 +161,6 @@ class ExternalASTSource : public RefCountedBase<ExternalASTSource> {
/// 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; }
-
/// Return a descriptor for the corresponding module, if one exists.
virtual llvm::Optional<ASTSourceDescriptor> getSourceDescriptor(unsigned ID);
diff --git a/clang/include/clang/Sema/MultiplexExternalSemaSource.h b/clang/include/clang/Sema/MultiplexExternalSemaSource.h
index e94dd5d46871..b54a6283d640 100644
--- a/clang/include/clang/Sema/MultiplexExternalSemaSource.h
+++ b/clang/include/clang/Sema/MultiplexExternalSemaSource.h
@@ -153,8 +153,6 @@ class MultiplexExternalSemaSource : public ExternalSemaSource {
/// 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/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h
index 1d5571cd2854..e883eb2f1749 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -2085,8 +2085,6 @@ class ASTReader
/// 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/ModuleFile.h b/clang/include/clang/Serialization/ModuleFile.h
index cec29da69372..598e61210702 100644
--- a/clang/include/clang/Serialization/ModuleFile.h
+++ b/clang/include/clang/Serialization/ModuleFile.h
@@ -155,9 +155,6 @@ class ModuleFile {
/// Whether timestamps are included in this module file.
bool HasTimestamps = false;
- /// Whether the PCH has a corresponding object file.
- bool PCHHasObjectFile = false;
-
/// Whether the top-level module has been read from the AST file.
bool DidReadTopLevelSubmodule = false;
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 4fde647cefde..fc7631712c3c 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -10473,37 +10473,6 @@ 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;
-
- // Implicit member function definitions, such as operator= might not be
- // marked as template specializations, since they're not coming from a
- // template but synthesized directly on the class.
- IsExpInstDef |=
- isa<CXXMethodDecl>(D) &&
- cast<CXXMethodDecl>(D)->getParent()->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;
diff --git a/clang/lib/Sema/MultiplexExternalSemaSource.cpp b/clang/lib/Sema/MultiplexExternalSemaSource.cpp
index 80333e63127e..252008cda15d 100644
--- a/clang/lib/Sema/MultiplexExternalSemaSource.cpp
+++ b/clang/lib/Sema/MultiplexExternalSemaSource.cpp
@@ -172,13 +172,6 @@ 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 4a1a995204e5..6b96b4ff59b8 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -2747,7 +2747,7 @@ ASTReader::ReadControlBlock(ModuleFile &F,
return VersionMismatch;
}
- bool hasErrors = Record[7];
+ bool hasErrors = Record[6];
if (hasErrors && !DisableValidation && !AllowASTWithCompilerErrors) {
Diag(diag::err_pch_with_compiler_errors);
return HadErrors;
@@ -2765,8 +2765,6 @@ 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) {
@@ -8590,11 +8588,6 @@ 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/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index eef4ab16ec15..0b87161ddeea 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -504,10 +504,9 @@ uint64_t ASTDeclReader::GetCurrentCursorOffset() {
void ASTDeclReader::ReadFunctionDefinition(FunctionDecl *FD) {
if (Record.readInt()) {
- Reader.DefinitionSource[FD] = Loc.F->Kind == ModuleKind::MK_MainFile;
- if (Reader.getContext().getLangOpts().BuildingPCHWithObjectFile &&
- Reader.DeclIsFromPCHWithObjectFile(FD))
- Reader.DefinitionSource[FD] = true;
+ Reader.DefinitionSource[FD] =
+ Loc.F->Kind == ModuleKind::MK_MainFile ||
+ Reader.getContext().getLangOpts().BuildingPCHWithObjectFile;
}
if (auto *CD = dyn_cast<CXXConstructorDecl>(FD)) {
CD->setNumCtorInitializers(Record.readInt());
@@ -1436,10 +1435,9 @@ ASTDeclReader::RedeclarableResult ASTDeclReader::VisitVarDeclImpl(VarDecl *VD) {
}
if (VD->getStorageDuration() == SD_Static && Record.readInt()) {
- Reader.DefinitionSource[VD] = Loc.F->Kind == ModuleKind::MK_MainFile;
- if (Reader.getContext().getLangOpts().BuildingPCHWithObjectFile &&
- Reader.DeclIsFromPCHWithObjectFile(VD))
- Reader.DefinitionSource[VD] = true;
+ Reader.DefinitionSource[VD] =
+ Loc.F->Kind == ModuleKind::MK_MainFile ||
+ Reader.getContext().getLangOpts().BuildingPCHWithObjectFile;
}
enum VarKind {
@@ -1700,10 +1698,9 @@ void ASTDeclReader::ReadCXXDefinitionData(
Data.HasODRHash = true;
if (Record.readInt()) {
- Reader.DefinitionSource[D] = Loc.F->Kind == ModuleKind::MK_MainFile;
- if (Reader.getContext().getLangOpts().BuildingPCHWithObjectFile &&
- Reader.DeclIsFromPCHWithObjectFile(D))
- Reader.DefinitionSource[D] = true;
+ Reader.DefinitionSource[D] =
+ Loc.F->Kind == ModuleKind::MK_MainFile ||
+ Reader.getContext().getLangOpts().BuildingPCHWithObjectFile;
}
Data.NumBases = Record.readInt();
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index a8803aeb1b8a..9ea90b2a0212 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -1119,7 +1119,6 @@ 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));
@@ -1134,7 +1133,6 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, ASTContext &Context,
CLANG_VERSION_MINOR,
!isysroot.empty(),
IncludeTimestamps,
- Context.getLangOpts().BuildingPCHWithObjectFile,
ASTHasCompilerErrors};
Stream.EmitRecordWithBlob(MetadataAbbrevCode, Record,
getClangFullRepositoryVersion());
diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp
index eecdf89c791a..ec05ab772d93 100644
--- a/clang/lib/Serialization/ASTWriterDecl.cpp
+++ b/clang/lib/Serialization/ASTWriterDecl.cpp
@@ -1031,8 +1031,10 @@ void ASTDeclWriter::VisitVarDecl(VarDecl *D) {
// that module interface unit, not by its users. (Inline variables are
// still emitted in module users.)
ModulesCodegen =
- (Writer.WritingModule->Kind == Module::ModuleInterfaceUnit &&
- Writer.Context->GetGVALinkageForVariable(D) == GVA_StrongExternal);
+ (Writer.WritingModule->Kind == Module::ModuleInterfaceUnit ||
+ (D->hasAttr<DLLExportAttr>() &&
+ Writer.Context->getLangOpts().BuildingPCHWithObjectFile)) &&
+ Writer.Context->GetGVALinkageForVariable(D) == GVA_StrongExternal;
}
Record.push_back(ModulesCodegen);
if (ModulesCodegen)
@@ -2469,7 +2471,10 @@ void ASTRecordWriter::AddFunctionDefinition(const FunctionDecl *FD) {
Linkage = Writer->Context->GetGVALinkageForFunction(FD);
ModulesCodegen = *Linkage == GVA_StrongExternal;
}
- if (Writer->Context->getLangOpts().ModulesCodegen) {
+ if (Writer->Context->getLangOpts().ModulesCodegen ||
+ (FD->hasAttr<DLLExportAttr>() &&
+ Writer->Context->getLangOpts().BuildingPCHWithObjectFile)) {
+
// Under -fmodules-codegen, codegen is performed for all non-internal,
// non-always_inline functions, unless they are available elsewhere.
if (!FD->hasAttr<AlwaysInlineAttr>()) {
diff --git a/clang/test/CodeGen/pch-dllexport.cpp b/clang/test/CodeGen/pch-dllexport.cpp
index 034995fdffc4..72f344c77be1 100644
--- a/clang/test/CodeGen/pch-dllexport.cpp
+++ b/clang/test/CodeGen/pch-dllexport.cpp
@@ -3,13 +3,20 @@
// 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
+// RUN: %clang_cc1 -triple i686-pc-win32 -O1 -fms-extensions -emit-pch -building-pch-with-obj -o %t %s
+// RUN: %clang_cc1 -triple i686-pc-win32 -O1 -disable-llvm-optzns -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 -O1 -disable-llvm-optzns -fms-extensions -emit-obj -emit-llvm -include-pch %t -o - %s | FileCheck -check-prefix=PCHWITHOBJ -check-prefix=PCHWITHOBJ-O1 %s
// Check for vars separately to avoid having to reorder the check statements.
+// RUN: %clang_cc1 -triple i686-pc-win32 -O1 -disable-llvm-optzns -fms-extensions -emit-obj -emit-llvm -include-pch %t -o - %s | FileCheck -check-prefix=PCHWITHOBJVARS %s
+
+// Test the PCHWITHOBJ at -O0 where available_externally definitions are not
+// provided:
+// 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 -o - %s | FileCheck -check-prefix=PCHWITHOBJ -check-prefix=PCHWITHOBJ-O0 %s
// 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
@@ -23,7 +30,8 @@ inline void __declspec(dllexport) foo() {}
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"
+// PCHWITHOBJ-O1: define available_externally dso_local void @"?baz@@YAXXZ"
+// PCHWITHOBJ-O0-NOT: define {{.*}}"?baz@@YAXXZ"
struct __declspec(dllexport) S {
More information about the cfe-commits
mailing list