Introduce -fno-load-external-specializations-lazily

llvm · Jan 29, 2024 · 22c9d11 · 22c9d11
1 parent 8dde170
commit 22c9d11
Show file tree

Hide file tree

Showing 18 changed files with 200 additions and 68 deletions.
diff --git a/clang/include/clang/AST/ExternalASTSource.h b/clang/include/clang/AST/ExternalASTSource.h
@@ -156,6 +156,9 @@ class ExternalASTSource : public RefCountedBase<ExternalASTSource> {
   LoadExternalSpecializations(const Decl *D,
                               ArrayRef<TemplateArgument> TemplateArgs);
 
+  /// Load all the external specializations for the Decl D.
+  virtual void LoadAllExternalSpecializations(const Decl *D);
+
   /// Ensures that the table of all visible declarations inside this
   /// context is up to date.
   ///

diff --git a/clang/include/clang/Basic/LangOptions.def b/clang/include/clang/Basic/LangOptions.def
@@ -177,6 +177,7 @@ COMPATIBLE_LANGOPT(CPlusPlusModules, 1, 0, "C++ modules syntax")
 LANGOPT(BuiltinHeadersInSystemModules, 1, 0, "builtin headers belong to system modules, and _Builtin_ modules are ignored for cstdlib headers")
 BENIGN_ENUM_LANGOPT(CompilingModule, CompilingModuleKind, 3, CMK_None,
                     "compiling a module interface")
+COMPATIBLE_LANGOPT(LoadExternalSpecializationsLazily, 1, 0, "Load External Specializations Lazily")
 BENIGN_LANGOPT(CompilingPCH, 1, 0, "building a pch")
 BENIGN_LANGOPT(BuildingPCHWithObjectFile, 1, 0, "building a pch which has a corresponding object file")
 BENIGN_LANGOPT(CacheGeneratedPCH, 1, 0, "cache generated PCH files in memory")

diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
@@ -2978,6 +2978,13 @@ defm prebuilt_implicit_modules : BoolFOption<"prebuilt-implicit-modules",
   PosFlag<SetTrue, [], [ClangOption], "Look up implicit modules in the prebuilt module path">,
   NegFlag<SetFalse>, BothFlags<[], [ClangOption, CC1Option]>>;
 
+defm load_external_specializations_lazily : BoolOption<"f", "load-external-specializations-lazily",
+  LangOpts<"LoadExternalSpecializationsLazily">, DefaultFalse,
+  PosFlag<SetTrue, [], [ClangOption, CC1Option],
+          "Load required external specialization only when required.">,
+  NegFlag<SetFalse, [], []>>,
+  Group<f_clang_Group>;
+
 def fmodule_output_EQ : Joined<["-"], "fmodule-output=">,
   Flags<[NoXarchOption]>, Visibility<[ClangOption, CC1Option]>,
   HelpText<"Save intermediate module file results when compiling a standard C++ module unit.">;

diff --git a/clang/include/clang/Sema/MultiplexExternalSemaSource.h b/clang/include/clang/Sema/MultiplexExternalSemaSource.h
@@ -103,6 +103,8 @@ class MultiplexExternalSemaSource : public ExternalSemaSource {
   LoadExternalSpecializations(const Decl *D,
                               ArrayRef<TemplateArgument> TemplateArgs) override;
 
+  void LoadAllExternalSpecializations(const Decl *D) 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/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h
@@ -2012,6 +2012,8 @@ class ASTReader
   LoadExternalSpecializations(const Decl *D,
                               ArrayRef<TemplateArgument> TemplateArgs) override;
 
+  void LoadAllExternalSpecializations(const Decl *D) override;
+
   /// Read all of the declarations lexically stored in a
   /// declaration context.
   ///

diff --git a/clang/lib/AST/DeclTemplate.cpp b/clang/lib/AST/DeclTemplate.cpp
@@ -342,6 +342,13 @@ void RedeclarableTemplateDecl::loadExternalSpecializations() const {
     for (uint32_t I = 0, N = *Specs++; I != N; ++I)
       (void)Context.getExternalSource()->GetExternalDecl(Specs[I]);
   }
+
+  // We still load all the external specializations explicitly in the case
+  // the writer specified `-fload-external-specializations-lazily`. 
+  if (!getASTContext().getLangOpts().LoadExternalSpecializationsLazily &&
+      getASTContext().getExternalSource())
+    getASTContext().getExternalSource()->LoadAllExternalSpecializations(
+        this->getCanonicalDecl());
 }
 
 template <class EntryType, typename... ProfileArguments>
@@ -368,6 +375,12 @@ RedeclarableTemplateDecl::findSpecializationImpl(
           Specs, InsertPos, std::forward<ProfileArguments>(ProfileArgs)...))
     return Ret;
 
+  if (!getASTContext().getLangOpts().LoadExternalSpecializationsLazily) {
+    // If we didn't enable LoadExternalSpecializationsLazily, we shouldn't see
+    // anything from the external sources.
+    return nullptr;
+  }
+
   // If it is partial specialization, we are done.
   if constexpr (std::is_same_v<EntryType,
                                ClassTemplatePartialSpecializationDecl> ||
@@ -395,6 +408,9 @@ RedeclarableTemplateDecl::findSpecializationImpl(
 
 bool RedeclarableTemplateDecl::loadLazySpecializationsWithArgs(
     ArrayRef<TemplateArgument> TemplateArgs) {
+  if (!getASTContext().getLangOpts().LoadExternalSpecializationsLazily)
+    return false;
+
   auto *ExternalSource = getASTContext().getExternalSource();
   if (!ExternalSource)
     return false;

diff --git a/clang/lib/AST/ExternalASTSource.cpp b/clang/lib/AST/ExternalASTSource.cpp
@@ -105,6 +105,8 @@ bool ExternalASTSource::LoadExternalSpecializations(
   return false;
 }
 
+void ExternalASTSource::LoadAllExternalSpecializations(const Decl *D) {}
+
 void ExternalASTSource::completeVisibleDeclsMap(const DeclContext *DC) {}
 
 void ExternalASTSource::FindExternalLexicalDecls(

diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -3940,6 +3940,11 @@ static bool RenderModulesOptions(Compilation &C, const Driver &D,
     Args.ClaimAllArgs(options::OPT_fmodules_disable_diagnostic_validation);
   }
 
+  if (Args.hasFlag(options::OPT_fload_external_specializations_lazily,
+                   options::OPT_fno_load_external_specializations_lazily,
+                   false))
+    CmdArgs.push_back("-fload-external-specializations-lazily");
+
   // Claim `-fmodule-output` and `-fmodule-output=` to avoid unused warnings.
   Args.ClaimAllArgs(options::OPT_fmodule_output);
   Args.ClaimAllArgs(options::OPT_fmodule_output_EQ);

diff --git a/clang/lib/Sema/MultiplexExternalSemaSource.cpp b/clang/lib/Sema/MultiplexExternalSemaSource.cpp
@@ -123,6 +123,12 @@ bool MultiplexExternalSemaSource::LoadExternalSpecializations(
   return LoadedAnyDecls;
 }
 
+void MultiplexExternalSemaSource::LoadAllExternalSpecializations(
+    const Decl *D) {
+  for (size_t i = 0; i < Sources.size(); ++i)
+    Sources[i]->LoadAllExternalSpecializations(D);
+}
+
 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/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_PARTIAL_SPECIALIZATION,
+  UPD_CXX_ADDED_TEMPLATE_EXTERNAL_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
@@ -8100,11 +8100,10 @@ bool ASTReader::LoadExternalSpecializations(
   if (It == SpecializationsLookups.end())
     return false;
 
+  Deserializing LookupResults(this);
   auto HashValue = GetTemplateArgsStableHash(TemplateArgs);
 
   bool LoadedAnyDecls = false;
-
-  Deserializing LookupResults(this);
   for (DeclID ID : It->second.Table.find(HashValue)) {
     if (GetExistingDecl(ID))
       continue;
@@ -8115,6 +8114,23 @@ bool ASTReader::LoadExternalSpecializations(
   return LoadedAnyDecls;
 }
 
+void ASTReader::LoadAllExternalSpecializations(const Decl *D) {
+  assert(D);
+
+  auto It = SpecializationsLookups.find(D);
+  if (It == SpecializationsLookups.end())
+    return;
+
+  Deserializing LookupResults(this);
+
+  for (DeclID ID : It->second.Table.findAll())
+    GetDecl(ID);
+
+  // Since we've loaded all the specializations, we can erase it from
+  // the lookup table.
+  SpecializationsLookups.erase(It);
+}
+
 void ASTReader::completeVisibleDeclsMap(const DeclContext *DC) {
   if (!DC->hasExternalVisibleStorage())
     return;

diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -266,7 +266,7 @@ namespace clang {
           ThisDeclLoc(ThisDeclLoc) {}
 
     template <typename T> static
-    void AddLazySpecializations(T *D,
+    void AddExternalSpecializations(T *D,
                                 SmallVectorImpl<serialization::DeclID>& IDs) {
       if (IDs.empty())
         return;
@@ -2461,7 +2461,7 @@ void ASTDeclReader::VisitClassTemplateDecl(ClassTemplateDecl *D) {
     if (Record.readInt())
       ReadSpecializations(*Loc.F, D, Loc.F->DeclsCursor);
 
-    ASTDeclReader::AddLazySpecializations(D, SpecIDs);
+    ASTDeclReader::AddExternalSpecializations(D, SpecIDs);
   }
 
   if (D->getTemplatedDecl()->TemplateOrInstantiation) {
@@ -2493,7 +2493,7 @@ void ASTDeclReader::VisitVarTemplateDecl(VarTemplateDecl *D) {
     if (Record.readInt())
       ReadSpecializations(*Loc.F, D, Loc.F->DeclsCursor);
 
-    ASTDeclReader::AddLazySpecializations(D, SpecIDs);
+    ASTDeclReader::AddExternalSpecializations(D, SpecIDs);
   }
 }
 
@@ -2595,7 +2595,7 @@ void ASTDeclReader::VisitFunctionTemplateDecl(FunctionTemplateDecl *D) {
     // This FunctionTemplateDecl owns a CommonPtr; read it.
     SmallVector<serialization::DeclID, 32> SpecIDs;
     readDeclIDList(SpecIDs);
-    ASTDeclReader::AddLazySpecializations(D, SpecIDs);
+    ASTDeclReader::AddExternalSpecializations(D, SpecIDs);
 
     if (Record.readInt())
       ReadSpecializations(*Loc.F, D, Loc.F->DeclsCursor);
@@ -4217,7 +4217,7 @@ void ASTReader::loadDeclUpdateRecords(PendingUpdateRecord &Record) {
   ProcessingUpdatesRAIIObj ProcessingUpdates(*this);
   DeclUpdateOffsetsMap::iterator UpdI = DeclUpdateOffsets.find(ID);
 
-  SmallVector<serialization::DeclID, 8> PendingLazyPartialSpecializationIDs;
+  SmallVector<serialization::DeclID, 8> PendingExternalSpecializationIDs;
 
   if (UpdI != DeclUpdateOffsets.end()) {
     auto UpdateOffsets = std::move(UpdI->second);
@@ -4256,7 +4256,7 @@ void ASTReader::loadDeclUpdateRecords(PendingUpdateRecord &Record) {
 
       ASTDeclReader Reader(*this, Record, RecordLocation(F, Offset), ID,
                            SourceLocation());
-      Reader.UpdateDecl(D, PendingLazyPartialSpecializationIDs);
+      Reader.UpdateDecl(D, PendingExternalSpecializationIDs);
 
       // We might have made this declaration interesting. If so, remember that
       // we need to hand it off to the consumer.
@@ -4269,16 +4269,16 @@ void ASTReader::loadDeclUpdateRecords(PendingUpdateRecord &Record) {
     }
   }
   // Add the lazy specializations to the template.
-  assert((PendingLazyPartialSpecializationIDs.empty() ||
-          isa<ClassTemplateDecl, VarTemplateDecl>(D)) &&
+  assert((PendingExternalSpecializationIDs.empty() ||
+          isa<ClassTemplateDecl, VarTemplateDecl, FunctionTemplateDecl>(D)) &&
          "Must not have pending specializations");
   if (auto *CTD = dyn_cast<ClassTemplateDecl>(D))
-    ASTDeclReader::AddLazySpecializations(CTD,
-                                          PendingLazyPartialSpecializationIDs);
+    ASTDeclReader::AddExternalSpecializations(CTD, PendingExternalSpecializationIDs);
+  else if (auto *FTD = dyn_cast<FunctionTemplateDecl>(D))
+    ASTDeclReader::AddExternalSpecializations(FTD, PendingExternalSpecializationIDs);
   else if (auto *VTD = dyn_cast<VarTemplateDecl>(D))
-    ASTDeclReader::AddLazySpecializations(VTD,
-                                          PendingLazyPartialSpecializationIDs);
-  PendingLazyPartialSpecializationIDs.clear();
+    ASTDeclReader::AddExternalSpecializations(VTD, PendingExternalSpecializationIDs);
+  PendingExternalSpecializationIDs.clear();
 
   // Load the pending visible updates for this decl context, if it has any.
   auto I = PendingVisibleUpdates.find(ID);
@@ -4498,7 +4498,7 @@ static void forAllLaterRedecls(DeclT *D, Fn F) {
 
 void ASTDeclReader::UpdateDecl(Decl *D,
                                llvm::SmallVectorImpl<serialization::DeclID>
-                                   &PendingLazyPartialSpecializationIDs) {
+                                   &PendingExternalSpecializationIDs) {
   while (Record.getIdx() < Record.size()) {
     switch ((DeclUpdateKind)Record.readInt()) {
     case UPD_CXX_ADDED_IMPLICIT_MEMBER: {
@@ -4509,9 +4509,9 @@ void ASTDeclReader::UpdateDecl(Decl *D,
       break;
     }
 
-    case UPD_CXX_ADDED_TEMPLATE_PARTIAL_SPECIALIZATION:
+    case UPD_CXX_ADDED_TEMPLATE_EXTERNAL_SPECIALIZATION:
       // It will be added to the template's lazy partial specialization set.
-      PendingLazyPartialSpecializationIDs.push_back(readDeclID());
+      PendingExternalSpecializationIDs.push_back(readDeclID());
       break;
 
     case UPD_CXX_ADDED_ANONYMOUS_NAMESPACE: {

diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
@@ -4074,6 +4074,10 @@ uint64_t ASTWriter::WriteSpecializationsLookupTable(
     const NamedDecl *D,
     llvm::SmallVectorImpl<const NamedDecl *> &Specializations) {
 
+  assert(getLangOpts().LoadExternalSpecializationsLazily &&
+         "WriteSpecializationsLookupTable shouldn't be reachable unless"
+         "'-fload-external-specializations-lazily' is specified");
+
   llvm::SmallString<4096> LookupTable;
   GenerateSpecializationsLookupTable(D, Specializations, LookupTable);
 
@@ -5461,7 +5465,7 @@ void ASTWriter::WriteDeclUpdatesBlocks(RecordDataImpl &OffsetsRecord) {
 
       switch (Kind) {
       case UPD_CXX_ADDED_IMPLICIT_MEMBER:
-      case UPD_CXX_ADDED_TEMPLATE_PARTIAL_SPECIALIZATION:
+      case UPD_CXX_ADDED_TEMPLATE_EXTERNAL_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
@@ -221,9 +221,6 @@ namespace clang {
         assert(!Common->ExternalSpecializations);
       }
 
-      for (auto &Entry : Common->Specializations)
-        OptionalSpecs.push_back(getSpecializationDecl(Entry));
-
       ArrayRef<DeclID> ExternalSpecializations;
       if (auto *LS = Common->ExternalSpecializations)
         ExternalSpecializations = llvm::ArrayRef(LS + 1, LS[0]);
@@ -232,17 +229,23 @@ namespace clang {
       unsigned I = Record.size();
       Record.push_back(0);
 
+      llvm::SmallVector<const Decl *, 16> EagerSpecs;
+      for (auto &Entry : Common->Specializations)
+        if (Context.getLangOpts().LoadExternalSpecializationsLazily)
+          OptionalSpecs.push_back(getSpecializationDecl(Entry));
+        else
+          EagerSpecs.push_back(getSpecializationDecl(Entry));
+
       // AddFirstDeclFromEachModule might trigger deserialization, invalidating
       // *Specializations iterators.
       //
       // We need to load all the partial specializations at once if the template
       // required. Since we can't know if a partial specializations will be
       // needed before resolving a request to instantiate the template.
-      llvm::SmallVector<const Decl *, 16> PartialSpecs;
       for (auto &Entry : getPartialSpecializations(Common))
-        PartialSpecs.push_back(getSpecializationDecl(Entry));
+        EagerSpecs.push_back(getSpecializationDecl(Entry));
 
-      for (auto *D : PartialSpecs) {
+      for (auto *D : EagerSpecs) {
         assert(D->isCanonicalDecl() && "non-canonical decl in set");
         AddFirstDeclFromEachModule(D, /*IncludeLocal*/true);
       }
@@ -271,10 +274,11 @@ namespace clang {
       if (Writer.getFirstLocalDecl(Specialization) != Specialization)
         return;
 
-      if (isa<ClassTemplatePartialSpecializationDecl,
+      if (!Context.getLangOpts().LoadExternalSpecializationsLazily ||
+          isa<ClassTemplatePartialSpecializationDecl,
               VarTemplatePartialSpecializationDecl>(Specialization))
         Writer.DeclUpdates[Template].push_back(ASTWriter::DeclUpdate(
-            UPD_CXX_ADDED_TEMPLATE_PARTIAL_SPECIALIZATION, Specialization));
+            UPD_CXX_ADDED_TEMPLATE_EXTERNAL_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
@@ -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[1]'
+// CHECK-DUMP-NEXT:     TemplateArgument type 'char[2]'
 // 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[1]'
-// CHECK-DUMP:        ClassTemplateSpecializationDecl {{.*}} prev {{.*}} SomeTemplate
 // CHECK-DUMP-NEXT:     TemplateArgument type 'char[2]'
+// CHECK-DUMP:        ClassTemplateSpecializationDecl {{.*}} prev {{.*}} SomeTemplate
+// CHECK-DUMP-NEXT:     TemplateArgument type 'char[1]'
 // CHECK-DUMP:        ClassTemplateSpecializationDecl {{.*}} SomeTemplate definition
 // CHECK-DUMP-NEXT:     DefinitionData
 // CHECK-DUMP-NEXT:       DefaultConstructor
@@ -271,5 +271,4 @@ namespace Std {
 // CHECK-DUMP-NEXT:       CopyAssignment
 // CHECK-DUMP-NEXT:       MoveAssignment
 // CHECK-DUMP-NEXT:       Destructor
-// CHECK-DUMP-NEXT:     TemplateArgument type 'char[2]'
-
+// CHECK-DUMP-NEXT:     TemplateArgument type 'char[1]'
diff --git a/clang/test/Modules/odr_hash.cpp b/clang/test/Modules/odr_hash.cpp
@@ -3084,7 +3084,7 @@ struct S5 {
 };
 #else
 S5 s5;
-// expected-error@first.h:* {{'PointersAndReferences::S5::x' from module 'FirstModule' is not present in definition of 'PointersAndReferences::S5' in module 'SecondModule'}}
+// expected-error@second.h:* {{'PointersAndReferences::S5::x' from module 'SecondModule' is not present in definition of 'PointersAndReferences::S5' in module 'FirstModule'}}
 // expected-note@first.h:* {{declaration of 'x' does not match}}
 #endif
 
@@ -4021,7 +4021,7 @@ struct Valid {
 #else
 Invalid::L2<1>::L3<1> invalid;
 // expected-error@second.h:* {{'Types::InjectedClassName::Invalid::L2::L3::x' from module 'SecondModule' is not present in definition of 'L3<>' in module 'FirstModule'}}
-// expected-note@second.h:* {{declaration of 'x' does not match}}
+// expected-note@first.h:* {{declaration of 'x' does not match}}
 Valid::L2<1>::L3<1> valid;
 #endif
 }  // namespace InjectedClassName

diff --git a/clang/unittests/Serialization/CMakeLists.txt b/clang/unittests/Serialization/CMakeLists.txt
@@ -8,7 +8,7 @@ set(LLVM_LINK_COMPONENTS
 add_clang_unittest(SerializationTests
   ForceCheckFileInputTest.cpp
   InMemoryModuleCacheTest.cpp
-  LoadSpecLazily.cpp
+  LoadSpecLazilyTest.cpp
   ModuleCacheTest.cpp
   NoCommentsTest.cpp
   SourceLocationEncodingTest.cpp