[llvm-branch-commits] [clang] [Serialization] Load Specialization Lazily (2/2) (PR #77417)
via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Mon Jan 8 23:04:50 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-modules
Author: Chuanqi Xu (ChuanqiXu9)
<details>
<summary>Changes</summary>
This is the successor of https://github.com/llvm/llvm-project/pull/76774.
I meant to use spr but I failed. So I created the stacked review here manually. Hope this won't be too bad.
The core idea of the patch is: when we see the new specialization for template decl in other modules, we don't add the specialization to the update list (so it'll be loaded at once if needed). But we add these specializations and the corresponding hashed values to a hash table and we wrote the hash table to disk instead. So when we read that template decl in other modules, we will associate the hash table with that template decl.
In fact, the whole process is exactly the same with how we update the name lookup table.
---
Full diff: https://github.com/llvm/llvm-project/pull/77417.diff
10 Files Affected:
- (modified) clang/include/clang/Serialization/ASTBitCodes.h (+2)
- (modified) clang/include/clang/Serialization/ASTReader.h (+8-3)
- (modified) clang/include/clang/Serialization/ASTWriter.h (+7)
- (modified) clang/lib/Serialization/ASTCommon.h (+1-1)
- (modified) clang/lib/Serialization/ASTReader.cpp (+20-4)
- (modified) clang/lib/Serialization/ASTReaderDecl.cpp (+23-14)
- (modified) clang/lib/Serialization/ASTWriter.cpp (+35-7)
- (modified) clang/lib/Serialization/ASTWriterDecl.cpp (+5-2)
- (modified) clang/test/Modules/cxx-templates.cpp (+5-4)
- (modified) clang/unittests/Serialization/LoadSpecLazily.cpp (+34)
``````````diff
diff --git a/clang/include/clang/Serialization/ASTBitCodes.h b/clang/include/clang/Serialization/ASTBitCodes.h
index 23a279de96ab15..212ae7db30faa0 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -695,6 +695,8 @@ enum ASTRecordTypes {
/// Record code for an unterminated \#pragma clang assume_nonnull begin
/// recorded in a preamble.
PP_ASSUME_NONNULL_LOC = 67,
+
+ UPDATE_SPECIALIZATION = 68,
};
/// Record types used within a source manager block.
diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h
index 293d6495d164ef..ecd4932c16a401 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -610,18 +610,22 @@ class ASTReader
// Updates for visible decls can occur for other contexts than just the
// TU, and when we read those update records, the actual context may not
// be available yet, so have this pending map using the ID as a key. It
- // will be realized when the context is actually loaded.
- struct PendingVisibleUpdate {
+ // will be realized when the data is actually loaded.
+ struct UpdateData {
ModuleFile *Mod;
const unsigned char *Data;
};
- using DeclContextVisibleUpdates = SmallVector<PendingVisibleUpdate, 1>;
+ using DeclContextVisibleUpdates = SmallVector<UpdateData, 1>;
/// Updates to the visible declarations of declaration contexts that
/// haven't been loaded yet.
llvm::DenseMap<serialization::DeclID, DeclContextVisibleUpdates>
PendingVisibleUpdates;
+ using SpecializationsUpdate = SmallVector<UpdateData, 1>;
+ llvm::DenseMap<serialization::DeclID, SpecializationsUpdate>
+ PendingSpecializationsUpdates;
+
/// The set of C++ or Objective-C classes that have forward
/// declarations that have not yet been linked to their definitions.
llvm::SmallPtrSet<Decl *, 4> PendingDefinitions;
@@ -650,6 +654,7 @@ class ASTReader
bool ReadSpecializations(ModuleFile &M, llvm::BitstreamCursor &Cursor,
uint64_t Offset, Decl *D);
+ void AddSpecializations(const Decl *D, const unsigned char *Data, ModuleFile &M);
/// A vector containing identifiers that have already been
/// loaded.
diff --git a/clang/include/clang/Serialization/ASTWriter.h b/clang/include/clang/Serialization/ASTWriter.h
index 09806b87590766..9f689b18652589 100644
--- a/clang/include/clang/Serialization/ASTWriter.h
+++ b/clang/include/clang/Serialization/ASTWriter.h
@@ -385,6 +385,9 @@ class ASTWriter : public ASTDeserializationListener,
/// record containing modifications to them.
DeclUpdateMap DeclUpdates;
+ using SpecializationUpdateMap = llvm::MapVector<const NamedDecl *, SmallVector<const NamedDecl *>>;
+ SpecializationUpdateMap SpecializationsUpdates;
+
using FirstLatestDeclMap = llvm::DenseMap<Decl *, Decl *>;
/// Map of first declarations from a chained PCH that point to the
@@ -527,6 +530,9 @@ class ASTWriter : public ASTDeserializationListener,
bool isLookupResultExternal(StoredDeclsList &Result, DeclContext *DC);
bool isLookupResultEntirelyExternal(StoredDeclsList &Result, DeclContext *DC);
+ void GenerateSpecializationsLookupTable(const NamedDecl *D,
+ llvm::SmallVectorImpl<const NamedDecl *> &Specs,
+ llvm::SmallVectorImpl<char> &LookupTable);
uint64_t WriteSpecializationsLookupTable(
const NamedDecl *D,
llvm::SmallVectorImpl<const NamedDecl *> &Specializations);
@@ -542,6 +548,7 @@ class ASTWriter : public ASTDeserializationListener,
void WriteReferencedSelectorsPool(Sema &SemaRef);
void WriteIdentifierTable(Preprocessor &PP, IdentifierResolver &IdResolver,
bool IsModule);
+ void WriteSpecializationsUpdates();
void WriteDeclUpdatesBlocks(RecordDataImpl &OffsetsRecord);
void WriteDeclContextVisibleUpdate(const DeclContext *DC);
void WriteFPPragmaOptions(const FPOptionsOverride &Opts);
diff --git a/clang/lib/Serialization/ASTCommon.h b/clang/lib/Serialization/ASTCommon.h
index 296642e3674a49..9d190c05062444 100644
--- a/clang/lib/Serialization/ASTCommon.h
+++ b/clang/lib/Serialization/ASTCommon.h
@@ -23,7 +23,7 @@ namespace serialization {
enum DeclUpdateKind {
UPD_CXX_ADDED_IMPLICIT_MEMBER,
- UPD_CXX_ADDED_TEMPLATE_SPECIALIZATION,
+ UPD_CXX_ADDED_TEMPLATE_PARTIAL_SPECIALIZATION,
UPD_CXX_ADDED_ANONYMOUS_NAMESPACE,
UPD_CXX_ADDED_FUNCTION_DEFINITION,
UPD_CXX_ADDED_VAR_DEFINITION,
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index bcdd2dfc491d8f..54ce94a1a56713 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -1344,6 +1344,12 @@ bool ASTReader::ReadVisibleDeclContextStorage(ModuleFile &M,
return false;
}
+void ASTReader::AddSpecializations(const Decl *D, const unsigned char *Data, ModuleFile &M) {
+ D = D->getCanonicalDecl();
+ SpecializationsLookups[D].Table.add(&M, Data,
+ reader::SpecializationsLookupTrait(*this, M));
+}
+
bool ASTReader::ReadSpecializations(ModuleFile &M, BitstreamCursor &Cursor,
uint64_t Offset, Decl *D) {
assert(Offset != 0);
@@ -1375,10 +1381,7 @@ bool ASTReader::ReadSpecializations(ModuleFile &M, BitstreamCursor &Cursor,
}
auto *Data = (const unsigned char *)Blob.data();
- D = D->getCanonicalDecl();
- SpecializationsLookups[D].Table.add(
- &M, Data, reader::SpecializationsLookupTrait(*this, M));
-
+ AddSpecializations(D, Data, M);
return false;
}
@@ -3481,6 +3484,19 @@ llvm::Error ASTReader::ReadASTBlock(ModuleFile &F,
break;
}
+ case UPDATE_SPECIALIZATION: {
+ unsigned Idx = 0;
+ serialization::DeclID ID = ReadDeclID(F, Record, Idx);
+ auto *Data = (const unsigned char*)Blob.data();
+ PendingSpecializationsUpdates[ID].push_back(PendingVisibleUpdate{&F, Data});
+ // If we've already loaded the decl, perform the updates when we finish
+ // loading this block.
+ if (Decl *D = GetExistingDecl(ID))
+ PendingUpdateRecords.push_back(
+ PendingUpdateRecord(ID, D, /*JustLoaded=*/false));
+ break;
+ }
+
case IDENTIFIER_TABLE:
F.IdentifierTableData =
reinterpret_cast<const unsigned char *>(Blob.data());
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index e3ffcee6988b3f..68c1536b192fa6 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -321,7 +321,8 @@ namespace clang {
void ReadFunctionDefinition(FunctionDecl *FD);
void Visit(Decl *D);
- void UpdateDecl(Decl *D, SmallVectorImpl<serialization::DeclID> &);
+ void UpdateDecl(Decl *D,
+ SmallVectorImpl<serialization::DeclID> &UpdatedPartialSpecializations);
static void setNextObjCCategory(ObjCCategoryDecl *Cat,
ObjCCategoryDecl *Next) {
@@ -4194,7 +4195,7 @@ void ASTReader::loadDeclUpdateRecords(PendingUpdateRecord &Record) {
ProcessingUpdatesRAIIObj ProcessingUpdates(*this);
DeclUpdateOffsetsMap::iterator UpdI = DeclUpdateOffsets.find(ID);
- SmallVector<serialization::DeclID, 8> PendingLazySpecializationIDs;
+ SmallVector<serialization::DeclID, 8> PendingLazyPartialSpecializationIDs;
if (UpdI != DeclUpdateOffsets.end()) {
auto UpdateOffsets = std::move(UpdI->second);
@@ -4233,7 +4234,8 @@ void ASTReader::loadDeclUpdateRecords(PendingUpdateRecord &Record) {
ASTDeclReader Reader(*this, Record, RecordLocation(F, Offset), ID,
SourceLocation());
- Reader.UpdateDecl(D, PendingLazySpecializationIDs);
+ Reader.UpdateDecl(D,
+ PendingLazyPartialSpecializationIDs);
// We might have made this declaration interesting. If so, remember that
// we need to hand it off to the consumer.
@@ -4246,16 +4248,14 @@ void ASTReader::loadDeclUpdateRecords(PendingUpdateRecord &Record) {
}
}
// Add the lazy specializations to the template.
- assert((PendingLazySpecializationIDs.empty() || isa<ClassTemplateDecl>(D) ||
- isa<FunctionTemplateDecl, VarTemplateDecl>(D)) &&
+ assert((PendingLazyPartialSpecializationIDs.empty() ||
+ isa<ClassTemplateDecl, VarTemplateDecl>(D)) &&
"Must not have pending specializations");
if (auto *CTD = dyn_cast<ClassTemplateDecl>(D))
- ASTDeclReader::AddLazySpecializations(CTD, PendingLazySpecializationIDs);
- else if (auto *FTD = dyn_cast<FunctionTemplateDecl>(D))
- ASTDeclReader::AddLazySpecializations(FTD, PendingLazySpecializationIDs);
+ ASTDeclReader::AddLazySpecializations(CTD, PendingLazyPartialSpecializationIDs);
else if (auto *VTD = dyn_cast<VarTemplateDecl>(D))
- ASTDeclReader::AddLazySpecializations(VTD, PendingLazySpecializationIDs);
- PendingLazySpecializationIDs.clear();
+ ASTDeclReader::AddLazySpecializations(VTD, PendingLazyPartialSpecializationIDs);
+ PendingLazyPartialSpecializationIDs.clear();
// Load the pending visible updates for this decl context, if it has any.
auto I = PendingVisibleUpdates.find(ID);
@@ -4270,6 +4270,15 @@ void ASTReader::loadDeclUpdateRecords(PendingUpdateRecord &Record) {
reader::ASTDeclContextNameLookupTrait(*this, *Update.Mod));
DC->setHasExternalVisibleStorage(true);
}
+
+ // Load the pending specializations update for this decl, if it has any.
+ if (auto I = PendingSpecializationsUpdates.find(ID); I != PendingSpecializationsUpdates.end()) {
+ auto SpecializationUpdates = std::move(I->second);
+ PendingSpecializationsUpdates.erase(I);
+
+ for (const auto &Update : SpecializationUpdates)
+ AddSpecializations(D, Update.Data, *Update.Mod);
+ }
}
void ASTReader::loadPendingDeclChain(Decl *FirstLocal, uint64_t LocalOffset) {
@@ -4464,7 +4473,7 @@ static void forAllLaterRedecls(DeclT *D, Fn F) {
}
void ASTDeclReader::UpdateDecl(Decl *D,
- llvm::SmallVectorImpl<serialization::DeclID> &PendingLazySpecializationIDs) {
+ llvm::SmallVectorImpl<serialization::DeclID> &PendingLazyPartialSpecializationIDs) {
while (Record.getIdx() < Record.size()) {
switch ((DeclUpdateKind)Record.readInt()) {
case UPD_CXX_ADDED_IMPLICIT_MEMBER: {
@@ -4475,9 +4484,9 @@ void ASTDeclReader::UpdateDecl(Decl *D,
break;
}
- case UPD_CXX_ADDED_TEMPLATE_SPECIALIZATION:
- // It will be added to the template's lazy specialization set.
- PendingLazySpecializationIDs.push_back(readDeclID());
+ case UPD_CXX_ADDED_TEMPLATE_PARTIAL_SPECIALIZATION:
+ // It will be added to the template's lazy partial specialization set.
+ PendingLazyPartialSpecializationIDs.push_back(readDeclID());
break;
case UPD_CXX_ADDED_ANONYMOUS_NAMESPACE: {
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index ed58e71b01c09a..e3347455747ea6 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -4023,9 +4023,9 @@ unsigned CalculateODRHashForSpecs(const Decl *Spec) {
}
} // namespace
-uint64_t ASTWriter::WriteSpecializationsLookupTable(
- const NamedDecl *D,
- llvm::SmallVectorImpl<const NamedDecl *> &Specializations) {
+void ASTWriter::GenerateSpecializationsLookupTable(const NamedDecl *D,
+ llvm::SmallVectorImpl<const NamedDecl *> &Specializations,
+ llvm::SmallVectorImpl<char> &LookupTable) {
assert(D->isFirstDecl());
// Create the on-disk hash table representation.
@@ -4053,13 +4053,18 @@ uint64_t ASTWriter::WriteSpecializationsLookupTable(
for (auto Iter : SpecializationMaps)
Generator.insert(Iter.first, Trait.getData(Iter.second), Trait);
- uint64_t Offset = Stream.GetCurrentBitNo();
-
auto *Lookups =
Chain ? Chain->getLoadedSpecializationsLookupTables(D) : nullptr;
- llvm::SmallString<4096> LookupTable;
Generator.emit(LookupTable, Trait, Lookups ? &Lookups->Table : nullptr);
+}
+uint64_t ASTWriter::WriteSpecializationsLookupTable(
+ const NamedDecl *D, llvm::SmallVectorImpl<const NamedDecl *> &Specializations) {
+
+ llvm::SmallString<4096> LookupTable;
+ GenerateSpecializationsLookupTable(D, Specializations, LookupTable);
+
+ uint64_t Offset = Stream.GetCurrentBitNo();
RecordData::value_type Record[] = {DECL_SPECIALIZATIONS};
Stream.EmitRecordWithBlob(DeclSpecializationsAbbrev, Record, LookupTable);
@@ -5239,6 +5244,10 @@ ASTFileSignature ASTWriter::WriteASTCore(Sema &SemaRef, StringRef isysroot,
WriteTypeDeclOffsets();
if (!DeclUpdatesOffsetsRecord.empty())
Stream.EmitRecord(DECL_UPDATE_OFFSETS, DeclUpdatesOffsetsRecord);
+
+ if (!SpecializationsUpdates.empty())
+ WriteSpecializationsUpdates();
+
WriteFileDeclIDsMap();
WriteSourceManagerBlock(Context.getSourceManager(), PP);
WriteComments();
@@ -5391,6 +5400,25 @@ ASTFileSignature ASTWriter::WriteASTCore(Sema &SemaRef, StringRef isysroot,
return backpatchSignature();
}
+void ASTWriter::WriteSpecializationsUpdates() {
+ auto Abv = std::make_shared<llvm::BitCodeAbbrev>();
+ Abv->Add(llvm::BitCodeAbbrevOp(UPDATE_SPECIALIZATION));
+ Abv->Add(llvm::BitCodeAbbrevOp(llvm::BitCodeAbbrevOp::VBR, 6));
+ Abv->Add(llvm::BitCodeAbbrevOp(llvm::BitCodeAbbrevOp::Blob));
+ auto UpdateSpecializationAbbrev = Stream.EmitAbbrev(std::move(Abv));
+
+ for (auto &SpecializationUpdate : SpecializationsUpdates) {
+ const NamedDecl *D = SpecializationUpdate.first;
+
+ llvm::SmallString<4096> LookupTable;
+ GenerateSpecializationsLookupTable(D, SpecializationUpdate.second, LookupTable);
+
+ // Write the lookup table
+ RecordData::value_type Record[] = {UPDATE_SPECIALIZATION, getDeclID(D)};
+ Stream.EmitRecordWithBlob(UpdateSpecializationAbbrev, Record, LookupTable);
+ }
+}
+
void ASTWriter::WriteDeclUpdatesBlocks(RecordDataImpl &OffsetsRecord) {
if (DeclUpdates.empty())
return;
@@ -5419,7 +5447,7 @@ void ASTWriter::WriteDeclUpdatesBlocks(RecordDataImpl &OffsetsRecord) {
switch (Kind) {
case UPD_CXX_ADDED_IMPLICIT_MEMBER:
- case UPD_CXX_ADDED_TEMPLATE_SPECIALIZATION:
+ case UPD_CXX_ADDED_TEMPLATE_PARTIAL_SPECIALIZATION:
case UPD_CXX_ADDED_ANONYMOUS_NAMESPACE:
assert(Update.getDecl() && "no decl to add?");
Record.push_back(GetDeclRef(Update.getDecl()));
diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp
index db3800025f8d27..9a1f3a0df6fb82 100644
--- a/clang/lib/Serialization/ASTWriterDecl.cpp
+++ b/clang/lib/Serialization/ASTWriterDecl.cpp
@@ -271,8 +271,11 @@ namespace clang {
if (Writer.getFirstLocalDecl(Specialization) != Specialization)
return;
- Writer.DeclUpdates[Template].push_back(ASTWriter::DeclUpdate(
- UPD_CXX_ADDED_TEMPLATE_SPECIALIZATION, Specialization));
+ if (isa<ClassTemplatePartialSpecializationDecl, VarTemplatePartialSpecializationDecl>(Specialization))
+ Writer.DeclUpdates[Template].push_back(ASTWriter::DeclUpdate(
+ UPD_CXX_ADDED_TEMPLATE_PARTIAL_SPECIALIZATION, Specialization));
+ else
+ Writer.SpecializationsUpdates[cast<NamedDecl>(Template)].push_back(cast<NamedDecl>(Specialization));
}
};
}
diff --git a/clang/test/Modules/cxx-templates.cpp b/clang/test/Modules/cxx-templates.cpp
index b7d5741e69af61..2d285c10ceec59 100644
--- a/clang/test/Modules/cxx-templates.cpp
+++ b/clang/test/Modules/cxx-templates.cpp
@@ -251,7 +251,7 @@ namespace Std {
// CHECK-DUMP: ClassTemplateDecl {{.*}} <{{.*[/\\]}}cxx-templates-common.h:1:1, {{.*}}> col:{{.*}} in cxx_templates_common SomeTemplate
// CHECK-DUMP: ClassTemplateSpecializationDecl {{.*}} prev {{.*}} SomeTemplate
-// CHECK-DUMP-NEXT: TemplateArgument type 'char[2]'
+// CHECK-DUMP-NEXT: TemplateArgument type 'char[1]'
// CHECK-DUMP: ClassTemplateSpecializationDecl {{.*}} SomeTemplate definition
// CHECK-DUMP-NEXT: DefinitionData
// CHECK-DUMP-NEXT: DefaultConstructor
@@ -260,9 +260,9 @@ namespace Std {
// CHECK-DUMP-NEXT: CopyAssignment
// CHECK-DUMP-NEXT: MoveAssignment
// CHECK-DUMP-NEXT: Destructor
-// CHECK-DUMP-NEXT: TemplateArgument type 'char[2]'
-// CHECK-DUMP: ClassTemplateSpecializationDecl {{.*}} prev {{.*}} SomeTemplate
// CHECK-DUMP-NEXT: TemplateArgument type 'char[1]'
+// CHECK-DUMP: ClassTemplateSpecializationDecl {{.*}} prev {{.*}} SomeTemplate
+// CHECK-DUMP-NEXT: TemplateArgument type 'char[2]'
// CHECK-DUMP: ClassTemplateSpecializationDecl {{.*}} SomeTemplate definition
// CHECK-DUMP-NEXT: DefinitionData
// CHECK-DUMP-NEXT: DefaultConstructor
@@ -271,4 +271,5 @@ namespace Std {
// CHECK-DUMP-NEXT: CopyAssignment
// CHECK-DUMP-NEXT: MoveAssignment
// CHECK-DUMP-NEXT: Destructor
-// CHECK-DUMP-NEXT: TemplateArgument type 'char[1]'
+// CHECK-DUMP-NEXT: TemplateArgument type 'char[2]'
+
diff --git a/clang/unittests/Serialization/LoadSpecLazily.cpp b/clang/unittests/Serialization/LoadSpecLazily.cpp
index 03f3ff3f786555..5a174f2335a4b9 100644
--- a/clang/unittests/Serialization/LoadSpecLazily.cpp
+++ b/clang/unittests/Serialization/LoadSpecLazily.cpp
@@ -156,4 +156,38 @@ A<int> a;
"test.cpp"));
}
+TEST_F(LoadSpecLazilyTest, ChainedTest) {
+ GenerateModuleInterface("M", R"cpp(
+export module M;
+export template <class T>
+class A {};
+ )cpp");
+
+ GenerateModuleInterface("N", R"cpp(
+export module N;
+export import M;
+export class ShouldNotBeLoaded {};
+
+export class Temp {
+ A<ShouldNotBeLoaded> AS;
+};
+ )cpp");
+
+ const char *test_file_contents = R"cpp(
+import N;
+A<int> a;
+ )cpp";
+ std::string DepArg = "-fprebuilt-module-path=" + TestDir.str().str();
+ EXPECT_TRUE(runToolOnCodeWithArgs(
+ std::make_unique<CheckLoadSpecLazilyAction>("ShouldNotBeLoaded"),
+ test_file_contents,
+ {
+ "-std=c++20",
+ DepArg.c_str(),
+ "-I",
+ TestDir.c_str(),
+ },
+ "test.cpp"));
+}
+
} // namespace
``````````
</details>
https://github.com/llvm/llvm-project/pull/77417
More information about the llvm-branch-commits
mailing list