[modules] [pch] Do not deserialize all lazy template specializations when looking for one.

[v.g.vassilev]

Summary

Currently, we load all lazy template specializations when we search whether there is a suitable template specialization for a template. This is especially suboptimal with modules. If module B specializes a template from module A the ASTReader would only read the specialization DeclID. This is observed to be especially pathological when module B is stl. However, the template instantiator (almost immediately after) will call findSpecialization. In turn, findSpecialization will load all lazy specializations to give an answer.

This patch teaches findSpecialization to work with lazy specializations without having to deserialize their full content. It provides along with the DeclID an cross-TU stable ODRHash of the template arguments which is enough to decide if we have already specialization and which are the exact ones (we load all decls with the same hash to avoid potential collisions) to deserialize.

While we make finding a template specialization more memory-efficient we are far from being done. There are still a few places which trigger eager deserialization of template specializations: the places where we require completion of the redeclaration chain.

[v.g.vassilev]

Fixed a comment typo.

[rsmith]

I think you should track lazy partial specializations separately from lazy full specializations -- we need to load all the partial specializations when doing partial specialization selection, and don't want to load all full specializations at the same time.

lib/Serialization/ASTReaderDecl.cpp

92 ↗

(On Diff #127606)

Move these subexpressions to separate statements; the two "read" calls are not sequenced here.

3972 ↗

(On Diff #127606)

Likewise here.

[v.g.vassilev]

Address inline comments: order the read operations.

[v.g.vassilev]

Teach ASTReader::CompleteRedeclChain to load only the template specializations with the same template argument list.

lib/Serialization/ASTReaderDecl.cpp

92 ↗

(On Diff #127606)

Good catch!

[rsmith]

This is looking like a really good start; we'll still want to do something about partial specializations, but this should already help a lot.

Can you produce some sample performance numbers for this change? This is replacing a linear-time algorithm (with a high constant) with a quadratic-time algorithm (with a lower constant), and it'd be useful to confirm that's not going to be a pressing problem in the short term. (In the medium term, we should move to using an on-disk hash table for template specialization lookup, at least for templates with large numbers of specializations.)

include/clang/AST/DeclTemplate.h

762–765 ↗

(On Diff #128465)

No space between operator and (, please.

lib/AST/DeclTemplate.cpp

209 ↗

(On Diff #128465)

Can we remove the loaded specializations from the list / zero them out, so know we've already asked the ExternalASTSource for them?

235–242 ↗

(On Diff #128465)

I'm a little concerned about this: a debug build could perform more deserialization than an optimized build here. It's probably not a big deal, but it could lead to some surprising debugging experiences down the line (where a debug build works but an optimized build misbehaves).

Perhaps we could assert that there are no lazy specializations with the relevant hash in our list before performing this lookup? (This'd require that we remove them as we load them.)

lib/Serialization/ASTReader.cpp

7009–7010 ↗

(On Diff #128465)

} and else on same line, please.

[v.g.vassilev]

Address comments:

• Fix style;
• Do not potentially deserialize a specialization in debug mode.

[v.g.vassilev]

Loading of lazy partial template specializations should not trigger loading of full template specializations.

Often during selection process we need to load all partial template specializations. It is very rare to do so for full template specialization.

[v.g.vassilev]

I have created a simple (-ish) benchmark targeted to stress test modules with a good number of template specializations. I use the mp11 library from boost. I am building two modules which contain around 1K specializations (mostly full specializations). The example can be found here. I use a small fraction of them. This patch deserializes (only) 1117 specializations for around 0.2 seconds whereas without it clang deserializes 1905 specializations for 0.4 s.

This I believe can be further improved and I am investigating but that should not be a blocker for this patch.

[v.g.vassilev]

Zero IsPartial and improve comments in assert and free text.

[v.g.vassilev]

Reverse template specialization order.

[v.g.vassilev]

Reduce hash collisions for template specializations.

Currently when we hash a tag type the visitor calls ODRHash::AddDecl which mostly relies on the decl name give distinct hash value. The types coming from template specializations have very similar properties (including decl names). For those we need to provide more information in order to disambiguate them.

This patch adds the template arguments for the template specialization decl corresponding to its type. We manage to reduce further the amount of deserializations from 1117 down to 451.

[v.g.vassilev]

Reduce further the deserializations from 451 to 449 by providing a more complete implementation of ODRHash::AddTemplateArgument.

Kudos @rtrieu!

[v.g.vassilev]

Finer grained clang stats can be seen here.

[rsmith]

I just tried this on one of our larger builds. The good news is that it appears to work: the compilation still succeeds, and significantly fewer nodes are deserialized. The bad news is that the final .cc file compilation got slower, from 42s to 86s. (There's a lot of noise here, and the builds might have been on vastly different machines, so take this with a pinch of salt for now.) Sample stats:

• "types read" is down from 30% to 17%
• "declarations read" is down from 34% to 23%
• number of ClassTemplateSpecializations read has decreased by 30%, number of CXXRecordDecls read is down 25%
• total ASTContext memory usage is down by 12%

[v.g.vassilev]

Add sanity check.

We assert that the ODR hash of the template arguments should be the same as the one for from the actually found template specialization.

This way we managed to catch a few collisions in the ODRHash logic.

[v.g.vassilev]

Rebase.

[ChuanqiXu]

Add me as a reviewer to add this to my stack.

[v.g.vassilev]

clang/lib/Serialization/ASTWriterDecl.cpp

288

We should not store the lazy specialization information as an array of items because that makes the search slow. Instead we should use the OnDiskHashTable approach which we use already to store the identifier data.

[v.g.vassilev]

Rebase.

[ChuanqiXu]

clang/include/clang/AST/DeclTemplate.h

786–788

Maybe this can save some space.

clang/lib/AST/DeclTemplate.cpp

366

TPL here looks not used.

clang/lib/Serialization/ASTWriterDecl.cpp

288

Do you want to implement it in this patch? Or this is a note for future optimizations?

[ChuanqiXu]

I tested this with our internal use cases with modules and it looks good. Things get compiled correctly and we saved 10% compilation time. (May be due to different code bases). But some local in tree tests got failed. I took some time to investigate them but I didn't get insights : (

[Hahnfeld]

In D41416#4487516, @ChuanqiXu wrote:

But some local in tree tests got failed. I took some time to investigate them but I didn't get insights : (

If the failures involve template template parameters, please note that you need https://reviews.llvm.org/D153003 for that to properly hash and find in the list of lazy specializations.

clang/lib/AST/DeclTemplate.cpp

366

This is required because of the argument forwarding from findSpecializationImpl below...

[SAtacker]

clang/lib/Serialization/ASTWriterDecl.cpp

288

I tried it here https://reviews.llvm.org/D144831 but to my surprise it gives worse performance for my testing environment which generates random specializations per module and a single main file.

[shafik]

clang/lib/AST/DeclTemplate.cpp

337

/*=false*/ is this left over code that was meant to be removed?

559

nit

1292

Was /*=false*/ meant to be removed?

1304

[Hahnfeld]

clang/include/clang/AST/DeclTemplate.h

786–788

Can you elaborate why this would save space? The best practice is to order data members by size to avoid padding gaps. As far as I can see, this is already done here.

[ChuanqiXu]

clang/include/clang/AST/DeclTemplate.h

786–788

Oh, sorry. This was an oversight.

[ChuanqiXu]

In D41416#4488517, @Hahnfeld wrote:

In D41416#4487516, @ChuanqiXu wrote:

But some local in tree tests got failed. I took some time to investigate them but I didn't get insights : (

If the failures involve template template parameters, please note that you need https://reviews.llvm.org/D153003 for that to properly hash and find in the list of lazy specializations.

Got it. This is on my TODO list. (Feel free to take it if you're hurry. Just tell me when you started. So that we can avoid reinventing the same wheel)

[v.g.vassilev]

clang/lib/Serialization/ASTWriterDecl.cpp

288

Do you want to implement it in this patch? Or this is a note for future optimizations?

That was the intent since it was blocking the google folks. If that's not the case then maybe we can do it as a separate improvement...

[v.g.vassilev]

Address most of the comments. I will need some help with the Modules/pr60085.cppm failure. I suspect we pass to CodeGen some instantiation by iterator index and that does not work as the instantiations are lazily triggered upon use now.

clang/lib/AST/DeclTemplate.cpp

337

That was not my intent. I added /*=false*/ to keep track of the the actual default argument.

However, you are right, we are not using it without passing the correct argument. I will drop the default arg.

366

Are you suggesting to make this replacement: TPL -> /*TPL*/?

[v.g.vassilev]

clang/test/Modules/odr_hash.cpp

2905

I realize that change is probably incorrect since we want to show both places where things differ.

[ChuanqiXu]

I will need some help with the Modules/pr60085.cppm failure.

I am looking at https://reviews.llvm.org/D154914 so maybe I can't look this deeply now. If there are concrete and small issues, maybe I can try to take a look. Hope this helps.

clang/lib/AST/DeclTemplate.cpp

366

I am not sure. I'm just wondering if there will be a static checker emits warnings for unused arguments.

[v.g.vassilev]

Fix a compilation error.

[v.g.vassilev]

Fix the odr_hash.cpp failure.

[v.g.vassilev]

In D41416#4492367, @v.g.vassilev wrote:

Address most of the comments. I will need some help with the Modules/pr60085.cppm failure. I suspect we pass to CodeGen some instantiation by iterator index and that does not work as the instantiations are lazily triggered upon use now.

In fact it looks like it fails exactly in the same way as described in the original report https://github.com/llvm/llvm-project/issues/60085. The commit message in https://github.com/llvm/llvm-project/commit/78e48977a6e67 hints at the fact that the issue was gone surprisingly. I suspect that test case stopped reproducing the underlying issue by chance. That probably means that this patch is not breaking anything but exposing an underlying problem...

[ChuanqiXu]

In D41416#4496293, @v.g.vassilev wrote:

In D41416#4492367, @v.g.vassilev wrote:

Address most of the comments. I will need some help with the Modules/pr60085.cppm failure. I suspect we pass to CodeGen some instantiation by iterator index and that does not work as the instantiations are lazily triggered upon use now.

In fact it looks like it fails exactly in the same way as described in the original report https://github.com/llvm/llvm-project/issues/60085. The commit message in https://github.com/llvm/llvm-project/commit/78e48977a6e67 hints at the fact that the issue was gone surprisingly. I suspect that test case stopped reproducing the underlying issue by chance. That probably means that this patch is not breaking anything but exposing an underlying problem...

Got it. I understand it is frustrating to fix an additional bug during development. Also it is generally not good to revert a valid test case.

Are you in a hurry to land this recently? (e.g., land this for clang17) If not, I suggest to wait for me to fix the underlying issue after clang17 gets released. And if I take too long to fix it (e.g, 9.30), given the speciality of the patch, I think we can revert that test case and land this one that time. How do you feel about the idea?

[v.g.vassilev]

In D41416#4496389, @ChuanqiXu wrote:

In D41416#4496293, @v.g.vassilev wrote:

In D41416#4492367, @v.g.vassilev wrote:

Address most of the comments. I will need some help with the Modules/pr60085.cppm failure. I suspect we pass to CodeGen some instantiation by iterator index and that does not work as the instantiations are lazily triggered upon use now.

In fact it looks like it fails exactly in the same way as described in the original report https://github.com/llvm/llvm-project/issues/60085. The commit message in https://github.com/llvm/llvm-project/commit/78e48977a6e67 hints at the fact that the issue was gone surprisingly. I suspect that test case stopped reproducing the underlying issue by chance. That probably means that this patch is not breaking anything but exposing an underlying problem...

Got it. I understand it is frustrating to fix an additional bug during development. Also it is generally not good to revert a valid test case.

I agree. Here is a reduced version of the same test case:

// RUN: rm -rf %t
// RUN: mkdir %t
// RUN: split-file %s %t
//
// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/d.cppm \
// RUN:     -emit-module-interface -o %t/d.pcm
// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/c.cppm \
// RUN:     -emit-module-interface -o %t/c.pcm -fmodule-file=%t/d.pcm
// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/b.cppm \
// RUN:     -emit-module-interface -o %t/b.pcm -fmodule-file=%t/d.pcm
// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/a.cppm \
// RUN:     -emit-module-interface -o %t/a.pcm -fmodule-file=%t/d.pcm \
// RUN:     -fmodule-file=%t/c.pcm -fmodule-file=%t/b.pcm 
// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/a.pcm \
// RUN:     -S -emit-llvm -disable-llvm-passes -o - | FileCheck %t/a.cppm
//
// Use -fmodule-file=<module-name>=<BMI-path>
// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/d.cppm \
// RUN:     -emit-module-interface -o %t/d.pcm
// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/c.cppm \
// RUN:     -emit-module-interface -o %t/c.pcm -fmodule-file=%t/d.pcm
// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/b.cppm \
// RUN:     -emit-module-interface -o %t/b.pcm -fmodule-file=%t/d.pcm
// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/a.cppm \
// RUN:     -emit-module-interface -o %t/a.pcm -fmodule-file=%t/d.pcm \
// RUN:     -fmodule-file=%t/c.pcm -fmodule-file=%t/b.pcm 
// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/a.pcm \
// RUN:     -S -emit-llvm -disable-llvm-passes -o - | FileCheck %t/a.cppm

//--- d.cppm
export module d;

export template<typename>
struct integer {
  using type = int;

  static constexpr auto value() {
    return 0;
  }

};

export constexpr void ddd(auto const v) {
  v.value();
}

//--- c.cppm
export module c;

import d;

int use = integer<int>().value();

//--- b.cppm
export module b;

import d;

using use = integer<int>::type;

//--- a.cppm
export module a;

import d;

export void a() {
  ddd(integer<int>());
}

Are you in a hurry to land this recently? (e.g., land this for clang17) If not, I suggest to wait for me to fix the underlying issue after clang17 gets released. And if I take too long to fix it (e.g, 9.30), given the speciality of the patch, I think we can revert that test case and land this one that time. How do you feel about the idea?

We are not in hurry - this patch was developed in 2017 :) It can probably wait a few more years ;) I had some spare cycles to work on it. That's all.

I do not think we can land it without asking @rsmith or @dblaikie to test it on their internal builds to make sure things are not regressing performance...

[v.g.vassilev]

@alexfh, could you test that patch on your internal infrastructure and check if it actually decreases the memory footprint and/or compile times? I believe this was the only blocker to land it as it is.

clang/include/clang/AST/DeclTemplate.h

public:
   /// stack object. It does not own its template arguments.
   enum OnStackType { OnStack };
 
   /// Create a new template argument list that copies the given set of
   /// template arguments.
   static TemplateArgumentList *CreateCopy(ASTContext &Context,
                                           ArrayRef<TemplateArgument> Args);
 
+  /// \brief Create hash for the given arguments.
+  static unsigned ComputeODRHash(ArrayRef<TemplateArgument> Args);
+
   /// Construct a new, temporary template argument list on the stack.
   ///
   /// The template argument list does not own the template arguments
   /// provided.
   explicit TemplateArgumentList(OnStackType, ArrayRef<TemplateArgument> Args)
       : Arguments(Args.data()), NumArguments(Args.size()) {}
 
   /// Produces a shallow copy of the given template argument list.
RedeclarableTemplateDecl *getPreviousDeclImpl() override {
     return getPreviousDecl();
   }
 
   RedeclarableTemplateDecl *getMostRecentDeclImpl() override {
     return getMostRecentDecl();
   }
 
   void anchor() override;
+  struct LazySpecializationInfo {
+    uint32_t DeclID = ~0U;
+    unsigned ODRHash = ~0U;
+    bool IsPartial = false;
+    LazySpecializationInfo(uint32_t ID, unsigned Hash = ~0U,
+                           bool Partial = false)
+        : DeclID(ID), ODRHash(Hash), IsPartial(Partial) {}
+    LazySpecializationInfo() {}
+    bool operator<(const LazySpecializationInfo &Other) const {
+      return DeclID < Other.DeclID;
+    }
+    bool operator==(const LazySpecializationInfo &Other) const {
+      assert((DeclID != Other.DeclID || ODRHash == Other.ODRHash) &&
+             "Hashes differ!");
+      assert((DeclID != Other.DeclID || IsPartial == Other.IsPartial) &&
+             "Both must be the same kinds!");
+      return DeclID == Other.DeclID;
+    }
+  };
+
 protected:
   template <typename EntryType> struct SpecEntryTraits {
     using DeclType = EntryType;
 
     static DeclType *getDecl(EntryType *D) {
       return D;
     }
 
protected:
   };
 
   template <typename EntryType>
   static SpecIterator<EntryType>
   makeSpecIterator(llvm::FoldingSetVector<EntryType> &Specs, bool isEnd) {
     return SpecIterator<EntryType>(isEnd ? Specs.end() : Specs.begin());
   }
 
-  void loadLazySpecializationsImpl() const;
+  void loadLazySpecializationsImpl(bool OnlyPartial) const;
+
+  void loadLazySpecializationsImpl(llvm::ArrayRef<TemplateArgument> Args,
+                                   TemplateParameterList *TPL = nullptr) const;
+
+  Decl *loadLazySpecializationImpl(LazySpecializationInfo &LazySpecInfo) const;
 
   template <class EntryType, typename ...ProfileArguments>
   typename SpecEntryTraits<EntryType>::DeclType*
   findSpecializationImpl(llvm::FoldingSetVector<EntryType> &Specs,
                          void *&InsertPos, ProfileArguments &&...ProfileArgs);
 
   template <class Derived, class EntryType>
   void addSpecializationImpl(llvm::FoldingSetVector<EntryType> &Specs,
struct CommonBase {
     llvm::PointerIntPair<RedeclarableTemplateDecl*, 1, bool>
       InstantiatedFromMember;
 
     /// If non-null, points to an array of specializations (including
     /// partial specializations) known only by their external declaration IDs.
     ///
     /// The first value in the array is the number of specializations/partial
     /// specializations that follow.
-    uint32_t *LazySpecializations = nullptr;
+    LazySpecializationInfo *LazySpecializations = nullptr;
 
     /// The set of "injected" template arguments used within this
     /// template.
     ///
     /// This pointer refers to the template arguments (there are as
     /// many template arguments as template parameters) for the
     /// template, and is allocated lazily, since most templates do not
     /// require the use of this information.

 
 public:
 
   friend class ASTDeclReader;
   friend class ASTDeclWriter;
   friend class TemplateDeclInstantiator;
 
   /// Load any lazily-loaded specializations from the external source.
-  void LoadLazySpecializations() const;
+  void LoadLazySpecializations(bool OnlyPartial = false) const;
 
   /// Get the underlying class declarations of the template.
   CXXRecordDecl *getTemplatedDecl() const {
     return static_cast<CXXRecordDecl *>(TemplatedDecl);
   }
 
   /// Returns whether this template declaration defines the primary
   /// class pattern.
Common *getCommonPtr() const {
     return static_cast<Common *>(RedeclarableTemplateDecl::getCommonPtr());
   }
 
 public:
   friend class ASTDeclReader;
   friend class ASTDeclWriter;
 
   /// Load any lazily-loaded specializations from the external source.
-  void LoadLazySpecializations() const;
+  void LoadLazySpecializations(bool OnlyPartial = false) const;
 
   /// Get the underlying variable declarations of the template.
   VarDecl *getTemplatedDecl() const {
     return static_cast<VarDecl *>(TemplatedDecl);
   }
 
   /// Returns whether this template declaration defines the primary
   /// variable pattern.

clang/lib/AST/DeclTemplate.cpp

 //===----------------------------------------------------------------------===//
 
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTMutationListener.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclarationName.h"
 #include "clang/AST/Expr.h"
+#include "clang/AST/ExprCXX.h"
 #include "clang/AST/ExternalASTSource.h"
+#include "clang/AST/ODRHash.h"
 #include "clang/AST/TemplateBase.h"
 #include "clang/AST/TemplateName.h"
 #include "clang/AST/Type.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/SourceLocation.h"
 #include "llvm/ADT/ArrayRef.h"
RedeclarableTemplateDecl::CommonBase *RedeclarableTemplateDecl::getCommonPtr() const {
 
   // Update any previous declarations we saw with the common pointer.
   for (const RedeclarableTemplateDecl *Prev : PrevDecls)
     Prev->Common = Common;
 
   return Common;
 }
 
-void RedeclarableTemplateDecl::loadLazySpecializationsImpl() const {
+void RedeclarableTemplateDecl::loadLazySpecializationsImpl(
+    bool OnlyPartial) const {
   // Grab the most recent declaration to ensure we've loaded any lazy
   // redeclarations of this template.
   CommonBase *CommonBasePtr = getMostRecentDecl()->getCommonPtr();
-  if (CommonBasePtr->LazySpecializations) {
-    ASTContext &Context = getASTContext();
-    uint32_t *Specs = CommonBasePtr->LazySpecializations;
+  if (auto *Specs = CommonBasePtr->LazySpecializations) {
+    if (!OnlyPartial)
       CommonBasePtr->LazySpecializations = nullptr;
-    for (uint32_t I = 0, N = *Specs++; I != N; ++I)
-      (void)Context.getExternalSource()->GetExternalDecl(Specs[I]);
+    for (uint32_t I = 0, N = Specs[0].DeclID; I != N; ++I) {
+      // Skip over already loaded specializations.
+      if (!Specs[I + 1].ODRHash)
+        continue;
+      if (!OnlyPartial || Specs[I + 1].IsPartial)
+        (void)loadLazySpecializationImpl(Specs[I + 1]);
+    }
+  }
+}
+
+Decl *RedeclarableTemplateDecl::loadLazySpecializationImpl(
+    LazySpecializationInfo &LazySpecInfo) const {
+  uint32_t ID = LazySpecInfo.DeclID;
+  assert(ID && "Loading already loaded specialization!");
+  // Note that we loaded the specialization.
+  LazySpecInfo.DeclID = LazySpecInfo.ODRHash = LazySpecInfo.IsPartial = 0;
+  return getASTContext().getExternalSource()->GetExternalDecl(ID);
+}
+
+void RedeclarableTemplateDecl::loadLazySpecializationsImpl(
+    ArrayRef<TemplateArgument> Args, TemplateParameterList *TPL) const {
+  CommonBase *CommonBasePtr = getMostRecentDecl()->getCommonPtr();
+  if (auto *Specs = CommonBasePtr->LazySpecializations) {
+    unsigned Hash = TemplateArgumentList::ComputeODRHash(Args);
+    for (uint32_t I = 0, N = Specs[0].DeclID; I != N; ++I)
+      if (Specs[I + 1].ODRHash && Specs[I + 1].ODRHash == Hash)
+        (void)loadLazySpecializationImpl(Specs[I + 1]);
   }
 }
 
 template<class EntryType, typename... ProfileArguments>
 typename RedeclarableTemplateDecl::SpecEntryTraits<EntryType>::DeclType *
 RedeclarableTemplateDecl::findSpecializationImpl(
     llvm::FoldingSetVector<EntryType> &Specs, void *&InsertPos,
     ProfileArguments&&... ProfileArgs) {
   using SETraits = SpecEntryTraits<EntryType>;
 
+  loadLazySpecializationsImpl(std::forward<ProfileArguments>(ProfileArgs)...);
+
   llvm::FoldingSetNodeID ID;
   EntryType::Profile(ID, std::forward<ProfileArguments>(ProfileArgs)...,
                      getASTContext());
   EntryType *Entry = Specs.FindNodeOrInsertPos(ID, InsertPos);
   return Entry ? SETraits::getDecl(Entry)->getMostRecentDecl() : nullptr;
 }
 
 template<class Derived, class EntryType>
 void RedeclarableTemplateDecl::addSpecializationImpl(
     llvm::FoldingSetVector<EntryType> &Specializations, EntryType *Entry,
     void *InsertPos) {
   using SETraits = SpecEntryTraits<EntryType>;
 
   if (InsertPos) {
 #ifndef NDEBUG
+    auto Args = SETraits::getTemplateArgs(Entry);
+    // Due to hash collisions, it can happen that we load another template
+    // specialization with the same hash. This is fine, as long as the next
+    // call to findSpecializationImpl does not find a matching Decl for the
+    // template arguments.
+    loadLazySpecializationsImpl(Args);
     void *CorrectInsertPos;
-    assert(!findSpecializationImpl(Specializations,
-                                   CorrectInsertPos,
-                                   SETraits::getTemplateArgs(Entry)) &&
+    assert(!findSpecializationImpl(Specializations, CorrectInsertPos, Args) &&
            InsertPos == CorrectInsertPos &&
            "given incorrect InsertPos for specialization");
 #endif
     Specializations.InsertNode(Entry, InsertPos);
   } else {
     EntryType *Existing = Specializations.GetOrInsertNode(Entry);
     (void)Existing;
     assert(SETraits::getDecl(Existing)->isCanonicalDecl() &&

 RedeclarableTemplateDecl::CommonBase *
 FunctionTemplateDecl::newCommon(ASTContext &C) const {
   auto *CommonPtr = new (C) Common;
   C.addDestruction(CommonPtr);
   return CommonPtr;
 }
 
 void FunctionTemplateDecl::LoadLazySpecializations() const {
-  loadLazySpecializationsImpl();
+  loadLazySpecializationsImpl(/*OnlyPartial=*/false);
 }
 
 llvm::FoldingSetVector<FunctionTemplateSpecializationInfo> &
 FunctionTemplateDecl::getSpecializations() const {
   LoadLazySpecializations();
   return getCommonPtr()->Specializations;
 }
 
 FunctionDecl *
 FunctionTemplateDecl::findSpecialization(ArrayRef<TemplateArgument> Args,
                                          void *&InsertPos) {
-  return findSpecializationImpl(getSpecializations(), InsertPos, Args);
+  auto *Common = getCommonPtr();
+  return findSpecializationImpl(Common->Specializations, InsertPos, Args);
 }
 
 void FunctionTemplateDecl::addSpecialization(
       FunctionTemplateSpecializationInfo *Info, void *InsertPos) {
-  addSpecializationImpl<FunctionTemplateDecl>(getSpecializations(), Info,
+  auto *Common = getCommonPtr();
+  addSpecializationImpl<FunctionTemplateDecl>(Common->Specializations, Info,
                                               InsertPos);
 }
 
 void FunctionTemplateDecl::mergePrevDecl(FunctionTemplateDecl *Prev) {
   using Base = RedeclarableTemplateDecl;
 
   // If we haven't created a common pointer yet, then it can just be created
   // with the usual method.

 }
 
 ClassTemplateDecl *ClassTemplateDecl::CreateDeserialized(ASTContext &C,
                                                          unsigned ID) {
   return new (C, ID) ClassTemplateDecl(C, nullptr, SourceLocation(),
                                        DeclarationName(), nullptr, nullptr);
 }
 
-void ClassTemplateDecl::LoadLazySpecializations() const {
-  loadLazySpecializationsImpl();
+void ClassTemplateDecl::LoadLazySpecializations(
+    bool OnlyPartial /*=false*/) const {
+  loadLazySpecializationsImpl(OnlyPartial);
 }
 
 llvm::FoldingSetVector<ClassTemplateSpecializationDecl> &
 ClassTemplateDecl::getSpecializations() const {
   LoadLazySpecializations();
   return getCommonPtr()->Specializations;
 }
 
 llvm::FoldingSetVector<ClassTemplatePartialSpecializationDecl> &
 ClassTemplateDecl::getPartialSpecializations() const {
-  LoadLazySpecializations();
+  LoadLazySpecializations(/*PartialOnly=*/true);
   return getCommonPtr()->PartialSpecializations;
 }
 
 RedeclarableTemplateDecl::CommonBase *
 ClassTemplateDecl::newCommon(ASTContext &C) const {
   auto *CommonPtr = new (C) Common;
   C.addDestruction(CommonPtr);
   return CommonPtr;
 }
 
 ClassTemplateSpecializationDecl *
 ClassTemplateDecl::findSpecialization(ArrayRef<TemplateArgument> Args,
                                       void *&InsertPos) {
-  return findSpecializationImpl(getSpecializations(), InsertPos, Args);
+  auto *Common = getCommonPtr();
+  return findSpecializationImpl(Common->Specializations, InsertPos, Args);
 }
 
 void ClassTemplateDecl::AddSpecialization(ClassTemplateSpecializationDecl *D,
                                           void *InsertPos) {
-  addSpecializationImpl<ClassTemplateDecl>(getSpecializations(), D, InsertPos);
+  auto *Common = getCommonPtr();
+  addSpecializationImpl<ClassTemplateDecl>(Common->Specializations, D,
+                                           InsertPos);
 }
 
 ClassTemplatePartialSpecializationDecl *
 ClassTemplateDecl::findPartialSpecialization(
     ArrayRef<TemplateArgument> Args,
     TemplateParameterList *TPL, void *&InsertPos) {
   return findSpecializationImpl(getPartialSpecializations(), InsertPos, Args,
                                 TPL);

 
 TemplateArgumentList *
 TemplateArgumentList::CreateCopy(ASTContext &Context,
                                  ArrayRef<TemplateArgument> Args) {
   void *Mem = Context.Allocate(totalSizeToAlloc<TemplateArgument>(Args.size()));
   return new (Mem) TemplateArgumentList(Args);
 }
 
+unsigned TemplateArgumentList::ComputeODRHash(ArrayRef<TemplateArgument> Args) {
+  ODRHash Hasher;
+  for (TemplateArgument TA : Args)
+    Hasher.AddTemplateArgument(TA);
+
+  return Hasher.CalculateHash();
+}
+
 FunctionTemplateSpecializationInfo *FunctionTemplateSpecializationInfo::Create(
     ASTContext &C, FunctionDecl *FD, FunctionTemplateDecl *Template,
     TemplateSpecializationKind TSK, const TemplateArgumentList *TemplateArgs,
     const TemplateArgumentListInfo *TemplateArgsAsWritten, SourceLocation POI,
     MemberSpecializationInfo *MSInfo) {
   const ASTTemplateArgumentListInfo *ArgsAsWritten = nullptr;
   if (TemplateArgsAsWritten)
     ArgsAsWritten = ASTTemplateArgumentListInfo::Create(C,

 }
 
 VarTemplateDecl *VarTemplateDecl::CreateDeserialized(ASTContext &C,
                                                      unsigned ID) {
   return new (C, ID) VarTemplateDecl(C, nullptr, SourceLocation(),
                                      DeclarationName(), nullptr, nullptr);
 }
 
-void VarTemplateDecl::LoadLazySpecializations() const {
-  loadLazySpecializationsImpl();
+void VarTemplateDecl::LoadLazySpecializations(
+    bool OnlyPartial /*=false*/) const {
+  loadLazySpecializationsImpl(OnlyPartial);
 }
 
 llvm::FoldingSetVector<VarTemplateSpecializationDecl> &
 VarTemplateDecl::getSpecializations() const {
   LoadLazySpecializations();
   return getCommonPtr()->Specializations;
 }
 
 llvm::FoldingSetVector<VarTemplatePartialSpecializationDecl> &
 VarTemplateDecl::getPartialSpecializations() const {
-  LoadLazySpecializations();
+  LoadLazySpecializations(/*PartialOnly=*/true);
   return getCommonPtr()->PartialSpecializations;
 }
 
 RedeclarableTemplateDecl::CommonBase *
 VarTemplateDecl::newCommon(ASTContext &C) const {
   auto *CommonPtr = new (C) Common;
   C.addDestruction(CommonPtr);
   return CommonPtr;
 }
 
 VarTemplateSpecializationDecl *
 VarTemplateDecl::findSpecialization(ArrayRef<TemplateArgument> Args,
                                     void *&InsertPos) {
-  return findSpecializationImpl(getSpecializations(), InsertPos, Args);
+  auto *Common = getCommonPtr();
+  return findSpecializationImpl(Common->Specializations, InsertPos, Args);
 }
 
 void VarTemplateDecl::AddSpecialization(VarTemplateSpecializationDecl *D,
                                         void *InsertPos) {
-  addSpecializationImpl<VarTemplateDecl>(getSpecializations(), D, InsertPos);
+  auto *Common = getCommonPtr();
+  addSpecializationImpl<VarTemplateDecl>(Common->Specializations, D, InsertPos);
 }
 
 VarTemplatePartialSpecializationDecl *
 VarTemplateDecl::findPartialSpecialization(ArrayRef<TemplateArgument> Args,
      TemplateParameterList *TPL, void *&InsertPos) {
   return findSpecializationImpl(getPartialSpecializations(), InsertPos, Args,
                                 TPL);
 }

clang/lib/AST/ODRHash.cpp

const auto *Specialization =
             dyn_cast<ClassTemplateSpecializationDecl>(D);
   AddBoolean(Specialization);
   if (Specialization) {
     const TemplateArgumentList &List = Specialization->getTemplateArgs();
     ID.AddInteger(List.size());
     for (const TemplateArgument &TA : List.asArray())
       AddTemplateArgument(TA);
   }
+
+  // If this was a specialization we should take into account its template
+  // arguments. This helps to reduce collisions coming when visiting template
+  // specialization types (eg. when processing type template arguments).
+  ArrayRef<TemplateArgument> Args;
+  if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(D))
+    Args = CTSD->getTemplateArgs().asArray();
+  else if (auto *VTSD = dyn_cast<VarTemplateSpecializationDecl>(D))
+    Args = VTSD->getTemplateArgs().asArray();
+  else if (auto *FD = dyn_cast<FunctionDecl>(D))
+    if (FD->getTemplateSpecializationArgs())
+      Args = FD->getTemplateSpecializationArgs()->asArray();
+
+  for (auto &TA : Args)
+    AddTemplateArgument(TA);
 }
 
 namespace {
 // Process a Type pointer.  Add* methods call back into ODRHash while Visit*
 // methods process the relevant parts of the Type.
 class ODRTypeVisitor : public TypeVisitor<ODRTypeVisitor> {
   typedef TypeVisitor<ODRTypeVisitor> Inherited;
   llvm::FoldingSetNodeID &ID;

clang/lib/Serialization/ASTReader.cpp

if (DeclarationName Name = cast<NamedDecl>(D)->getDeclName()) {
         auto *DC = cast<DeclContext>(DCDecl);
         SmallVector<Decl*, 8> Decls;
         FindExternalLexicalDecls(
             DC, [&](Decl::Kind K) { return K == D->getKind(); }, Decls);
       }
     }
   }
 
-  if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(D))
-    CTSD->getSpecializedTemplate()->LoadLazySpecializations();
-  if (auto *VTSD = dyn_cast<VarTemplateSpecializationDecl>(D))
-    VTSD->getSpecializedTemplate()->LoadLazySpecializations();
-  if (auto *FD = dyn_cast<FunctionDecl>(D)) {
-    if (auto *Template = FD->getPrimaryTemplate())
-      Template->LoadLazySpecializations();
+  RedeclarableTemplateDecl *Template = nullptr;
+  ArrayRef<TemplateArgument> Args;
+  if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(D)) {
+    Template = CTSD->getSpecializedTemplate();
+    Args = CTSD->getTemplateArgs().asArray();
+  } else if (auto *VTSD = dyn_cast<VarTemplateSpecializationDecl>(D)) {
+    Template = VTSD->getSpecializedTemplate();
+    Args = VTSD->getTemplateArgs().asArray();
+  } else if (auto *FD = dyn_cast<FunctionDecl>(D)) {
+    if (auto *Tmplt = FD->getPrimaryTemplate()) {
+      Template = Tmplt;
+      Args = FD->getTemplateSpecializationArgs()->asArray();
     }
   }
 
+  if (Template)
+    Template->loadLazySpecializationsImpl(Args);
+}
+
 CXXCtorInitializer **
 ASTReader::GetExternalCXXCtorInitializers(uint64_t Offset) {
   RecordLocation Loc = getLocalBitOffset(Offset);
   BitstreamCursor &Cursor = Loc.F->DeclsCursor;
   SavedStreamPosition SavedPosition(Cursor);
   if (llvm::Error Err = Cursor.JumpToBit(Loc.Offset)) {
     Error(std::move(Err));
     return nullptr;

clang/lib/Serialization/ASTReaderDecl.cpp

namespace clang {
   class ASTDeclReader : public DeclVisitor<ASTDeclReader, void> {
     ASTReader &Reader;
     ASTRecordReader &Record;
     ASTReader::RecordLocation Loc;
     const DeclID ThisDeclID;
     const SourceLocation ThisDeclLoc;
 
     using RecordData = ASTReader::RecordData;
+    using LazySpecializationInfo =
+        RedeclarableTemplateDecl::LazySpecializationInfo;
 
     TypeID DeferredTypeID = 0;
     unsigned AnonymousDeclNumber = 0;
     GlobalDeclID NamedDeclForTagDecl = 0;
     IdentifierInfo *TypedefNameForLinkage = nullptr;
 
     bool HasPendingBody = false;
 
class ASTDeclReader : public DeclVisitor<ASTDeclReader, void> {
     serialization::DeclID readDeclID() {
       return Record.readDeclID();
     }
 
     std::string readString() {
       return Record.readString();
     }
 
-    void readDeclIDList(SmallVectorImpl<DeclID> &IDs) {
+    LazySpecializationInfo ReadLazySpecializationInfo() {
+      DeclID ID = readDeclID();
+      unsigned Hash = Record.readInt();
+      bool IsPartial = Record.readInt();
+      return LazySpecializationInfo(ID, Hash, IsPartial);
+    }
+
+    void readDeclIDList(SmallVectorImpl<LazySpecializationInfo> &IDs) {
       for (unsigned I = 0, Size = Record.readInt(); I != Size; ++I)
-        IDs.push_back(readDeclID());
+        IDs.push_back(ReadLazySpecializationInfo());
     }
 
     Decl *readDecl() {
       return Record.readDecl();
     }
 
     template<typename T>
     T *readDeclAs() {
namespace clang {
 
   public:
     ASTDeclReader(ASTReader &Reader, ASTRecordReader &Record,
                   ASTReader::RecordLocation Loc,
                   DeclID thisDeclID, SourceLocation ThisDeclLoc)
         : Reader(Reader), Record(Record), Loc(Loc), ThisDeclID(thisDeclID),
           ThisDeclLoc(ThisDeclLoc) {}
 
-    template <typename T> static
-    void AddLazySpecializations(T *D,
-                                SmallVectorImpl<serialization::DeclID>& IDs) {
+    template <typename T>
+    static void
+    AddLazySpecializations(T *D, SmallVectorImpl<LazySpecializationInfo> &IDs) {
       if (IDs.empty())
         return;
 
       // FIXME: We should avoid this pattern of getting the ASTContext.
       ASTContext &C = D->getASTContext();
 
       auto *&LazySpecializations = D->getCommonPtr()->LazySpecializations;
 
       if (auto &Old = LazySpecializations) {
-        IDs.insert(IDs.end(), Old + 1, Old + 1 + Old[0]);
+        IDs.insert(IDs.end(), Old + 1, Old + 1 + Old[0].DeclID);
         llvm::sort(IDs);
         IDs.erase(std::unique(IDs.begin(), IDs.end()), IDs.end());
       }
-
-      auto *Result = new (C) serialization::DeclID[1 + IDs.size()];
+      auto *Result = new (C) LazySpecializationInfo[1 + IDs.size()];
       *Result = IDs.size();
       std::copy(IDs.begin(), IDs.end(), Result + 1);
 
       LazySpecializations = Result;
     }
 
     template <typename DeclT>
     static Decl *getMostRecentDeclImpl(Redeclarable<DeclT> *D);
public:
     static void markIncompleteDeclChainImpl(...);
 
     /// Determine whether this declaration has a pending body.
     bool hasPendingBody() const { return HasPendingBody; }
 
     void ReadFunctionDefinition(FunctionDecl *FD);
     void Visit(Decl *D);
 
-    void UpdateDecl(Decl *D, SmallVectorImpl<serialization::DeclID> &);
+    void UpdateDecl(Decl *D, llvm::SmallVectorImpl<LazySpecializationInfo> &);
 
     static void setNextObjCCategory(ObjCCategoryDecl *Cat,
                                     ObjCCategoryDecl *Next) {
       Cat->NextClassCategory = Next;
     }
 
     void VisitDecl(Decl *D);
     void VisitPragmaCommentDecl(PragmaCommentDecl *D);

 
 void ASTDeclReader::VisitClassTemplateDecl(ClassTemplateDecl *D) {
   RedeclarableResult Redecl = VisitRedeclarableTemplateDecl(D);
   mergeRedeclarableTemplate(D, Redecl);
 
   if (ThisDeclID == Redecl.getFirstID()) {
     // This ClassTemplateDecl owns a CommonPtr; read it to keep track of all of
     // the specializations.
-    SmallVector<serialization::DeclID, 32> SpecIDs;
+    SmallVector<LazySpecializationInfo, 32> SpecIDs;
     readDeclIDList(SpecIDs);
     ASTDeclReader::AddLazySpecializations(D, SpecIDs);
   }
 
   if (D->getTemplatedDecl()->TemplateOrInstantiation) {
     // We were loaded before our templated declaration was. We've not set up
     // its corresponding type yet (see VisitCXXRecordDeclImpl), so reconstruct
     // it now.

 ///        VarTemplateDecl beyond TemplateDecl...
 void ASTDeclReader::VisitVarTemplateDecl(VarTemplateDecl *D) {
   RedeclarableResult Redecl = VisitRedeclarableTemplateDecl(D);
   mergeRedeclarableTemplate(D, Redecl);
 
   if (ThisDeclID == Redecl.getFirstID()) {
     // This VarTemplateDecl owns a CommonPtr; read it to keep track of all of
     // the specializations.
-    SmallVector<serialization::DeclID, 32> SpecIDs;
+    SmallVector<LazySpecializationInfo, 32> SpecIDs;
     readDeclIDList(SpecIDs);
     ASTDeclReader::AddLazySpecializations(D, SpecIDs);
   }
 }
 
 ASTDeclReader::RedeclarableResult
 ASTDeclReader::VisitClassTemplateSpecializationDeclImpl(
     ClassTemplateSpecializationDecl *D) {
if (Record.readInt())
     D->TemplateArgs = Record.readASTTemplateArgumentListInfo();
 }
 
 void ASTDeclReader::VisitFunctionTemplateDecl(FunctionTemplateDecl *D) {
   RedeclarableResult Redecl = VisitRedeclarableTemplateDecl(D);
 
   if (ThisDeclID == Redecl.getFirstID()) {
     // This FunctionTemplateDecl owns a CommonPtr; read it.
-    SmallVector<serialization::DeclID, 32> SpecIDs;
+    SmallVector<LazySpecializationInfo, 32> SpecIDs;
     readDeclIDList(SpecIDs);
     ASTDeclReader::AddLazySpecializations(D, SpecIDs);
   }
 }
 
 /// TODO: Unify with ClassTemplateSpecializationDecl version?
 ///       May require unifying ClassTemplate(Partial)SpecializationDecl and
 ///        VarTemplate(Partial)SpecializationDecl with a new data
void ASTReader::loadDeclUpdateRecords(PendingUpdateRecord &Record) {
   // The declaration may have been modified by files later in the chain.
   // If this is the case, read the record containing the updates from each file
   // and pass it to ASTDeclReader to make the modifications.
   serialization::GlobalDeclID ID = Record.ID;
   Decl *D = Record.D;
   ProcessingUpdatesRAIIObj ProcessingUpdates(*this);
   DeclUpdateOffsetsMap::iterator UpdI = DeclUpdateOffsets.find(ID);
 
-  SmallVector<serialization::DeclID, 8> PendingLazySpecializationIDs;
+  using LazySpecializationInfo =
+      RedeclarableTemplateDecl::LazySpecializationInfo;
+  llvm::SmallVector<LazySpecializationInfo, 8> PendingLazySpecializationIDs;
 
   if (UpdI != DeclUpdateOffsets.end()) {
     auto UpdateOffsets = std::move(UpdI->second);
     DeclUpdateOffsets.erase(UpdI);
 
     // Check if this decl was interesting to the consumer. If we just loaded
     // the declaration, then we know it was interesting and we skip the call
     // to isConsumerInterestedIn because it is unsafe to call in the
static void forAllLaterRedecls(DeclT *D, Fn F) {
   // If this declaration is merged, apply the functor to all later decls.
   if (Found) {
     for (auto *Redecl = MostRecent; Redecl != D;
          Redecl = Redecl->getPreviousDecl())
       F(Redecl);
   }
 }
 
-void ASTDeclReader::UpdateDecl(Decl *D,
-   llvm::SmallVectorImpl<serialization::DeclID> &PendingLazySpecializationIDs) {
+void ASTDeclReader::UpdateDecl(
+    Decl *D,
+    SmallVectorImpl<LazySpecializationInfo> &PendingLazySpecializationIDs) {
   while (Record.getIdx() < Record.size()) {
     switch ((DeclUpdateKind)Record.readInt()) {
     case UPD_CXX_ADDED_IMPLICIT_MEMBER: {
       auto *RD = cast<CXXRecordDecl>(D);
       Decl *MD = Record.readDecl();
       assert(MD && "couldn't read decl from update record");
       Reader.PendingAddedClassMembers.push_back({RD, MD});
       break;
     }
 
     case UPD_CXX_ADDED_TEMPLATE_SPECIALIZATION:
       // It will be added to the template's lazy specialization set.
-      PendingLazySpecializationIDs.push_back(readDeclID());
+      PendingLazySpecializationIDs.push_back(ReadLazySpecializationInfo());
       break;
 
     case UPD_CXX_ADDED_ANONYMOUS_NAMESPACE: {
       auto *Anon = readDeclAs<NamespaceDecl>();
 
       // Each module has its own anonymous namespace, which is disjoint from
       // any other module's anonymous namespaces, so don't attach the anonymous
       // namespace at all.

clang/lib/Serialization/ASTWriter.cpp

for (auto &Update : DeclUpdate.second) {
         HasUpdatedBody = true;
       else if (Kind == UPD_CXX_ADDED_VAR_DEFINITION)
         HasAddedVarDefinition = true;
       else
         Record.push_back(Kind);
 
       switch (Kind) {
       case UPD_CXX_ADDED_IMPLICIT_MEMBER:
-      case UPD_CXX_ADDED_TEMPLATE_SPECIALIZATION:
       case UPD_CXX_ADDED_ANONYMOUS_NAMESPACE:
         assert(Update.getDecl() && "no decl to add?");
         Record.push_back(GetDeclRef(Update.getDecl()));
         break;
-
+      case UPD_CXX_ADDED_TEMPLATE_SPECIALIZATION: {
+        const Decl *Spec = Update.getDecl();
+        assert(Spec && "no decl to add?");
+        Record.push_back(GetDeclRef(Spec));
+        ArrayRef<TemplateArgument> Args;
+        if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(Spec))
+          Args = CTSD->getTemplateArgs().asArray();
+        else if (auto *VTSD = dyn_cast<VarTemplateSpecializationDecl>(Spec))
+          Args = VTSD->getTemplateArgs().asArray();
+        else if (auto *FD = dyn_cast<FunctionDecl>(Spec))
+          Args = FD->getTemplateSpecializationArgs()->asArray();
+        assert(Args.size());
+        Record.push_back(TemplateArgumentList::ComputeODRHash(Args));
+        bool IsPartialSpecialization =
+            isa<ClassTemplatePartialSpecializationDecl>(Spec) ||
+            isa<VarTemplatePartialSpecializationDecl>(Spec);
+        Record.push_back(IsPartialSpecialization);
+        break;
+      }
       case UPD_CXX_ADDED_FUNCTION_DEFINITION:
       case UPD_CXX_ADDED_VAR_DEFINITION:
         break;
 
       case UPD_CXX_POINT_OF_INSTANTIATION:
         // FIXME: Do we need to also save the template specialization kind here?
         Record.AddSourceLocation(Update.getLoc());
         break;

clang/lib/Serialization/ASTWriterDecl.cpp

void AddObjCTypeParamList(ObjCTypeParamList *typeParams) {
       Record.push_back(typeParams->size());
       for (auto *typeParam : *typeParams) {
         Record.AddDeclRef(typeParam);
       }
       Record.AddSourceLocation(typeParams->getLAngleLoc());
       Record.AddSourceLocation(typeParams->getRAngleLoc());
     }
 
-    /// Add to the record the first declaration from each module file that
-    /// provides a declaration of D. The intent is to provide a sufficient
-    /// set such that reloading this set will load all current redeclarations.
-    void AddFirstDeclFromEachModule(const Decl *D, bool IncludeLocal) {
-      llvm::MapVector<ModuleFile*, const Decl*> Firsts;
+    /// Collect the first declaration from each module file that provides a
+    /// declaration of D.
+    void CollectFirstDeclFromEachModule(
+        const Decl *D, bool IncludeLocal,
+        llvm::MapVector<ModuleFile *, const Decl *> &Firsts) {
+
       // FIXME: We can skip entries that we know are implied by others.
       for (const Decl *R = D->getMostRecentDecl(); R; R = R->getPreviousDecl()) {
         if (R->isFromASTFile())
           Firsts[Writer.Chain->getOwningModuleFile(R)] = R;
         else if (IncludeLocal)
           Firsts[nullptr] = R;
       }
+    }
+
+    /// Add to the record the first declaration from each module file that
+    /// provides a declaration of D. The intent is to provide a sufficient
+    /// set such that reloading this set will load all current redeclarations.
+    void AddFirstDeclFromEachModule(const Decl *D, bool IncludeLocal) {
+      llvm::MapVector<ModuleFile *, const Decl *> Firsts;
+      CollectFirstDeclFromEachModule(D, IncludeLocal, Firsts);
+
       for (const auto &F : Firsts)
         Record.AddDeclRef(F.second);
     }
 
+    /// Add to the record the first template specialization from each module
+    /// file that provides a declaration of D. We store the DeclId and an
+    /// ODRHash of the template arguments of D which should provide enough
+    /// information to load D only if the template instantiator needs it.
+    void AddFirstSpecializationDeclFromEachModule(const Decl *D,
+                                                  bool IncludeLocal) {
+      assert(isa<ClassTemplateSpecializationDecl>(D) ||
+             isa<VarTemplateSpecializationDecl>(D) ||
+             isa<FunctionDecl>(D) && "Must not be called with other decls");
+      llvm::MapVector<ModuleFile *, const Decl *> Firsts;
+      CollectFirstDeclFromEachModule(D, IncludeLocal, Firsts);
+
+      for (const auto &F : Firsts) {
+        Record.AddDeclRef(F.second);
+        ArrayRef<TemplateArgument> Args;
+        if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(D))
+          Args = CTSD->getTemplateArgs().asArray();
+        else if (auto *VTSD = dyn_cast<VarTemplateSpecializationDecl>(D))
+          Args = VTSD->getTemplateArgs().asArray();
+        else if (auto *FD = dyn_cast<FunctionDecl>(D))
+          Args = FD->getTemplateSpecializationArgs()->asArray();
+        assert(Args.size());
+        Record.push_back(TemplateArgumentList::ComputeODRHash(Args));
+        bool IsPartialSpecialization =
+            isa<ClassTemplatePartialSpecializationDecl>(D) ||
+            isa<VarTemplatePartialSpecializationDecl>(D);
+        Record.push_back(IsPartialSpecialization);
+      }
+    }
+
     /// Get the specialization decl from an entry in the specialization list.
     template <typename EntryType>
     typename RedeclarableTemplateDecl::SpecEntryTraits<EntryType>::DeclType *
     getSpecializationDecl(EntryType &T) {
       return RedeclarableTemplateDecl::SpecEntryTraits<EntryType>::getDecl(&T);
     }
 
     /// Get the list of partial specializations from a template's common ptr.
     template<typename T>
     decltype(T::PartialSpecializations) &getPartialSpecializations(T *Common) {
       return Common->PartialSpecializations;
     }
-    ArrayRef<Decl> getPartialSpecializations(FunctionTemplateDecl::Common *) {
+    MutableArrayRef<FunctionTemplateSpecializationInfo>
+    getPartialSpecializations(FunctionTemplateDecl::Common *) {
       return std::nullopt;
     }
 
     template<typename DeclTy>
     void AddTemplateSpecializations(DeclTy *D) {
       auto *Common = D->getCommonPtr();
 
       // If we have any lazy specializations, and the external AST source is
       // our chained AST reader, we can just write out the DeclIDs. Otherwise,
       // we need to resolve them to actual declarations.
       if (Writer.Chain != Writer.Context->getExternalSource() &&
           Common->LazySpecializations) {
         D->LoadLazySpecializations();
         assert(!Common->LazySpecializations);
       }
 
-      ArrayRef<DeclID> LazySpecializations;
+      using LazySpecializationInfo =
+          RedeclarableTemplateDecl::LazySpecializationInfo;
+      ArrayRef<LazySpecializationInfo> LazySpecializations;
       if (auto *LS = Common->LazySpecializations)
-        LazySpecializations = llvm::ArrayRef(LS + 1, LS[0]);
+        LazySpecializations = llvm::ArrayRef(LS + 1, LS[0].DeclID);
 
       // Add a slot to the record for the number of specializations.
       unsigned I = Record.size();
       Record.push_back(0);
 
       // AddFirstDeclFromEachModule might trigger deserialization, invalidating
       // *Specializations iterators.
       llvm::SmallVector<const Decl*, 16> Specs;
       for (auto &Entry : Common->Specializations)
         Specs.push_back(getSpecializationDecl(Entry));
       for (auto &Entry : getPartialSpecializations(Common))
         Specs.push_back(getSpecializationDecl(Entry));
 
       for (auto *D : Specs) {
         assert(D->isCanonicalDecl() && "non-canonical decl in set");
-        AddFirstDeclFromEachModule(D, /*IncludeLocal*/true);
+        AddFirstSpecializationDeclFromEachModule(D, /*IncludeLocal*/ true);
+      }
+      for (auto &SpecInfo : LazySpecializations) {
+        Record.push_back(SpecInfo.DeclID);
+        Record.push_back(SpecInfo.ODRHash);
+        Record.push_back(SpecInfo.IsPartial);
       }
-      Record.append(LazySpecializations.begin(), LazySpecializations.end());
 
-      // Update the size entry we added earlier.
-      Record[I] = Record.size() - I - 1;
+      // Update the size entry we added earlier. We linerized the
+      // LazySpecializationInfo members and we need to adjust the size as we
+      // will read them always together.
+      assert((Record.size() - I - 1) % 3 == 0 &&
+             "Must be divisible by LazySpecializationInfo count!");
+      Record[I] = (Record.size() - I - 1) / 3;
     }
 
     /// Ensure that this template specialization is associated with the specified
     /// template on reload.
     void RegisterTemplateSpecialization(const Decl *Template,
                                         const Decl *Specialization) {
       Template = Template->getCanonicalDecl();
 

clang/test/Modules/cxx-templates.cpp

 // CHECK-NAMESPACE-N:      DeclarationName 'f'
 // CHECK-NAMESPACE-N-NEXT: |-FunctionTemplate {{.*}} 'f'
 // CHECK-NAMESPACE-N-NEXT: |-FunctionTemplate {{.*}} 'f'
 // CHECK-NAMESPACE-N-NEXT: |-FunctionTemplate {{.*}} 'f'
 // CHECK-NAMESPACE-N-NEXT: `-FunctionTemplate {{.*}} 'f'
 
 // 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
 // CHECK-DUMP-NEXT:       CopyConstructor
 // CHECK-DUMP-NEXT:       MoveConstructor
 // 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
 // CHECK-DUMP-NEXT:       CopyConstructor
 // CHECK-DUMP-NEXT:       MoveConstructor
 // 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]'

clang/test/Modules/odr_hash.cpp

struct S5 {
   Wrapper<S5 *> x;
 };
 #elif defined(SECOND)
 struct S5 {
   Wrapper<const S5 *> x;
 };
 #else
 S5 s5;
-// 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}}
+// expected-error@first.h:* {{'PointersAndReferences::S5::x' from module 'FirstModule' is not present in definition of 'PointersAndReferences::S5' in module 'SecondModule'}}
+// expected-note@second.h:* {{declaration of 'x' does not match}}
 #endif
 
 #if defined(FIRST)
 struct S6 {
   Wrapper<int &> x;
 };
 #elif defined(SECOND)
 struct S6 {