<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Sat, Aug 22, 2015 at 1:16 AM, Vassil Vassilev <span dir="ltr"><<a href="mailto:vvasilev@cern.ch" target="_blank">vvasilev@cern.ch</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On 22/08/15 03:47, Richard Smith via cfe-commits wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Author: rsmith<br>
Date: Fri Aug 21 20:47:18 2015<br>
New Revision: 245779<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=245779&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=245779&view=rev</a><br>
Log:<br>
[modules] Rearrange how redeclaration chains are loaded, to remove a walk over<br>
all modules and reduce the number of declarations we load when loading a<br>
redeclaration chain.<br>
<br>
The new approach is:<br>
  * when loading the first declaration of an entity within a module file, we<br>
    first load all declarations of the entity that were imported into that<br>
    module file, and then load all the other declarations of that entity from<br>
    that module file and build a suitable decl chain from them<br>
  * when loading any other declaration of an entity, we first load the first<br>
    declaration from the same module file<br>
<br>
As before, we complete redecl chains through name lookup where necessary.<br>
<br>
To make this work, I also had to change the way that template specializations<br>
are stored -- it no longer suffices to track only canonical specializations; we<br>
now emit all "first local" declarations when emitting a list of specializations<br>
for a template.<br>
<br>
On one testcase with several thousand imported module files, this reduces the<br>
total runtime by 72%.<br>
</blockquote></span>
Very nice!<br>
Does it reduce the depth of the redecl chains when merging? I.e. does this mean memory footprint reduction too?</blockquote><div><br></div><div>I wouldn't expect any difference there, and in any case, I think this would only affect the stack depth, which is typically bounded anyway since we normally build modules on a separate thread.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Modified:<br>
     cfe/trunk/include/clang/Serialization/ASTWriter.h<br>
     cfe/trunk/lib/Serialization/ASTReader.cpp<br>
     cfe/trunk/lib/Serialization/ASTReaderDecl.cpp<br>
     cfe/trunk/lib/Serialization/ASTWriter.cpp<br>
     cfe/trunk/lib/Serialization/ASTWriterDecl.cpp<br>
     cfe/trunk/test/Modules/cxx-templates.cpp<br>
<br>
Modified: cfe/trunk/include/clang/Serialization/ASTWriter.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTWriter.h?rev=245779&r1=245778&r2=245779&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTWriter.h?rev=245779&r1=245778&r2=245779&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Serialization/ASTWriter.h (original)<br>
+++ cfe/trunk/include/clang/Serialization/ASTWriter.h Fri Aug 21 20:47:18 2015<br>
@@ -405,6 +405,10 @@ private:<br>
    /// \brief The set of declarations that may have redeclaration chains that<br>
    /// need to be serialized.<br>
    llvm::SmallVector<const Decl *, 16> Redeclarations;<br>
+<br>
+  /// \brief A cache of the first local declaration for "interesting"<br>
+  /// redeclaration chains.<br>
+  llvm::DenseMap<const Decl *, const Decl *> FirstLocalDeclCache;<br>
                                            /// \brief Statements that we've encountered while serializing a<br>
    /// declaration or type.<br>
@@ -676,6 +680,10 @@ public:<br>
                            const ASTTemplateArgumentListInfo *ASTTemplArgList,<br>
                            RecordDataImpl &Record);<br>
  +  /// \brief Find the first local declaration of a given local redeclarable<br>
+  /// decl.<br>
+  const Decl *getFirstLocalDecl(const Decl *D);<br>
+<br>
    /// \brief Emit a reference to a declaration.<br>
    void AddDeclRef(const Decl *D, RecordDataImpl &Record);<br>
  @@ -857,12 +865,6 @@ public:<br>
    void CompletedTagDefinition(const TagDecl *D) override;<br>
    void AddedVisibleDecl(const DeclContext *DC, const Decl *D) override;<br>
    void AddedCXXImplicitMember(const CXXRecordDecl *RD, const Decl *D) override;<br>
-  void AddedCXXTemplateSpecialization(const ClassTemplateDecl *TD,<br>
-                             const ClassTemplateSpecializationDecl *D) override;<br>
-  void AddedCXXTemplateSpecialization(const VarTemplateDecl *TD,<br>
-                               const VarTemplateSpecializationDecl *D) override;<br>
-  void AddedCXXTemplateSpecialization(const FunctionTemplateDecl *TD,<br>
-                                      const FunctionDecl *D) override;<br>
    void ResolvedExceptionSpec(const FunctionDecl *FD) override;<br>
    void DeducedReturnType(const FunctionDecl *FD, QualType ReturnType) override;<br>
    void ResolvedOperatorDelete(const CXXDestructorDecl *DD,<br>
<br>
Modified: cfe/trunk/lib/Serialization/ASTReader.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=245779&r1=245778&r2=245779&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=245779&r1=245778&r2=245779&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)<br>
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Fri Aug 21 20:47:18 2015<br>
@@ -8123,11 +8123,8 @@ void ASTReader::finishPendingActions() {<br>
      PendingIncompleteDeclChains.clear();<br>
        // Load pending declaration chains.<br>
-    for (unsigned I = 0; I != PendingDeclChains.size(); ++I) {<br>
-      PendingDeclChainsKnown.erase(PendingDeclChains[I]);<br>
+    for (unsigned I = 0; I != PendingDeclChains.size(); ++I)<br>
        loadPendingDeclChain(PendingDeclChains[I]);<br>
-    }<br>
-    assert(PendingDeclChainsKnown.empty());<br>
      PendingDeclChains.clear();<br>
        assert(RedeclsDeserialized.empty() && "some redecls not wired up");<br>
<br>
Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=245779&r1=245778&r2=245779&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=245779&r1=245778&r2=245779&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)<br>
+++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Fri Aug 21 20:47:18 2015<br>
@@ -147,12 +147,6 @@ namespace clang {<br>
        }<br>
          ~RedeclarableResult() {<br>
-        if (FirstID && Owning &&<br>
-            isRedeclarableDeclKind(LoadedDecl->getKind())) {<br>
-          auto Canon = Reader.GetDecl(FirstID)->getCanonicalDecl();<br>
-          if (Reader.PendingDeclChainsKnown.insert(Canon).second)<br>
-            Reader.PendingDeclChains.push_back(Canon);<br>
-        }<br>
        }<br>
          /// \brief Note that a RedeclarableDecl is not actually redeclarable.<br>
@@ -2186,23 +2180,33 @@ ASTDeclReader::RedeclarableResult<br>
  ASTDeclReader::VisitRedeclarable(Redeclarable<T> *D) {<br>
    DeclID FirstDeclID = ReadDeclID(Record, Idx);<br>
    Decl *MergeWith = nullptr;<br>
+<br>
    bool IsKeyDecl = ThisDeclID == FirstDeclID;<br>
+  bool IsFirstLocalDecl = false;<br>
      // 0 indicates that this declaration was the only declaration of its entity,<br>
    // and is used for space optimization.<br>
    if (FirstDeclID == 0) {<br>
      FirstDeclID = ThisDeclID;<br>
      IsKeyDecl = true;<br>
+    IsFirstLocalDecl = true;<br>
    } else if (unsigned N = Record[Idx++]) {<br>
-    IsKeyDecl = false;<br>
+    // This declaration was the first local declaration, but may have imported<br>
+    // other declarations.<br>
+    IsKeyDecl = N == 1;<br>
+    IsFirstLocalDecl = true;<br>
        // We have some declarations that must be before us in our redeclaration<br>
      // chain. Read them now, and remember that we ought to merge with one of<br>
      // them.<br>
      // FIXME: Provide a known merge target to the second and subsequent such<br>
      // declaration.<br>
-    for (unsigned I = 0; I != N; ++I)<br>
+    for (unsigned I = 0; I != N - 1; ++I)<br>
        MergeWith = ReadDecl(Record, Idx/*, MergeWith*/);<br>
+  } else {<br>
+    // This declaration was not the first local declaration. Read the first<br>
+    // local declaration now, to trigger the import of other redeclarations.<br>
+    (void)ReadDecl(Record, Idx);<br>
    }<br>
      T *FirstDecl = cast_or_null<T>(Reader.GetDecl(FirstDeclID));<br>
@@ -2214,13 +2218,21 @@ ASTDeclReader::VisitRedeclarable(Redecla<br>
      D->RedeclLink = Redeclarable<T>::PreviousDeclLink(FirstDecl);<br>
      D->First = FirstDecl->getCanonicalDecl();<br>
    }<br>
-<br>
+<br>
    // Note that this declaration has been deserialized.<br>
-  Reader.RedeclsDeserialized.insert(static_cast<T *>(D));<br>
-<br>
+  T *DAsT = static_cast<T*>(D);<br>
+  Reader.RedeclsDeserialized.insert(DAsT);<br>
+<br>
+  // Note that we need to load local redeclarations of this decl and build a<br>
+  // decl chain for them. This must happen *after* we perform the preloading<br>
+  // above; this ensures that the redeclaration chain is built in the correct<br>
+  // order.<br>
+  if (IsFirstLocalDecl)<br>
+    Reader.PendingDeclChains.push_back(DAsT);<br>
+<br>
    // The result structure takes care to note that we need to load the<br>
    // other declaration chains for this ID.<br>
-  return RedeclarableResult(Reader, FirstDeclID, static_cast<T *>(D), MergeWith,<br>
+  return RedeclarableResult(Reader, FirstDeclID, DAsT, MergeWith,<br>
                              IsKeyDecl);<br>
  }<br>
  @@ -2330,11 +2342,8 @@ void ASTDeclReader::mergeRedeclarable(Re<br>
            TemplatePatternID, Redecl.isKeyDecl());<br>
        // If this declaration is a key declaration, make a note of that.<br>
-    if (Redecl.isKeyDecl()) {<br>
+    if (Redecl.isKeyDecl())<br>
        Reader.KeyDecls[ExistingCanon].push_back(Redecl.getFirstID());<br>
-      if (Reader.PendingDeclChainsKnown.insert(ExistingCanon).second)<br>
-        Reader.PendingDeclChains.push_back(ExistingCanon);<br>
-    }<br>
    }<br>
  }<br>
  @@ -3424,15 +3433,12 @@ namespace {<br>
      ASTReader &Reader;<br>
      SmallVectorImpl<DeclID> &SearchDecls;<br>
      llvm::SmallPtrSetImpl<Decl *> &Deserialized;<br>
-    GlobalDeclID CanonID;<br>
      SmallVector<Decl *, 4> Chain;<br>
      public:<br>
      RedeclChainVisitor(ASTReader &Reader, SmallVectorImpl<DeclID> &SearchDecls,<br>
-                       llvm::SmallPtrSetImpl<Decl *> &Deserialized,<br>
-                       GlobalDeclID CanonID)<br>
-      : Reader(Reader), SearchDecls(SearchDecls), Deserialized(Deserialized),<br>
-        CanonID(CanonID) {<br>
+                       llvm::SmallPtrSetImpl<Decl *> &Deserialized)<br>
+      : Reader(Reader), SearchDecls(SearchDecls), Deserialized(Deserialized) {<br>
        assert(std::is_sorted(SearchDecls.begin(), SearchDecls.end()));<br>
      }<br>
  @@ -3518,10 +3524,10 @@ namespace {<br>
            break;<br>
        }<br>
  -      if (LocalSearchDeclID && LocalSearchDeclID != CanonID) {<br>
+      assert(LocalSearchDeclID);<br>
+      if (LocalSearchDeclID) {<br>
          // If the search decl was from this module, add it to the chain.<br>
          // Note, the chain is sorted from newest to oldest, so this goes last.<br>
-        // We exclude the canonical declaration; it implicitly goes at the end.<br>
          addToChain(Reader.GetDecl(LocalSearchDeclID));<br>
        }<br>
  @@ -3531,26 +3537,14 @@ namespace {<br>
    };<br>
  }<br>
  -void ASTReader::loadPendingDeclChain(Decl *CanonDecl) {<br>
-  // The decl might have been merged into something else after being added to<br>
-  // our list. If it was, just skip it.<br>
-  if (!CanonDecl->isCanonicalDecl())<br>
-    return;<br>
-<br>
+void ASTReader::loadPendingDeclChain(Decl *FirstLocal) {<br>
    // Determine the set of declaration IDs we'll be searching for.<br>
-  SmallVector<DeclID, 16> SearchDecls;<br>
-  GlobalDeclID CanonID = CanonDecl->getGlobalID();<br>
-  if (CanonID)<br>
-    SearchDecls.push_back(CanonDecl->getGlobalID()); // Always first.<br>
-  KeyDeclsMap::iterator KeyPos = KeyDecls.find(CanonDecl);<br>
-  if (KeyPos != KeyDecls.end())<br>
-    SearchDecls.append(KeyPos->second.begin(), KeyPos->second.end());<br>
-  llvm::array_pod_sort(SearchDecls.begin(), SearchDecls.end());<br>
+  SmallVector<DeclID, 1> SearchDecls;<br>
+  SearchDecls.push_back(FirstLocal->getGlobalID());<br>
      // Build up the list of redeclarations.<br>
-  RedeclChainVisitor Visitor(*this, SearchDecls, RedeclsDeserialized, CanonID);<br>
-  ModuleMgr.visit(Visitor);<br>
-  RedeclsDeserialized.erase(CanonDecl);<br>
+  RedeclChainVisitor Visitor(*this, SearchDecls, RedeclsDeserialized);<br>
+  Visitor(*getOwningModuleFile(FirstLocal));<br>
      // Retrieve the chains.<br>
    ArrayRef<Decl *> Chain = Visitor.getChain();<br>
@@ -3561,11 +3555,14 @@ void ASTReader::loadPendingDeclChain(Dec<br>
    //<br>
    // FIXME: We have three different dispatches on decl kind here; maybe<br>
    // we should instead generate one loop per kind and dispatch up-front?<br>
+  Decl *CanonDecl = FirstLocal->getCanonicalDecl();<br>
    Decl *MostRecent = ASTDeclReader::getMostRecentDecl(CanonDecl);<br>
    if (!MostRecent)<br>
      MostRecent = CanonDecl;<br>
    for (unsigned I = 0, N = Chain.size(); I != N; ++I) {<br>
      auto *D = Chain[N - I - 1];<br>
+    if (D == CanonDecl)<br>
+      continue;<br>
      ASTDeclReader::attachPreviousDecl(*this, D, MostRecent, CanonDecl);<br>
      MostRecent = D;<br>
    }<br>
<br>
Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=245779&r1=245778&r2=245779&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=245779&r1=245778&r2=245779&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)<br>
+++ cfe/trunk/lib/Serialization/ASTWriter.cpp Fri Aug 21 20:47:18 2015<br>
@@ -3799,41 +3799,39 @@ void ASTWriter::WriteRedeclarations() {<br>
    SmallVector<serialization::LocalRedeclarationsInfo, 2> LocalRedeclsMap;<br>
      for (unsigned I = 0, N = Redeclarations.size(); I != N; ++I) {<br>
-    const Decl *Key = Redeclarations[I];<br>
-    assert((Chain ? Chain->getKeyDeclaration(Key) == Key<br>
-                  : Key->isFirstDecl()) &&<br>
-           "not the key declaration");<br>
+    const Decl *FirstLocal = Redeclarations[I];<br>
+    assert(!FirstLocal->isFromASTFile() &&<br>
+           (!FirstLocal->getPreviousDecl() ||<br>
+            FirstLocal->getPreviousDecl()->isFromASTFile() ||<br>
+            getDeclID(FirstLocal->getPreviousDecl()) < NUM_PREDEF_DECL_IDS) &&<br>
+           "not the first local declaration");<br>
+    assert(getDeclID(FirstLocal) >= NUM_PREDEF_DECL_IDS &&<br>
+           "should not have predefined decl as first decl");<br>
  -    const Decl *First = Key->getCanonicalDecl();<br>
-    const Decl *MostRecent = First->getMostRecentDecl();<br>
-<br>
-    assert((getDeclID(First) >= NUM_PREDEF_DECL_IDS || First == Key) &&<br>
-           "should not have imported key decls for predefined decl");<br>
-<br>
-    // If we only have a single declaration, there is no point in storing<br>
-    // a redeclaration chain.<br>
-    if (First == MostRecent)<br>
-      continue;<br>
-<br>
      unsigned Offset = LocalRedeclChains.size();<br>
      unsigned Size = 0;<br>
      LocalRedeclChains.push_back(0); // Placeholder for the size.<br>
        // Collect the set of local redeclarations of this declaration, from newest<br>
      // to oldest.<br>
-    for (const Decl *Prev = MostRecent; Prev;<br>
-         Prev = Prev->getPreviousDecl()) {<br>
-      if (!Prev->isFromASTFile() && Prev != Key) {<br>
+    for (const Decl *Prev = FirstLocal->getMostRecentDecl(); Prev != FirstLocal;<br>
+         Prev = Prev->getPreviousDecl()) {<br>
+      if (!Prev->isFromASTFile()) {<br>
          AddDeclRef(Prev, LocalRedeclChains);<br>
          ++Size;<br>
        }<br>
      }<br>
  +    // If we only have a single local declaration, there is no point in storing<br>
+    // a redeclaration chain.<br>
+    if (LocalRedeclChains.size() == 1)<br>
+      continue;<br>
+<br>
      LocalRedeclChains[Offset] = Size;<br>
        // Add the mapping from the first ID from the AST to the set of local<br>
      // declarations.<br>
-    LocalRedeclarationsInfo Info = { getDeclID(Key), Offset };<br>
+    LocalRedeclarationsInfo Info = { getDeclID(FirstLocal), Offset };<br>
      LocalRedeclsMap.push_back(Info);<br>
        assert(N == Redeclarations.size() &&<br>
@@ -4145,8 +4143,6 @@ void ASTWriter::WriteASTCore(Sema &SemaR<br>
      if (D) {<br>
        assert(D->isCanonicalDecl() && "predefined decl is not canonical");<br>
        DeclIDs[D] = ID;<br>
-      if (D->getMostRecentDecl() != D)<br>
-        Redeclarations.push_back(D);<br>
      }<br>
    };<br>
    RegisterPredefDecl(Context.getTranslationUnitDecl(),<br>
@@ -5730,42 +5726,6 @@ void ASTWriter::AddedCXXImplicitMember(c<br>
    DeclUpdates[RD].push_back(DeclUpdate(UPD_CXX_ADDED_IMPLICIT_MEMBER, D));<br>
  }<br>
  -void ASTWriter::AddedCXXTemplateSpecialization(const ClassTemplateDecl *TD,<br>
-                                     const ClassTemplateSpecializationDecl *D) {<br>
-  // The specializations set is kept in the canonical template.<br>
-  TD = TD->getCanonicalDecl();<br>
-  if (!(!D->isFromASTFile() && TD->isFromASTFile()))<br>
-    return; // Not a source specialization added to a template from PCH.<br>
-<br>
-  assert(!WritingAST && "Already writing the AST!");<br>
-  DeclUpdates[TD].push_back(DeclUpdate(UPD_CXX_ADDED_TEMPLATE_SPECIALIZATION,<br>
-                                       D));<br>
-}<br>
-<br>
-void ASTWriter::AddedCXXTemplateSpecialization(<br>
-    const VarTemplateDecl *TD, const VarTemplateSpecializationDecl *D) {<br>
-  // The specializations set is kept in the canonical template.<br>
-  TD = TD->getCanonicalDecl();<br>
-  if (!(!D->isFromASTFile() && TD->isFromASTFile()))<br>
-    return; // Not a source specialization added to a template from PCH.<br>
-<br>
-  assert(!WritingAST && "Already writing the AST!");<br>
-  DeclUpdates[TD].push_back(DeclUpdate(UPD_CXX_ADDED_TEMPLATE_SPECIALIZATION,<br>
-                                       D));<br>
-}<br>
-<br>
-void ASTWriter::AddedCXXTemplateSpecialization(const FunctionTemplateDecl *TD,<br>
-                                               const FunctionDecl *D) {<br>
-  // The specializations set is kept in the canonical template.<br>
-  TD = TD->getCanonicalDecl();<br>
-  if (!(!D->isFromASTFile() && TD->isFromASTFile()))<br>
-    return; // Not a source specialization added to a template from PCH.<br>
-<br>
-  assert(!WritingAST && "Already writing the AST!");<br>
-  DeclUpdates[TD].push_back(DeclUpdate(UPD_CXX_ADDED_TEMPLATE_SPECIALIZATION,<br>
-                                       D));<br>
-}<br>
-<br>
  void ASTWriter::ResolvedExceptionSpec(const FunctionDecl *FD) {<br>
    assert(!DoneWritingDeclsAndTypes && "Already done writing updates!");<br>
    if (!Chain) return;<br>
<br>
Modified: cfe/trunk/lib/Serialization/ASTWriterDecl.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriterDecl.cpp?rev=245779&r1=245778&r2=245779&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriterDecl.cpp?rev=245779&r1=245778&r2=245779&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Serialization/ASTWriterDecl.cpp (original)<br>
+++ cfe/trunk/lib/Serialization/ASTWriterDecl.cpp Fri Aug 21 20:47:18 2015<br>
@@ -159,6 +159,19 @@ namespace clang {<br>
        Writer.AddStmt(FD->getBody());<br>
      }<br>
  +    /// Add to the record the first declaration from each module file that<br>
+    /// provides a declaration of D. The intent is to provide a sufficient<br>
+    /// set such that reloading this set will load all current redeclarations.<br>
+    void AddFirstDeclFromEachModule(const Decl *D, bool IncludeLocal) {<br>
+      llvm::MapVector<ModuleFile*, const Decl*> Firsts;<br>
+      // FIXME: We can skip entries that we know are implied by others.<br>
+      for (const Decl *R = D->getMostRecentDecl(); R; R = R->getPreviousDecl())<br>
+        if (IncludeLocal || R->isFromASTFile())<br>
+          Firsts[Writer.Chain->getOwningModuleFile(R)] = R;<br>
+      for (const auto &F : Firsts)<br>
+        Writer.AddDeclRef(F.second, Record);<br>
+    }<br>
+<br>
      /// Get the specialization decl from an entry in the specialization list.<br>
      template <typename EntryType><br>
      typename RedeclarableTemplateDecl::SpecEntryTraits<EntryType>::DeclType *<br>
@@ -194,20 +207,46 @@ namespace clang {<br>
        if (auto *LS = Common->LazySpecializations)<br>
          LazySpecializations = ArrayRef<DeclID>(LS + 1, LS + 1 + LS[0]);<br>
  -      Record.push_back(Specializations.size() +<br>
-                       PartialSpecializations.size() +<br>
-                       LazySpecializations.size());<br>
+      // Add a slot to the record for the number of specializations.<br>
+      unsigned I = Record.size();<br>
+      Record.push_back(0);<br>
+<br>
        for (auto &Entry : Specializations) {<br>
          auto *D = getSpecializationDecl(Entry);<br>
          assert(D->isCanonicalDecl() && "non-canonical decl in set");<br>
-        Writer.AddDeclRef(D, Record);<br>
+        AddFirstDeclFromEachModule(D, /*IncludeLocal*/true);<br>
        }<br>
        for (auto &Entry : PartialSpecializations) {<br>
          auto *D = getSpecializationDecl(Entry);<br>
          assert(D->isCanonicalDecl() && "non-canonical decl in set");<br>
-        Writer.AddDeclRef(D, Record);<br>
+        AddFirstDeclFromEachModule(D, /*IncludeLocal*/true);<br>
        }<br>
        Record.append(LazySpecializations.begin(), LazySpecializations.end());<br>
+<br>
+      // Update the size entry we added earlier.<br>
+      Record[I] = Record.size() - I - 1;<br>
+    }<br>
+<br>
+    /// Ensure that this template specialization is associated with the specified<br>
+    /// template on reload.<br>
+    void RegisterTemplateSpecialization(const Decl *Template,<br>
+                                        const Decl *Specialization) {<br>
+      Template = Template->getCanonicalDecl();<br>
+<br>
+      // If the canonical template is local, we'll write out this specialization<br>
+      // when we emit it.<br>
+      // FIXME: We can do the same thing if there is any local declaration of<br>
+      // the template, to avoid emitting an update record.<br>
+      if (!Template->isFromASTFile())<br>
+        return;<br>
+<br>
+      // We only need to associate the first local declaration of the<br>
+      // specialization. The other declarations will get pulled in by it.<br>
+      if (Writer.getFirstLocalDecl(Specialization) != Specialization)<br>
+        return;<br>
+<br>
+      Writer.DeclUpdates[Template].push_back(ASTWriter::DeclUpdate(<br>
+          UPD_CXX_ADDED_TEMPLATE_SPECIALIZATION, Specialization));<br>
      }<br>
    };<br>
  }<br>
@@ -479,6 +518,9 @@ void ASTDeclWriter::VisitFunctionDecl(Fu<br>
    case FunctionDecl::TK_FunctionTemplateSpecialization: {<br>
      FunctionTemplateSpecializationInfo *<br>
        FTSInfo = D->getTemplateSpecializationInfo();<br>
+<br>
+    RegisterTemplateSpecialization(FTSInfo->getTemplate(), D);<br>
+<br>
      Writer.AddDeclRef(FTSInfo->getTemplate(), Record);<br>
      Record.push_back(FTSInfo->getTemplateSpecializationKind());<br>
      @@ -1249,6 +1291,8 @@ void ASTDeclWriter::VisitClassTemplateDe<br>
    void ASTDeclWriter::VisitClassTemplateSpecializationDecl(<br>
                                             ClassTemplateSpecializationDecl *D) {<br>
+  RegisterTemplateSpecialization(D->getSpecializedTemplate(), D);<br>
+<br>
    VisitCXXRecordDecl(D);<br>
      llvm::PointerUnion<ClassTemplateDecl *,<br>
@@ -1308,6 +1352,8 @@ void ASTDeclWriter::VisitVarTemplateDecl<br>
    void ASTDeclWriter::VisitVarTemplateSpecializationDecl(<br>
      VarTemplateSpecializationDecl *D) {<br>
+  RegisterTemplateSpecialization(D->getSpecializedTemplate(), D);<br>
+<br>
    VisitVarDecl(D);<br>
      llvm::PointerUnion<VarTemplateDecl *, VarTemplatePartialSpecializationDecl *><br>
@@ -1478,48 +1524,61 @@ void ASTDeclWriter::VisitDeclContext(Dec<br>
    Record.push_back(VisibleOffset);<br>
  }<br>
  -/// Determine whether D is the first declaration in its redeclaration chain that<br>
-/// is not from an AST file.<br>
-template <typename T><br>
-static bool isFirstLocalDecl(Redeclarable<T> *D) {<br>
-  assert(D && !static_cast<T*>(D)->isFromASTFile());<br>
-  do<br>
-    D = D->getPreviousDecl();<br>
-  while (D && static_cast<T*>(D)->isFromASTFile());<br>
-  return !D;<br>
+/// \brief Is this a local declaration (that is, one that will be written to<br>
+/// our AST file)? This is the case for declarations that are neither imported<br>
+/// from another AST file nor predefined.<br>
+static bool isLocalDecl(ASTWriter &W, const Decl *D) {<br>
+  if (D->isFromASTFile())<br>
+    return false;<br>
+  return W.getDeclID(D) >= NUM_PREDEF_DECL_IDS;<br>
+}<br>
+<br>
+const Decl *ASTWriter::getFirstLocalDecl(const Decl *D) {<br>
+  assert(isLocalDecl(*this, D) && "expected a local declaration");<br>
+<br>
+  const Decl *Canon = D->getCanonicalDecl();<br>
+  if (isLocalDecl(*this, Canon))<br>
+    return Canon;<br>
+<br>
+  const Decl *&CacheEntry = FirstLocalDeclCache[Canon];<br>
+  if (CacheEntry)<br>
+    return CacheEntry;<br>
+<br>
+  for (const Decl *Redecl = D; Redecl; Redecl = Redecl->getPreviousDecl())<br>
+    if (isLocalDecl(*this, Redecl))<br>
+      D = Redecl;<br>
+  return CacheEntry = D;<br>
  }<br>
    template <typename T><br>
  void ASTDeclWriter::VisitRedeclarable(Redeclarable<T> *D) {<br>
    T *First = D->getFirstDecl();<br>
    T *MostRecent = First->getMostRecentDecl();<br>
+  T *DAsT = static_cast<T *>(D);<br>
    if (MostRecent != First) {<br>
-    assert(isRedeclarableDeclKind(static_cast<T *>(D)->getKind()) &&<br>
+    assert(isRedeclarableDeclKind(DAsT->getKind()) &&<br>
             "Not considered redeclarable?");<br>
        Writer.AddDeclRef(First, Record);<br>
  -    // In a modules build, emit a list of all imported key declarations<br>
-    // (excluding First, if it was imported), so that we can be sure that all<br>
-    // redeclarations visible to this module are before D in the redecl chain.<br>
-    unsigned I = Record.size();<br>
-    Record.push_back(0);<br>
-    if (Context.getLangOpts().Modules && Writer.Chain) {<br>
-      if (isFirstLocalDecl(D)) {<br>
-        Writer.Chain->forEachImportedKeyDecl(First, [&](const Decl *D) {<br>
-          if (D != First)<br>
-            Writer.AddDeclRef(D, Record);<br>
-        });<br>
-        Record[I] = Record.size() - I - 1;<br>
-<br>
-        // Write a redeclaration chain, attached to the first key decl.<br>
-        Writer.Redeclarations.push_back(Writer.Chain->getKeyDeclaration(First));<br>
-      }<br>
-    } else if (D == First || D->getPreviousDecl()->isFromASTFile()) {<br>
-      assert(isFirstLocalDecl(D) && "imported decl after local decl");<br>
-<br>
-      // Write a redeclaration chain attached to the first decl.<br>
-      Writer.Redeclarations.push_back(First);<br>
+    // Write out a list of local redeclarations of this declaration if it's the<br>
+    // first local declaration in the chain.<br>
+    const Decl *FirstLocal = Writer.getFirstLocalDecl(DAsT);<br>
+    if (DAsT == FirstLocal) {<br>
+      Writer.Redeclarations.push_back(DAsT);<br>
+<br>
+      // Emit a list of all imported first declarations so that we can be sure<br>
+      // that all redeclarations visible to this module are before D in the<br>
+      // redecl chain.<br>
+      unsigned I = Record.size();<br>
+      Record.push_back(0);<br>
+      if (Writer.Chain)<br>
+        AddFirstDeclFromEachModule(DAsT, /*IncludeLocal*/false);<br>
+      // This is the number of imported first declarations + 1.<br>
+      Record[I] = Record.size() - I;<br>
+    } else {<br>
+      Record.push_back(0);<br>
+      Writer.AddDeclRef(FirstLocal, Record);<br>
      }<br>
        // Make sure that we serialize both the previous and the most-recent<br>
<br>
Modified: cfe/trunk/test/Modules/cxx-templates.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/cxx-templates.cpp?rev=245779&r1=245778&r2=245779&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/cxx-templates.cpp?rev=245779&r1=245778&r2=245779&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/Modules/cxx-templates.cpp (original)<br>
+++ cfe/trunk/test/Modules/cxx-templates.cpp Fri Aug 21 20:47:18 2015<br>
@@ -187,10 +187,10 @@ namespace Std {<br>
    // CHECK-DUMP:      ClassTemplateDecl {{.*}} <{{.*[/\\]}}cxx-templates-common.h:1:1, {{.*}}>  col:{{.*}} in cxx_templates_common SomeTemplate<br>
  // CHECK-DUMP:        ClassTemplateSpecializationDecl {{.*}} prev {{.*}} SomeTemplate<br>
-// CHECK-DUMP-NEXT:     TemplateArgument type 'char [2]'<br>
-// CHECK-DUMP:        ClassTemplateSpecializationDecl {{.*}} SomeTemplate definition<br>
-// CHECK-DUMP-NEXT:     TemplateArgument type 'char [2]'<br>
-// CHECK-DUMP:        ClassTemplateSpecializationDecl {{.*}} prev {{.*}} SomeTemplate<br>
  // CHECK-DUMP-NEXT:     TemplateArgument type 'char [1]'<br>
  // CHECK-DUMP:        ClassTemplateSpecializationDecl {{.*}} SomeTemplate definition<br>
  // CHECK-DUMP-NEXT:     TemplateArgument type 'char [1]'<br>
+// CHECK-DUMP:        ClassTemplateSpecializationDecl {{.*}} prev {{.*}} SomeTemplate<br>
+// CHECK-DUMP-NEXT:     TemplateArgument type 'char [2]'<br>
+// CHECK-DUMP:        ClassTemplateSpecializationDecl {{.*}} SomeTemplate definition<br>
+// CHECK-DUMP-NEXT:     TemplateArgument type 'char [2]'<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote>
<br>
</div></div></blockquote></div><br></div></div>