Skip to content

[Serialization] Introduce OnDiskHashTable for specializations #83233

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

ChuanqiXu9
Copy link
Member

@ChuanqiXu9 ChuanqiXu9 commented Feb 28, 2024

Following up for #83108

This follows the suggestion literally from
#76774 (comment)

which introduces OnDiskHashTable for specializations based on D41416.

Note that I didn't polish this patch to reduce the diff from D41416 to it easier to review. I'll make the polishing patch later. So that we can focus what we're doing in this patch and focus on the style in the next patch.

Unfortunately it is still big though. But probably we can't make it shorter due to the complexity of OnDiskHashTable.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:modules C++20 modules and Clang Header Modules labels Feb 28, 2024
@llvmbot
Copy link
Member

llvmbot commented Feb 28, 2024

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-modules

Author: Chuanqi Xu (ChuanqiXu9)

Changes

Following up for #83108

This follows the suggestion literally from
#76774 (comment)

which introduces OnDiskHashTable for specializations based on D41416.

Note that I didn't polish this patch to reduce the diff from D41416 to it easier to review. I'll make the polishing patch later. So that we can focus what we're doing in this patch and focus on the style in the next patch.


Patch is 45.10 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/83233.diff

15 Files Affected:

  • (modified) clang/include/clang/AST/ExternalASTSource.h (+11)
  • (modified) clang/include/clang/Sema/MultiplexExternalSemaSource.h (+6)
  • (modified) clang/include/clang/Serialization/ASTBitCodes.h (+5)
  • (modified) clang/include/clang/Serialization/ASTReader.h (+31-3)
  • (modified) clang/include/clang/Serialization/ASTWriter.h (+14)
  • (modified) clang/lib/AST/DeclTemplate.cpp (+17)
  • (modified) clang/lib/AST/ExternalASTSource.cpp (+5)
  • (modified) clang/lib/Sema/MultiplexExternalSemaSource.cpp (+12)
  • (modified) clang/lib/Serialization/ASTReader.cpp (+144-2)
  • (modified) clang/lib/Serialization/ASTReaderDecl.cpp (+27)
  • (modified) clang/lib/Serialization/ASTReaderInternals.h (+121)
  • (modified) clang/lib/Serialization/ASTWriter.cpp (+171-1)
  • (modified) clang/lib/Serialization/ASTWriterDecl.cpp (+24-6)
  • (modified) clang/unittests/Serialization/CMakeLists.txt (+1)
  • (added) clang/unittests/Serialization/LoadSpecLazilyTest.cpp (+211)
diff --git a/clang/include/clang/AST/ExternalASTSource.h b/clang/include/clang/AST/ExternalASTSource.h
index 8e573965b0a336..af476aa8c57824 100644
--- a/clang/include/clang/AST/ExternalASTSource.h
+++ b/clang/include/clang/AST/ExternalASTSource.h
@@ -150,6 +150,17 @@ class ExternalASTSource : public RefCountedBase<ExternalASTSource> {
   virtual bool
   FindExternalVisibleDeclsByName(const DeclContext *DC, DeclarationName Name);
 
+  /// Load all the external specializations for the Decl \param D if \param
+  /// OnlyPartial is false. Otherwise, load all the external **partial**
+  /// specializations for the \param D.
+  virtual void LoadExternalSpecializations(const Decl *D, bool OnlyPartial);
+
+  /// Load all the specializations for the Decl \param D with the same template
+  /// args specified by \param TemplateArgs.
+  virtual void
+  LoadExternalSpecializations(const Decl *D,
+                              ArrayRef<TemplateArgument> TemplateArgs);
+
   /// Ensures that the table of all visible declarations inside this
   /// context is up to date.
   ///
diff --git a/clang/include/clang/Sema/MultiplexExternalSemaSource.h b/clang/include/clang/Sema/MultiplexExternalSemaSource.h
index 2bf91cb5212c5e..f09f037da0556a 100644
--- a/clang/include/clang/Sema/MultiplexExternalSemaSource.h
+++ b/clang/include/clang/Sema/MultiplexExternalSemaSource.h
@@ -97,6 +97,12 @@ class MultiplexExternalSemaSource : public ExternalSemaSource {
   bool FindExternalVisibleDeclsByName(const DeclContext *DC,
                                       DeclarationName Name) override;
 
+  void LoadExternalSpecializations(const Decl *D, bool OnlyPartial) override;
+
+  void
+  LoadExternalSpecializations(const Decl *D,
+                              ArrayRef<TemplateArgument> TemplateArgs) override;
+
   /// Ensures that the table of all visible declarations inside this
   /// context is up to date.
   void completeVisibleDeclsMap(const DeclContext *DC) override;
diff --git a/clang/include/clang/Serialization/ASTBitCodes.h b/clang/include/clang/Serialization/ASTBitCodes.h
index f31efa5117f0d1..15e7aef826a52a 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -698,6 +698,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.
@@ -1523,6 +1525,9 @@ enum DeclCode {
   /// A HLSLBufferDecl record.
   DECL_HLSL_BUFFER,
 
+  // A decls specilization record.
+  DECL_SPECIALIZATIONS,
+
   /// An ImplicitConceptSpecializationDecl record.
   DECL_IMPLICIT_CONCEPT_SPECIALIZATION,
 
diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h
index 2002bf23c9595f..f1edd2fae645d4 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -340,6 +340,9 @@ class ASTIdentifierLookupTrait;
 /// The on-disk hash table(s) used for DeclContext name lookup.
 struct DeclContextLookupTable;
 
+/// The on-disk hash table(s) used for specialization decls.
+struct LazySpecializationInfoLookupTable;
+
 } // namespace reader
 
 } // namespace serialization
@@ -603,21 +606,30 @@ class ASTReader
   llvm::DenseMap<const DeclContext *,
                  serialization::reader::DeclContextLookupTable> Lookups;
 
+  /// Map from decls to specialized decls.
+  llvm::DenseMap<const Decl *,
+                 serialization::reader::LazySpecializationInfoLookupTable>
+      SpecializationsLookups;
+
   // 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;
@@ -644,6 +656,11 @@ class ASTReader
                                      llvm::BitstreamCursor &Cursor,
                                      uint64_t Offset, serialization::DeclID ID);
 
+  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.
   ///
@@ -1348,6 +1365,11 @@ class ASTReader
   const serialization::reader::DeclContextLookupTable *
   getLoadedLookupTables(DeclContext *Primary) const;
 
+  /// Get the loaded specializations lookup tables for \p D,
+  /// if any.
+  serialization::reader::LazySpecializationInfoLookupTable *
+  getLoadedSpecializationsLookupTables(const Decl *D);
+
 private:
   struct ImportedModule {
     ModuleFile *Mod;
@@ -1982,6 +2004,12 @@ class ASTReader
                                       unsigned BlockID,
                                       uint64_t *StartOfBlockOffset = nullptr);
 
+  void LoadExternalSpecializations(const Decl *D, bool OnlyPartial) override;
+
+  void
+  LoadExternalSpecializations(const Decl *D,
+                              ArrayRef<TemplateArgument> TemplateArgs) override;
+
   /// Finds all the visible declarations with a given name.
   /// The current implementation of this method just loads the entire
   /// lookup table as unmaterialized references.
diff --git a/clang/include/clang/Serialization/ASTWriter.h b/clang/include/clang/Serialization/ASTWriter.h
index 5e2f305b294caf..d82b7b9fb123a8 100644
--- a/clang/include/clang/Serialization/ASTWriter.h
+++ b/clang/include/clang/Serialization/ASTWriter.h
@@ -385,6 +385,10 @@ class ASTWriter : public ASTDeserializationListener,
   /// record containing modifications to them.
   DeclUpdateMap DeclUpdates;
 
+  using SpecializationUpdateMap =
+      llvm::MapVector<const NamedDecl *, SmallVector<const Decl *>>;
+  SpecializationUpdateMap SpecializationsUpdates;
+
   using FirstLatestDeclMap = llvm::DenseMap<Decl *, Decl *>;
 
   /// Map of first declarations from a chained PCH that point to the
@@ -527,6 +531,13 @@ class ASTWriter : public ASTDeserializationListener,
   bool isLookupResultExternal(StoredDeclsList &Result, DeclContext *DC);
   bool isLookupResultEntirelyExternal(StoredDeclsList &Result, DeclContext *DC);
 
+  void GenerateSpecializationInfoLookupTable(
+      const NamedDecl *D, llvm::SmallVectorImpl<const Decl *> &Specializations,
+      llvm::SmallVectorImpl<char> &LookupTable);
+  uint64_t WriteSpecializationInfoLookupTable(
+      const NamedDecl *D,
+      llvm::SmallVectorImpl<const Decl *> &Specializations);
+
   void GenerateNameLookupTable(const DeclContext *DC,
                                llvm::SmallVectorImpl<char> &LookupTable);
   uint64_t WriteDeclContextLexicalBlock(ASTContext &Context, DeclContext *DC);
@@ -538,6 +549,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);
@@ -564,6 +576,8 @@ class ASTWriter : public ASTDeserializationListener,
   unsigned DeclEnumAbbrev = 0;
   unsigned DeclObjCIvarAbbrev = 0;
   unsigned DeclCXXMethodAbbrev = 0;
+  unsigned DeclSpecializationsAbbrev = 0;
+
   unsigned DeclDependentNonTemplateCXXMethodAbbrev = 0;
   unsigned DeclTemplateCXXMethodAbbrev = 0;
   unsigned DeclMemberSpecializedCXXMethodAbbrev = 0;
diff --git a/clang/lib/AST/DeclTemplate.cpp b/clang/lib/AST/DeclTemplate.cpp
index aa2368783df7b5..7b98b046d00725 100644
--- a/clang/lib/AST/DeclTemplate.cpp
+++ b/clang/lib/AST/DeclTemplate.cpp
@@ -335,6 +335,14 @@ RedeclarableTemplateDecl::CommonBase *RedeclarableTemplateDecl::getCommonPtr() c
 
 void RedeclarableTemplateDecl::loadLazySpecializationsImpl(
     bool OnlyPartial /*=false*/) const {
+  auto *ExternalSource = getASTContext().getExternalSource();
+  if (!ExternalSource)
+    return;
+
+  ExternalSource->LoadExternalSpecializations(this->getCanonicalDecl(),
+                                              OnlyPartial);
+  return;
+
   // Grab the most recent declaration to ensure we've loaded any lazy
   // redeclarations of this template.
   CommonBase *CommonBasePtr = getMostRecentDecl()->getCommonPtr();
@@ -353,6 +361,8 @@ void RedeclarableTemplateDecl::loadLazySpecializationsImpl(
 
 Decl *RedeclarableTemplateDecl::loadLazySpecializationImpl(
     LazySpecializationInfo &LazySpecInfo) const {
+  llvm_unreachable("We don't use LazySpecializationInfo any more");
+
   uint32_t ID = LazySpecInfo.DeclID;
   assert(ID && "Loading already loaded specialization!");
   // Note that we loaded the specialization.
@@ -362,6 +372,13 @@ Decl *RedeclarableTemplateDecl::loadLazySpecializationImpl(
 
 void RedeclarableTemplateDecl::loadLazySpecializationsImpl(
     ArrayRef<TemplateArgument> Args, TemplateParameterList *TPL) const {
+  auto *ExternalSource = getASTContext().getExternalSource();
+  if (!ExternalSource)
+    return;
+
+  ExternalSource->LoadExternalSpecializations(this->getCanonicalDecl(), Args);
+  return;
+
   CommonBase *CommonBasePtr = getMostRecentDecl()->getCommonPtr();
   if (auto *Specs = CommonBasePtr->LazySpecializations) {
     unsigned Hash = TemplateArgumentList::ComputeODRHash(Args);
diff --git a/clang/lib/AST/ExternalASTSource.cpp b/clang/lib/AST/ExternalASTSource.cpp
index 090ef02aa4224d..14a41651360b67 100644
--- a/clang/lib/AST/ExternalASTSource.cpp
+++ b/clang/lib/AST/ExternalASTSource.cpp
@@ -100,6 +100,11 @@ ExternalASTSource::FindExternalVisibleDeclsByName(const DeclContext *DC,
   return false;
 }
 
+void ExternalASTSource::LoadExternalSpecializations(const Decl *D, bool) {}
+
+void ExternalASTSource::LoadExternalSpecializations(
+    const Decl *D, ArrayRef<TemplateArgument>) {}
+
 void ExternalASTSource::completeVisibleDeclsMap(const DeclContext *DC) {}
 
 void ExternalASTSource::FindExternalLexicalDecls(
diff --git a/clang/lib/Sema/MultiplexExternalSemaSource.cpp b/clang/lib/Sema/MultiplexExternalSemaSource.cpp
index 058e22cb2b814e..397c2b215d2c6d 100644
--- a/clang/lib/Sema/MultiplexExternalSemaSource.cpp
+++ b/clang/lib/Sema/MultiplexExternalSemaSource.cpp
@@ -115,6 +115,18 @@ FindExternalVisibleDeclsByName(const DeclContext *DC, DeclarationName Name) {
   return AnyDeclsFound;
 }
 
+void MultiplexExternalSemaSource::LoadExternalSpecializations(
+    const Decl *D, bool OnlyPartial) {
+  for (size_t i = 0; i < Sources.size(); ++i)
+    Sources[i]->LoadExternalSpecializations(D, OnlyPartial);
+}
+
+void MultiplexExternalSemaSource::LoadExternalSpecializations(
+    const Decl *D, ArrayRef<TemplateArgument> TemplateArgs) {
+  for (size_t i = 0; i < Sources.size(); ++i)
+    Sources[i]->LoadExternalSpecializations(D, TemplateArgs);
+}
+
 void MultiplexExternalSemaSource::completeVisibleDeclsMap(const DeclContext *DC){
   for(size_t i = 0; i < Sources.size(); ++i)
     Sources[i]->completeVisibleDeclsMap(DC);
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index 3a4fcc8c118fee..0c6c19dfe5f908 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -1226,6 +1226,41 @@ void ASTDeclContextNameLookupTrait::ReadDataInto(internal_key_type,
   }
 }
 
+ModuleFile *LazySpecializationInfoLookupTrait::ReadFileRef(const unsigned char *&d) {
+  using namespace llvm::support;
+
+  uint32_t ModuleFileID =
+      endian::readNext<uint32_t, llvm::endianness::little, unaligned>(d);
+  return Reader.getLocalModuleFile(F, ModuleFileID);
+}
+
+LazySpecializationInfoLookupTrait::internal_key_type
+LazySpecializationInfoLookupTrait::ReadKey(const unsigned char *d, unsigned) {
+  using namespace llvm::support;
+  return endian::readNext<uint32_t, llvm::endianness::little, unaligned>(d);
+}
+
+std::pair<unsigned, unsigned>
+LazySpecializationInfoLookupTrait::ReadKeyDataLength(const unsigned char *&d) {
+  return readULEBKeyDataLength(d);
+}
+
+void LazySpecializationInfoLookupTrait::ReadDataInto(internal_key_type,
+                                              const unsigned char *d,
+                                              unsigned DataLen,
+                                              data_type_builder &Val) {
+  using namespace llvm::support;
+
+  for (unsigned NumDecls = DataLen / serialization::reader::LazySpecializationInfo::Length;
+       NumDecls; --NumDecls) {
+    uint32_t LocalID =
+        endian::readNext<uint32_t, llvm::endianness::little, unaligned>(d);
+    const bool IsPartial =
+        endian::readNext<bool, llvm::endianness::little, unaligned>(d);
+    Val.insert({Reader.getGlobalDeclID(F, LocalID), IsPartial});
+  }
+}
+
 bool ASTReader::ReadLexicalDeclContextStorage(ModuleFile &M,
                                               BitstreamCursor &Cursor,
                                               uint64_t Offset,
@@ -1311,7 +1346,49 @@ bool ASTReader::ReadVisibleDeclContextStorage(ModuleFile &M,
   // We can't safely determine the primary context yet, so delay attaching the
   // lookup table until we're done with recursive deserialization.
   auto *Data = (const unsigned char*)Blob.data();
-  PendingVisibleUpdates[ID].push_back(PendingVisibleUpdate{&M, Data});
+  PendingVisibleUpdates[ID].push_back(UpdateData{&M, Data});
+  return false;
+}
+
+void ASTReader::AddSpecializations(const Decl *D, const unsigned char *Data,
+                                   ModuleFile &M) {
+  D = D->getCanonicalDecl();
+  SpecializationsLookups[D].Table.add(
+      &M, Data, reader::LazySpecializationInfoLookupTrait(*this, M));
+}
+
+bool ASTReader::ReadSpecializations(ModuleFile &M, BitstreamCursor &Cursor,
+                                    uint64_t Offset, Decl *D) {
+  assert(Offset != 0);
+
+  SavedStreamPosition SavedPosition(Cursor);
+  if (llvm::Error Err = Cursor.JumpToBit(Offset)) {
+    Error(std::move(Err));
+    return true;
+  }
+
+  RecordData Record;
+  StringRef Blob;
+  Expected<unsigned> MaybeCode = Cursor.ReadCode();
+  if (!MaybeCode) {
+    Error(MaybeCode.takeError());
+    return true;
+  }
+  unsigned Code = MaybeCode.get();
+
+  Expected<unsigned> MaybeRecCode = Cursor.readRecord(Code, Record, &Blob);
+  if (!MaybeRecCode) {
+    Error(MaybeRecCode.takeError());
+    return true;
+  }
+  unsigned RecCode = MaybeRecCode.get();
+  if (RecCode != DECL_SPECIALIZATIONS) {
+    Error("Expected decl specs block");
+    return true;
+  }
+
+  auto *Data = (const unsigned char *)Blob.data();
+  AddSpecializations(D, Data, M);
   return false;
 }
 
@@ -3411,7 +3488,20 @@ llvm::Error ASTReader::ReadASTBlock(ModuleFile &F,
       unsigned Idx = 0;
       serialization::DeclID ID = ReadDeclID(F, Record, Idx);
       auto *Data = (const unsigned char*)Blob.data();
-      PendingVisibleUpdates[ID].push_back(PendingVisibleUpdate{&F, Data});
+      PendingVisibleUpdates[ID].push_back(UpdateData{&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 UPDATE_SPECIALIZATION: {
+      unsigned Idx = 0;
+      serialization::DeclID ID = ReadDeclID(F, Record, Idx);
+      auto *Data = (const unsigned char *)Blob.data();
+      PendingSpecializationsUpdates[ID].push_back(UpdateData{&F, Data});
       // If we've already loaded the decl, perform the updates when we finish
       // loading this block.
       if (Decl *D = GetExistingDecl(ID))
@@ -7849,6 +7939,51 @@ Stmt *ASTReader::GetExternalDeclStmt(uint64_t Offset) {
   return ReadStmtFromStream(*Loc.F);
 }
 
+void ASTReader::LoadExternalSpecializations(const Decl *D, bool OnlyPartial) {
+  assert(D);
+
+  auto It = SpecializationsLookups.find(D);
+  if (It == SpecializationsLookups.end())
+    return;
+
+  // Get Decl may violate the iterator from SpecializationsLookups
+  llvm::SmallVector<serialization::reader::LazySpecializationInfo, 8> Infos;
+  for (serialization::reader::LazySpecializationInfo Info :
+       It->second.Table.findAll())
+    Infos.push_back(Info);
+
+  Deserializing LookupResults(this);
+  for (auto &Info : Infos)
+    if (!OnlyPartial || Info.IsPartial)
+      GetDecl(Info.ID);
+
+  // Since we've loaded all the specializations, we can erase it from
+  // the lookup table.
+  if (!OnlyPartial)
+    SpecializationsLookups.erase(It);
+}
+
+void ASTReader::LoadExternalSpecializations(
+    const Decl *D, ArrayRef<TemplateArgument> TemplateArgs) {
+  assert(D);
+
+  auto It = SpecializationsLookups.find(D);
+  if (It == SpecializationsLookups.end())
+    return;
+
+  Deserializing LookupResults(this);
+  auto HashValue = TemplateArgumentList::ComputeODRHash(TemplateArgs);
+
+  // Get Decl may violate the iterator from SpecializationsLookups
+  llvm::SmallVector<serialization::reader::LazySpecializationInfo, 8> Infos;
+  for (serialization::reader::LazySpecializationInfo Info :
+       It->second.Table.find(HashValue))
+    Infos.push_back(Info);
+
+  for (auto &Info : Infos)
+    GetDecl(Info.ID);
+}
+
 void ASTReader::FindExternalLexicalDecls(
     const DeclContext *DC, llvm::function_ref<bool(Decl::Kind)> IsKindWeWant,
     SmallVectorImpl<Decl *> &Decls) {
@@ -8023,6 +8158,13 @@ ASTReader::getLoadedLookupTables(DeclContext *Primary) const {
   return I == Lookups.end() ? nullptr : &I->second;
 }
 
+serialization::reader::LazySpecializationInfoLookupTable *
+ASTReader::getLoadedSpecializationsLookupTables(const Decl *D) {
+  assert(D->isCanonicalDecl());
+  auto I = SpecializationsLookups.find(D);
+  return I == SpecializationsLookups.end() ? nullptr : &I->second;
+}
+
 /// Under non-PCH compilation the consumer receives the objc methods
 /// before receiving the implementation, and codegen depends on this.
 /// We simulate this by deserializing and passing to consumer the methods of the
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index 95942387a9e18e..0b42b4c2713162 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -434,6 +434,9 @@ namespace clang {
 
     std::pair<uint64_t, uint64_t> VisitDeclContext(DeclContext *DC);
 
+    void ReadSpecializations(ModuleFile &M, Decl *D,
+                             llvm::BitstreamCursor &Cursor);
+
     template<typename T>
     RedeclarableResult VisitRedeclarable(Redeclarable<T> *D);
 
@@ -2420,6 +2423,14 @@ void ASTDeclReader::VisitImplicitConceptSpecializationDecl(
 void ASTDeclReader::VisitRequiresExprBodyDecl(RequiresExprBodyDecl *D) {
 }
 
+void ASTDeclReader::ReadSpecializations(ModuleFile &M, Decl *D,
+                                        llvm::BitstreamCursor &DeclsCursor) {
+  uint64_t Offset = ReadLocalOffset();
+  bool Failed = Reader.ReadSpecializations(M, DeclsCursor, Offset, D);
+  (void)Failed;
+  assert(!Failed);
+}
+
 ASTDeclReader::RedeclarableResult
 ASTDeclReader::VisitRedeclarableTemplateDecl(RedeclarableTemplateDecl *D) {
   RedeclarableResult Redecl = VisitRedeclarable(D);
@@ -2461,6 +2472,7 @@ void ASTDeclReader::VisitClassTemplateDecl(ClassTemplateDecl *D) {
     SmallVector<LazySpecializationInfo, 32> SpecIDs...
[truncated]

@vgvassilev
Copy link
Contributor

Can you rebase on top of #83108 ? That'd make it easier for me to review.

@ChuanqiXu9
Copy link
Member Author

ChuanqiXu9 commented Feb 28, 2024

Can you rebase on top of #83108 ? That'd make it easier for me to review.

Weird. It should be on top of #83108 already. https://github.com/llvm/llvm-project/commits/users/ChuanqiXu9/D41416_on_disk_hash_table/

@vgvassilev
Copy link
Contributor

Can you rebase on top of #83108 ? That'd make it easier for me to review.

Weird. It should be on top of #83108 already. https://github.com/llvm/llvm-project/commits/users/ChuanqiXu9/D41416_on_disk_hash_table/

Ah, it is a single commit that includes what's in the other PR?

@ChuanqiXu9
Copy link
Member Author

Can you rebase on top of #83108 ? That'd make it easier for me to review.

Weird. It should be on top of #83108 already. https://github.com/llvm/llvm-project/commits/users/ChuanqiXu9/D41416_on_disk_hash_table/

Ah, it is a single commit that includes what's in the other PR?

Yes. This mimics the stacked review on phab. I assumed you're familiar with it.

@vgvassilev
Copy link
Contributor

Can you rebase on top of #83108 ? That'd make it easier for me to review.

Weird. It should be on top of #83108 already. https://github.com/llvm/llvm-project/commits/users/ChuanqiXu9/D41416_on_disk_hash_table/

Ah, it is a single commit that includes what's in the other PR?

Yes. This mimics the stacked review on phab. I assumed you're familiar with it.

No, I rarely did this on phab. I am more of a GitHub person :)

If that's ready to give it a try on our infrastructure I can do it.

@ChuanqiXu9
Copy link
Member Author

If that's ready to give it a try on our infrastructure I can do it.

I'll try to update the polishing patch. While it might purely be removing codes, I think it may be better to test on that.

@ChuanqiXu9 ChuanqiXu9 force-pushed the users/ChuanqiXu9/D41416_on_disk_hash_table branch from 9a88b07 to 03e7d56 Compare March 5, 2024 03:35
@ChuanqiXu9 ChuanqiXu9 force-pushed the users/ChuanqiXu9/D41416 branch from 244e745 to f44d7f8 Compare April 7, 2024 06:01
@ChuanqiXu9 ChuanqiXu9 force-pushed the users/ChuanqiXu9/D41416_on_disk_hash_table branch from 03e7d56 to f87a54e Compare April 7, 2024 06:02
@ChuanqiXu9 ChuanqiXu9 force-pushed the users/ChuanqiXu9/D41416 branch from f44d7f8 to 672fde1 Compare April 25, 2024 08:52
@ChuanqiXu9 ChuanqiXu9 force-pushed the users/ChuanqiXu9/D41416_on_disk_hash_table branch from f87a54e to 80c9ab1 Compare April 25, 2024 09:28
@ChuanqiXu9
Copy link
Member Author

Rebased with main

@ChuanqiXu9 ChuanqiXu9 force-pushed the users/ChuanqiXu9/D41416 branch from 672fde1 to a06599e Compare July 5, 2024 08:28
@ChuanqiXu9 ChuanqiXu9 force-pushed the users/ChuanqiXu9/D41416 branch from a06599e to 45ed7aa Compare July 10, 2024 09:42
@ChuanqiXu9 ChuanqiXu9 force-pushed the users/ChuanqiXu9/D41416_on_disk_hash_table branch from 3e84d3e to 2bf5a6c Compare July 10, 2024 09:43
@ChuanqiXu9 ChuanqiXu9 force-pushed the users/ChuanqiXu9/D41416 branch from 45ed7aa to a2cac54 Compare October 17, 2024 05:01
@ChuanqiXu9 ChuanqiXu9 force-pushed the users/ChuanqiXu9/D41416_on_disk_hash_table branch from 2bf5a6c to b61bcaa Compare October 17, 2024 05:02
Copy link

github-actions bot commented Oct 17, 2024

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff a5265801086897dfe0770c9552acd6d70b41bd92 11726437efb760c9f2aba9b2258337b2b8eb4bb6 --extensions cpp,h -- clang/unittests/Serialization/LoadSpecLazilyTest.cpp clang/include/clang/AST/ExternalASTSource.h clang/include/clang/Sema/MultiplexExternalSemaSource.h clang/include/clang/Serialization/ASTBitCodes.h clang/include/clang/Serialization/ASTReader.h clang/include/clang/Serialization/ASTWriter.h clang/lib/AST/DeclTemplate.cpp clang/lib/AST/ExternalASTSource.cpp clang/lib/Sema/MultiplexExternalSemaSource.cpp clang/lib/Serialization/ASTReader.cpp clang/lib/Serialization/ASTReaderDecl.cpp clang/lib/Serialization/ASTReaderInternals.h clang/lib/Serialization/ASTWriter.cpp clang/lib/Serialization/ASTWriterDecl.cpp
View the diff from clang-format here.
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index 75d81a25dd..1f25ed2428 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -1312,8 +1312,9 @@ void LazySpecializationInfoLookupTrait::ReadDataInto(internal_key_type,
   for (unsigned NumDecls =
            DataLen / serialization::reader::LazySpecializationInfo::Length;
        NumDecls; --NumDecls) {
-    LocalDeclID LocalID =
-        LocalDeclID::get(Reader, F, endian::readNext<DeclID, llvm::endianness::little, unaligned>(d));
+    LocalDeclID LocalID = LocalDeclID::get(
+        Reader, F,
+        endian::readNext<DeclID, llvm::endianness::little, unaligned>(d));
     const bool IsPartial =
         endian::readNext<bool, llvm::endianness::little, unaligned>(d);
     Val.insert({Reader.getGlobalDeclID(F, LocalID), IsPartial});
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index adfe9fc8b3..dbd1135cf2 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -4130,10 +4130,10 @@ public:
     for (auto *D : C) {
       bool IsPartial = isa<ClassTemplatePartialSpecializationDecl,
                            VarTemplatePartialSpecializationDecl>(D);
-      NamedDecl *ND = getDeclForLocalLookup(
-                           Writer.getLangOpts(), const_cast<NamedDecl *>(D));
-      Specs.push_back({GlobalDeclID(Writer.GetDeclRef(ND).getRawValue()),
-                       IsPartial});
+      NamedDecl *ND = getDeclForLocalLookup(Writer.getLangOpts(),
+                                            const_cast<NamedDecl *>(D));
+      Specs.push_back(
+          {GlobalDeclID(Writer.GetDeclRef(ND).getRawValue()), IsPartial});
     }
     return std::make_pair(Start, Specs.size());
   }
@@ -5799,7 +5799,7 @@ void ASTWriter::WriteDeclAndTypes(ASTContext &Context) {
   // Keep writing types, declarations, and declaration update records
   // until we've emitted all of them.
   RecordData DeclUpdatesOffsetsRecord;
-  Stream.EnterSubblock(DECLTYPES_BLOCK_ID, /*bits for abbreviations*/6);
+  Stream.EnterSubblock(DECLTYPES_BLOCK_ID, /*bits for abbreviations*/ 6);
   DeclTypesBlockStartOffset = Stream.GetCurrentBitNo();
   WriteTypeAbbrevs();
   WriteDeclAbbrevs();
@@ -5934,7 +5934,8 @@ void ASTWriter::WriteSpecializationsUpdates() {
                                           LookupTable);
 
     // Write the lookup table
-    RecordData::value_type Record[] = {UPDATE_SPECIALIZATION, getDeclID(D).getRawValue()};
+    RecordData::value_type Record[] = {UPDATE_SPECIALIZATION,
+                                       getDeclID(D).getRawValue()};
     Stream.EmitRecordWithBlob(UpdateSpecializationAbbrev, Record, LookupTable);
   }
 }

@ChuanqiXu9 ChuanqiXu9 force-pushed the users/ChuanqiXu9/D41416 branch from a2cac54 to 0d8de51 Compare November 8, 2024 08:52
@ChuanqiXu9 ChuanqiXu9 force-pushed the users/ChuanqiXu9/D41416_on_disk_hash_table branch from b61bcaa to f565dd3 Compare November 8, 2024 08:53
@ChuanqiXu9 ChuanqiXu9 force-pushed the users/ChuanqiXu9/D41416 branch from 0d8de51 to a526580 Compare November 8, 2024 09:07
@ChuanqiXu9 ChuanqiXu9 force-pushed the users/ChuanqiXu9/D41416_on_disk_hash_table branch from f565dd3 to 1172643 Compare November 8, 2024 09:20
@ChuanqiXu9
Copy link
Member Author

Sent b5bd192 and see #83237 (comment)

@ChuanqiXu9 ChuanqiXu9 closed this Dec 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:modules C++20 modules and Clang Header Modules clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants