r246582 - Re-commit r246497 (and dependent changes r246524 and r246521), reverted in

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Tue Sep 1 13:35:42 PDT 2015


Author: rsmith
Date: Tue Sep  1 15:35:42 2015
New Revision: 246582

URL: http://llvm.org/viewvc/llvm-project?rev=246582&view=rev
Log:
Re-commit r246497 (and dependent changes r246524 and r246521), reverted in
r246546, with a workaround for an MSVC 2013 miscompile and an MSVC 2015
rejects-valid.

Original commit message:

[modules] Rework serialized DeclContext lookup table management. Instead of
walking the loaded ModuleFiles looking for lookup tables for the context, store
them all in one place, and merge them together if we find we have too many
(currently, more than 4). If we do merge, include the merged form in our
serialized lookup table, so that downstream readers never need to look at our
imports' tables.

This gives a huge performance improvement to builds with very large numbers of
modules (in some cases, more than a 2x speedup was observed).

Added:
    cfe/trunk/lib/Serialization/MultiOnDiskHashTable.h
      - copied, changed from r246545, cfe/trunk/lib/Serialization/MultiOnDiskHashTable.h
Modified:
    cfe/trunk/include/clang/Serialization/ASTBitCodes.h
    cfe/trunk/include/clang/Serialization/ASTReader.h
    cfe/trunk/include/clang/Serialization/ASTWriter.h
    cfe/trunk/include/clang/Serialization/Module.h
    cfe/trunk/lib/Serialization/ASTReader.cpp
    cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
    cfe/trunk/lib/Serialization/ASTReaderInternals.h
    cfe/trunk/lib/Serialization/ASTWriter.cpp
    cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
    cfe/trunk/lib/Serialization/Module.cpp
    cfe/trunk/test/Modules/cxx-templates.cpp
    cfe/trunk/test/Modules/merge-using-decls.cpp

Modified: cfe/trunk/include/clang/Serialization/ASTBitCodes.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTBitCodes.h?rev=246582&r1=246581&r2=246582&view=diff
==============================================================================
--- cfe/trunk/include/clang/Serialization/ASTBitCodes.h (original)
+++ cfe/trunk/include/clang/Serialization/ASTBitCodes.h Tue Sep  1 15:35:42 2015
@@ -1530,4 +1530,23 @@ namespace clang {
   }
 } // end namespace clang
 
+namespace llvm {
+  template <> struct DenseMapInfo<clang::serialization::DeclarationNameKey> {
+    static clang::serialization::DeclarationNameKey getEmptyKey() {
+      return clang::serialization::DeclarationNameKey(-1, 1);
+    }
+    static clang::serialization::DeclarationNameKey getTombstoneKey() {
+      return clang::serialization::DeclarationNameKey(-1, 2);
+    }
+    static unsigned
+    getHashValue(const clang::serialization::DeclarationNameKey &Key) {
+      return Key.getHash();
+    }
+    static bool isEqual(const clang::serialization::DeclarationNameKey &L,
+                        const clang::serialization::DeclarationNameKey &R) {
+      return L == R;
+    }
+  };
+}
+
 #endif

Modified: cfe/trunk/include/clang/Serialization/ASTReader.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTReader.h?rev=246582&r1=246581&r2=246582&view=diff
==============================================================================
--- cfe/trunk/include/clang/Serialization/ASTReader.h (original)
+++ cfe/trunk/include/clang/Serialization/ASTReader.h Tue Sep  1 15:35:42 2015
@@ -282,9 +282,8 @@ class ReadMethodPoolVisitor;
 
 namespace reader {
   class ASTIdentifierLookupTrait;
-  /// \brief The on-disk hash table used for the DeclContext's Name lookup table.
-  typedef llvm::OnDiskIterableChainedHashTable<ASTDeclContextNameLookupTrait>
-    ASTDeclContextNameLookupTable;
+  /// \brief The on-disk hash table(s) used for DeclContext name lookup.
+  struct DeclContextLookupTable;
 }
 
 } // end namespace serialization
@@ -507,6 +506,10 @@ private:
   /// \brief Map from the TU to its lexical contents from each module file.
   std::vector<std::pair<ModuleFile*, LexicalContents>> TULexicalDecls;
 
+  /// \brief Map from a DeclContext to its lookup tables.
+  llvm::DenseMap<const DeclContext *,
+                 serialization::reader::DeclContextLookupTable> Lookups;
+
   // Updates for visible decls can occur for other contexts than just the
   // TU, and when we read those update records, the actual context may not
   // be available yet, so have this pending map using the ID as a key. It
@@ -514,7 +517,6 @@ private:
   struct PendingVisibleUpdate {
     ModuleFile *Mod;
     const unsigned char *Data;
-    unsigned BucketOffset;
   };
   typedef SmallVector<PendingVisibleUpdate, 1> DeclContextVisibleUpdates;
 
@@ -1089,6 +1091,10 @@ public:
         Visit(GetExistingDecl(ID));
   }
 
+  /// \brief Get the loaded lookup tables for \p Primary, if any.
+  const serialization::reader::DeclContextLookupTable *
+  getLoadedLookupTables(DeclContext *Primary) const;
+
 private:
   struct ImportedModule {
     ModuleFile *Mod;
@@ -1870,6 +1876,13 @@ public:
   /// Note: overrides method in ExternalASTSource
   Module *getModule(unsigned ID) override;
 
+  /// \brief Retrieve the module file with a given local ID within the specified
+  /// ModuleFile.
+  ModuleFile *getLocalModuleFile(ModuleFile &M, unsigned ID);
+
+  /// \brief Get an ID for the given module file.
+  unsigned getModuleFileID(ModuleFile *M);
+
   /// \brief Return a descriptor for the corresponding module.
   llvm::Optional<ASTSourceDescriptor> getSourceDescriptor(unsigned ID) override;
   /// \brief Return a descriptor for the module.

Modified: cfe/trunk/include/clang/Serialization/ASTWriter.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTWriter.h?rev=246582&r1=246581&r2=246582&view=diff
==============================================================================
--- cfe/trunk/include/clang/Serialization/ASTWriter.h (original)
+++ cfe/trunk/include/clang/Serialization/ASTWriter.h Tue Sep  1 15:35:42 2015
@@ -529,8 +529,8 @@ private:
   bool isLookupResultExternal(StoredDeclsList &Result, DeclContext *DC);
   bool isLookupResultEntirelyExternal(StoredDeclsList &Result, DeclContext *DC);
 
-  uint32_t GenerateNameLookupTable(const DeclContext *DC,
-                                   llvm::SmallVectorImpl<char> &LookupTable);
+  void GenerateNameLookupTable(const DeclContext *DC,
+                               llvm::SmallVectorImpl<char> &LookupTable);
   uint64_t WriteDeclContextLexicalBlock(ASTContext &Context, DeclContext *DC);
   uint64_t WriteDeclContextVisibleBlock(ASTContext &Context, DeclContext *DC);
   void WriteTypeDeclOffsets();
@@ -849,6 +849,7 @@ public:
   unsigned getExprImplicitCastAbbrev() const { return ExprImplicitCastAbbrev; }
 
   bool hasChain() const { return Chain; }
+  ASTReader *getChain() const { return Chain; }
 
   // ASTDeserializationListener implementation
   void ReaderInitialized(ASTReader *Reader) override;

Modified: cfe/trunk/include/clang/Serialization/Module.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/Module.h?rev=246582&r1=246581&r2=246582&view=diff
==============================================================================
--- cfe/trunk/include/clang/Serialization/Module.h (original)
+++ cfe/trunk/include/clang/Serialization/Module.h Tue Sep  1 15:35:42 2015
@@ -50,14 +50,6 @@ enum ModuleKind {
   MK_MainFile        ///< File is a PCH file treated as the actual main file.
 };
 
-/// \brief Information about the contents of a DeclContext.
-struct DeclContextInfo {
-  DeclContextInfo() : NameLookupTableData() {}
-
-  llvm::OnDiskIterableChainedHashTable<reader::ASTDeclContextNameLookupTrait>
-    *NameLookupTableData; // an ASTDeclContextNameLookupTable.
-};
-
 /// \brief The input file that has been loaded from this AST file, along with
 /// bools indicating whether this was an overridden buffer or if it was
 /// out-of-date or not-found.
@@ -416,13 +408,6 @@ public:
   /// indexed by the C++ ctor initializer list ID minus 1.
   const uint32_t *CXXCtorInitializersOffsets;
 
-  typedef llvm::DenseMap<const DeclContext *, DeclContextInfo>
-  DeclContextInfosMap;
-
-  /// \brief Information about the lexical and visible declarations
-  /// for each DeclContext.
-  DeclContextInfosMap DeclContextInfos;
-
   /// \brief Array of file-level DeclIDs sorted by file.
   const serialization::DeclID *FileSortedDecls;
   unsigned NumFileSortedDecls;

Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=246582&r1=246581&r2=246582&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Tue Sep  1 15:35:42 2015
@@ -20,6 +20,7 @@
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/Frontend/PCHContainerOperations.h"
+#include "clang/AST/ASTMutationListener.h"
 #include "clang/AST/NestedNameSpecifier.h"
 #include "clang/AST/Type.h"
 #include "clang/AST/TypeLocVisitor.h"
@@ -903,6 +904,13 @@ unsigned DeclarationNameKey::getHash() c
   return ID.ComputeHash();
 }
 
+ModuleFile *
+ASTDeclContextNameLookupTrait::ReadFileRef(const unsigned char *&d) {
+  using namespace llvm::support;
+  uint32_t ModuleFileID = endian::readNext<uint32_t, little, unaligned>(d);
+  return Reader.getLocalModuleFile(F, ModuleFileID);
+}
+
 std::pair<unsigned, unsigned>
 ASTDeclContextNameLookupTrait::ReadKeyDataLength(const unsigned char *&d) {
   using namespace llvm::support;
@@ -948,15 +956,15 @@ ASTDeclContextNameLookupTrait::ReadKey(c
   return DeclarationNameKey(Kind, Data);
 }
 
-ASTDeclContextNameLookupTrait::data_type
-ASTDeclContextNameLookupTrait::ReadData(internal_key_type,
-                                        const unsigned char *d,
-                                        unsigned DataLen) {
+void ASTDeclContextNameLookupTrait::ReadDataInto(internal_key_type,
+                                                 const unsigned char *d,
+                                                 unsigned DataLen,
+                                                 data_type_builder &Val) {
   using namespace llvm::support;
-  unsigned NumDecls = DataLen / 4;
-  LE32DeclID *Start = reinterpret_cast<LE32DeclID *>(
-                        const_cast<unsigned char *>(d));
-  return std::make_pair(Start, Start + NumDecls);
+  for (unsigned NumDecls = DataLen / 4; NumDecls; --NumDecls) {
+    uint32_t LocalID = endian::readNext<uint32_t, little, unaligned>(d);
+    Val.insert(Reader.getGlobalDeclID(F, LocalID));
+  }
 }
 
 bool ASTReader::ReadLexicalDeclContextStorage(ModuleFile &M,
@@ -1015,9 +1023,8 @@ bool ASTReader::ReadVisibleDeclContextSt
 
   // We can't safely determine the primary context yet, so delay attaching the
   // lookup table until we're done with recursive deserialization.
-  unsigned BucketOffset = Record[0];
-  PendingVisibleUpdates[ID].push_back(PendingVisibleUpdate{
-      &M, (const unsigned char *)Blob.data(), BucketOffset});
+  auto *Data = (const unsigned char*)Blob.data();
+  PendingVisibleUpdates[ID].push_back(PendingVisibleUpdate{&M, Data});
   return false;
 }
 
@@ -2552,9 +2559,7 @@ ASTReader::ReadASTBlock(ModuleFile &F, u
       unsigned Idx = 0;
       serialization::DeclID ID = ReadDeclID(F, Record, Idx);
       auto *Data = (const unsigned char*)Blob.data();
-      unsigned BucketOffset = Record[Idx++];
-      PendingVisibleUpdates[ID].push_back(
-          PendingVisibleUpdate{&F, Data, BucketOffset});
+      PendingVisibleUpdates[ID].push_back(PendingVisibleUpdate{&F, Data});
       // If we've already loaded the decl, perform the updates when we finish
       // loading this block.
       if (Decl *D = GetExistingDecl(ID))
@@ -6355,196 +6360,48 @@ void ASTReader::FindFileRegionDecls(File
     Decls.push_back(GetDecl(getGlobalDeclID(*DInfo.Mod, *DIt)));
 }
 
-/// \brief Retrieve the "definitive" module file for the definition of the
-/// given declaration context, if there is one.
-///
-/// The "definitive" module file is the only place where we need to look to
-/// find information about the declarations within the given declaration
-/// context. For example, C++ and Objective-C classes, C structs/unions, and
-/// Objective-C protocols, categories, and extensions are all defined in a
-/// single place in the source code, so they have definitive module files
-/// associated with them. C++ namespaces, on the other hand, can have
-/// definitions in multiple different module files.
-///
-/// Note: this needs to be kept in sync with ASTWriter::AddedVisibleDecl's
-/// NDEBUG checking.
-static ModuleFile *getDefinitiveModuleFileFor(const DeclContext *DC,
-                                              ASTReader &Reader) {
-  if (const DeclContext *DefDC = getDefinitiveDeclContext(DC))
-    return Reader.getOwningModuleFile(cast<Decl>(DefDC));
-
-  return nullptr;
-}
-
-namespace {
-  /// \brief ModuleFile visitor used to perform name lookup into a
-  /// declaration context.
-  class DeclContextNameLookupVisitor {
-    ASTReader &Reader;
-    const DeclContext *Context;
-    DeclarationName Name;
-    DeclarationNameKey NameKey;
-    unsigned NameHash;
-    SmallVectorImpl<NamedDecl *> &Decls;
-    llvm::SmallPtrSetImpl<NamedDecl *> &DeclSet;
-
-  public:
-    DeclContextNameLookupVisitor(ASTReader &Reader, const DeclContext *Context,
-                                 DeclarationName Name,
-                                 SmallVectorImpl<NamedDecl *> &Decls,
-                                 llvm::SmallPtrSetImpl<NamedDecl *> &DeclSet)
-        : Reader(Reader), Context(Context), Name(Name), NameKey(Name),
-          NameHash(NameKey.getHash()), Decls(Decls), DeclSet(DeclSet) {}
-
-    bool operator()(ModuleFile &M) {
-      // Check whether we have any visible declaration information for
-      // this context in this module.
-      auto Info = M.DeclContextInfos.find(Context);
-      if (Info == M.DeclContextInfos.end() || !Info->second.NameLookupTableData)
-        return false;
-
-      // Look for this name within this module.
-      ASTDeclContextNameLookupTable *LookupTable =
-          Info->second.NameLookupTableData;
-      ASTDeclContextNameLookupTable::iterator Pos =
-          LookupTable->find_hashed(NameKey, NameHash);
-      if (Pos == LookupTable->end())
-        return false;
-
-      bool FoundAnything = false;
-      ASTDeclContextNameLookupTrait::data_type Data = *Pos;
-      for (; Data.first != Data.second; ++Data.first) {
-        NamedDecl *ND = Reader.GetLocalDeclAs<NamedDecl>(M, *Data.first);
-        if (!ND)
-          continue;
-
-        if (ND->getDeclName() != Name) {
-          // Not all names map to a unique DeclarationNameKey.
-          assert(DeclarationNameKey(ND->getDeclName()) == NameKey &&
-                 "mismatched name for decl in decl context lookup table?");
-          continue;
-        }
-
-        // Record this declaration.
-        FoundAnything = true;
-        if (DeclSet.insert(ND).second)
-          Decls.push_back(ND);
-      }
-
-      return FoundAnything;
-    }
-  };
-}
-
 bool
 ASTReader::FindExternalVisibleDeclsByName(const DeclContext *DC,
                                           DeclarationName Name) {
-  assert(DC->hasExternalVisibleStorage() &&
+  assert(DC->hasExternalVisibleStorage() && DC == DC->getPrimaryContext() &&
          "DeclContext has no visible decls in storage");
   if (!Name)
     return false;
 
+  auto It = Lookups.find(DC);
+  if (It == Lookups.end())
+    return false;
+
   Deserializing LookupResults(this);
 
+  // Load the list of declarations.
   SmallVector<NamedDecl *, 64> Decls;
-  llvm::SmallPtrSet<NamedDecl*, 64> DeclSet;
-
-  DeclContextNameLookupVisitor Visitor(*this, DC, Name, Decls, DeclSet);
-
-  // If we can definitively determine which module file to look into,
-  // only look there. Otherwise, look in all module files.
-  if (ModuleFile *Definitive = getDefinitiveModuleFileFor(DC, *this))
-    Visitor(*Definitive);
-  else
-    ModuleMgr.visit(Visitor);
+  for (DeclID ID : It->second.Table.find(Name)) {
+    NamedDecl *ND = cast<NamedDecl>(GetDecl(ID));
+    if (ND->getDeclName() == Name)
+      Decls.push_back(ND);
+  }
 
   ++NumVisibleDeclContextsRead;
   SetExternalVisibleDeclsForName(DC, Name, Decls);
   return !Decls.empty();
 }
 
-namespace {
-  /// \brief ModuleFile visitor used to retrieve all visible names in a
-  /// declaration context.
-  class DeclContextAllNamesVisitor {
-    ASTReader &Reader;
-    SmallVectorImpl<const DeclContext *> &Contexts;
-    DeclsMap &Decls;
-    llvm::SmallPtrSet<NamedDecl *, 256> DeclSet;
-    bool VisitAll;
-
-  public:
-    DeclContextAllNamesVisitor(ASTReader &Reader,
-                               SmallVectorImpl<const DeclContext *> &Contexts,
-                               DeclsMap &Decls, bool VisitAll)
-      : Reader(Reader), Contexts(Contexts), Decls(Decls), VisitAll(VisitAll) { }
-
-    bool operator()(ModuleFile &M) {
-      // Check whether we have any visible declaration information for
-      // this context in this module.
-      ModuleFile::DeclContextInfosMap::iterator Info;
-      bool FoundInfo = false;
-      for (unsigned I = 0, N = Contexts.size(); I != N; ++I) {
-        Info = M.DeclContextInfos.find(Contexts[I]);
-        if (Info != M.DeclContextInfos.end() &&
-            Info->second.NameLookupTableData) {
-          FoundInfo = true;
-          break;
-        }
-      }
-
-      if (!FoundInfo)
-        return false;
-
-      ASTDeclContextNameLookupTable *LookupTable =
-        Info->second.NameLookupTableData;
-      bool FoundAnything = false;
-      for (ASTDeclContextNameLookupTable::data_iterator
-             I = LookupTable->data_begin(), E = LookupTable->data_end();
-           I != E;
-           ++I) {
-        ASTDeclContextNameLookupTrait::data_type Data = *I;
-        for (; Data.first != Data.second; ++Data.first) {
-          NamedDecl *ND = Reader.GetLocalDeclAs<NamedDecl>(M, *Data.first);
-          if (!ND)
-            continue;
-
-          // Record this declaration.
-          FoundAnything = true;
-          if (DeclSet.insert(ND).second)
-            Decls[ND->getDeclName()].push_back(ND);
-        }
-      }
-
-      return FoundAnything && !VisitAll;
-    }
-  };
-}
-
 void ASTReader::completeVisibleDeclsMap(const DeclContext *DC) {
   if (!DC->hasExternalVisibleStorage())
     return;
+
+  auto It = Lookups.find(DC);
+  assert(It != Lookups.end() &&
+         "have external visible storage but no lookup tables");
+
   DeclsMap Decls;
 
-  // Compute the declaration contexts we need to look into. Multiple such
-  // declaration contexts occur when two declaration contexts from disjoint
-  // modules get merged, e.g., when two namespaces with the same name are
-  // independently defined in separate modules.
-  SmallVector<const DeclContext *, 2> Contexts;
-  Contexts.push_back(DC);
-
-  if (DC->isNamespace()) {
-    KeyDeclsMap::iterator Key =
-        KeyDecls.find(const_cast<Decl *>(cast<Decl>(DC)));
-    if (Key != KeyDecls.end()) {
-      for (unsigned I = 0, N = Key->second.size(); I != N; ++I)
-        Contexts.push_back(cast<DeclContext>(GetDecl(Key->second[I])));
-    }
+  for (DeclID ID : It->second.Table.findAll()) {
+    NamedDecl *ND = cast<NamedDecl>(GetDecl(ID));
+    Decls[ND->getDeclName()].push_back(ND);
   }
 
-  DeclContextAllNamesVisitor Visitor(*this, Contexts, Decls,
-                                     /*VisitAll=*/DC->isFileContext());
-  ModuleMgr.visit(Visitor);
   ++NumVisibleDeclContextsRead;
 
   for (DeclsMap::iterator I = Decls.begin(), E = Decls.end(); I != E; ++I) {
@@ -6553,6 +6410,12 @@ void ASTReader::completeVisibleDeclsMap(
   const_cast<DeclContext *>(DC)->setHasExternalVisibleStorage(false);
 }
 
+const serialization::reader::DeclContextLookupTable *
+ASTReader::getLoadedLookupTables(DeclContext *Primary) const {
+  auto I = Lookups.find(Primary);
+  return I == Lookups.end() ? nullptr : &I->second;
+}
+
 /// \brief Under non-PCH compilation the consumer receives the objc methods
 /// before receiving the implementation, and codegen depends on this.
 /// We simulate this by deserializing and passing to consumer the methods of the
@@ -7384,6 +7247,36 @@ Module *ASTReader::getModule(unsigned ID
   return getSubmodule(ID);
 }
 
+ModuleFile *ASTReader::getLocalModuleFile(ModuleFile &F, unsigned ID) {
+  if (ID & 1) {
+    // It's a module, look it up by submodule ID.
+    auto I = GlobalSubmoduleMap.find(getGlobalSubmoduleID(F, ID >> 1));
+    return I == GlobalSubmoduleMap.end() ? nullptr : I->second;
+  } else {
+    // It's a prefix (preamble, PCH, ...). Look it up by index.
+    unsigned IndexFromEnd = ID >> 1;
+    assert(IndexFromEnd && "got reference to unknown module file");
+    return getModuleManager().pch_modules().end()[-IndexFromEnd];
+  }
+}
+
+unsigned ASTReader::getModuleFileID(ModuleFile *F) {
+  if (!F)
+    return 1;
+
+  // For a file representing a module, use the submodule ID of the top-level
+  // module as the file ID. For any other kind of file, the number of such
+  // files loaded beforehand will be the same on reload.
+  // FIXME: Is this true even if we have an explicit module file and a PCH?
+  if (F->isModule())
+    return ((F->BaseSubmoduleID + NUM_PREDEF_SUBMODULE_IDS) << 1) | 1;
+
+  auto PCHModules = getModuleManager().pch_modules();
+  auto I = std::find(PCHModules.begin(), PCHModules.end(), F);
+  assert(I != PCHModules.end() && "emitting reference to unknown file");
+  return (I - PCHModules.end()) << 1;
+}
+
 ExternalASTSource::ASTSourceDescriptor
 ASTReader::getSourceDescriptor(const Module &M) {
   StringRef Dir, Filename;
@@ -8432,6 +8325,8 @@ void ASTReader::FinishedDeserializing()
       for (auto Update : Updates) {
         auto *FPT = Update.second->getType()->castAs<FunctionProtoType>();
         auto ESI = FPT->getExtProtoInfo().ExceptionSpec;
+        if (auto *Listener = Context.getASTMutationListener())
+          Listener->ResolvedExceptionSpec(cast<FunctionDecl>(Update.second));
         for (auto *Redecl : Update.second->redecls())
           Context.adjustExceptionSpec(cast<FunctionDecl>(Redecl), ESI);
       }

Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=246582&r1=246581&r2=246582&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Tue Sep  1 15:35:42 2015
@@ -1489,6 +1489,8 @@ void ASTDeclReader::MergeDefinitionData(
     Reader.PendingDefinitions.erase(MergeDD.Definition);
     MergeDD.Definition->IsCompleteDefinition = false;
     mergeDefinitionVisibility(DD.Definition, MergeDD.Definition);
+    assert(Reader.Lookups.find(MergeDD.Definition) == Reader.Lookups.end() &&
+           "already loaded pending lookups for merged definition");
   }
 
   auto PFDI = Reader.PendingFakeDefinitionData.find(&DD);
@@ -3346,15 +3348,10 @@ void ASTReader::loadDeclUpdateRecords(se
     PendingVisibleUpdates.erase(I);
 
     auto *DC = cast<DeclContext>(D)->getPrimaryContext();
-    for (const PendingVisibleUpdate &Update : VisibleUpdates) {
-      auto *&LookupTable = Update.Mod->DeclContextInfos[DC].NameLookupTableData;
-      assert(!LookupTable && "multiple lookup tables for DC in module");
-      LookupTable = reader::ASTDeclContextNameLookupTable::Create(
-          Update.Data + Update.BucketOffset,
-          Update.Data + sizeof(uint32_t),
-          Update.Data,
+    for (const PendingVisibleUpdate &Update : VisibleUpdates)
+      Lookups[DC].Table.add(
+          Update.Mod, Update.Data,
           reader::ASTDeclContextNameLookupTrait(*this, *Update.Mod));
-    }
     DC->setHasExternalVisibleStorage(true);
   }
 

Modified: cfe/trunk/lib/Serialization/ASTReaderInternals.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderInternals.h?rev=246582&r1=246581&r2=246582&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReaderInternals.h (original)
+++ cfe/trunk/lib/Serialization/ASTReaderInternals.h Tue Sep  1 15:35:42 2015
@@ -15,8 +15,12 @@
 
 #include "clang/AST/DeclarationName.h"
 #include "clang/Serialization/ASTBitCodes.h"
+#include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/PointerUnion.h"
+#include "llvm/ADT/TinyPtrVector.h"
 #include "llvm/Support/Endian.h"
 #include "llvm/Support/OnDiskHashTable.h"
+#include "MultiOnDiskHashTable.h"
 #include <utility>
 
 namespace clang {
@@ -39,14 +43,38 @@ class ASTDeclContextNameLookupTrait {
   ModuleFile &F;
   
 public:
-  /// \brief Pair of begin/end iterators for DeclIDs.
-  ///
-  /// Note that these declaration IDs are local to the module that contains this
-  /// particular lookup t
-  typedef llvm::support::ulittle32_t LE32DeclID;
-  typedef std::pair<LE32DeclID *, LE32DeclID *> data_type;
+  // Maximum number of lookup tables we allow before condensing the tables.
+  static const int MaxTables = 4;
+
+  /// The lookup result is a list of global declaration IDs.
+  typedef llvm::SmallVector<DeclID, 4> data_type;
+  struct data_type_builder {
+    data_type &Data;
+    llvm::DenseSet<DeclID> Found;
+
+    data_type_builder(data_type &D) : Data(D) {}
+    void insert(DeclID ID) {
+      // Just use a linear scan unless we have more than a few IDs.
+      if (Found.empty() && !Data.empty()) {
+        if (Data.size() <= 4) {
+          for (auto I : Found)
+            if (I == ID)
+              return;
+          Data.push_back(ID);
+          return;
+        }
+
+        // Switch to tracking found IDs in the set.
+        Found.insert(Data.begin(), Data.end());
+      }
+
+      if (Found.insert(ID).second)
+        Data.push_back(ID);
+    }
+  };
   typedef unsigned hash_value_type;
   typedef unsigned offset_type;
+  typedef ModuleFile *file_type;
 
   typedef DeclarationName external_key_type;
   typedef DeclarationNameKey internal_key_type;
@@ -54,8 +82,7 @@ public:
   explicit ASTDeclContextNameLookupTrait(ASTReader &Reader, ModuleFile &F)
     : Reader(Reader), F(F) { }
 
-  static bool EqualKey(const internal_key_type& a,
-                       const internal_key_type& b) {
+  static bool EqualKey(const internal_key_type &a, const internal_key_type &b) {
     return a == b;
   }
 
@@ -71,8 +98,31 @@ public:
 
   internal_key_type ReadKey(const unsigned char *d, unsigned);
 
-  data_type ReadData(internal_key_type, const unsigned char *d,
-                     unsigned DataLen);
+  void ReadDataInto(internal_key_type, const unsigned char *d,
+                    unsigned DataLen, data_type_builder &Val);
+
+  static void MergeDataInto(const data_type &From, data_type_builder &To) {
+    To.Data.reserve(To.Data.size() + From.size());
+    for (DeclID ID : From)
+      To.insert(ID);
+  }
+
+  file_type ReadFileRef(const unsigned char *&d);
+};
+
+struct DeclContextLookupTable {
+  MultiOnDiskHashTable<ASTDeclContextNameLookupTrait> Table;
+
+  // These look redundant, but don't remove them -- they work around MSVC 2013's
+  // inability to synthesize move operations. Without them, the
+  // MultiOnDiskHashTable will be copied (despite being move-only!).
+  DeclContextLookupTable() : Table() {}
+  DeclContextLookupTable(DeclContextLookupTable &&O)
+      : Table(std::move(O.Table)) {}
+  DeclContextLookupTable &operator=(DeclContextLookupTable &&O) {
+    Table = std::move(O.Table);
+    return *this;
+  }
 };
 
 /// \brief Base class for the trait describing the on-disk hash table for the

Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=246582&r1=246581&r2=246582&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTWriter.cpp Tue Sep  1 15:35:42 2015
@@ -13,6 +13,8 @@
 
 #include "clang/Serialization/ASTWriter.h"
 #include "ASTCommon.h"
+#include "ASTReaderInternals.h"
+#include "MultiOnDiskHashTable.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclContextInternals.h"
@@ -3377,12 +3379,14 @@ namespace {
 // Trait used for the on-disk hash table used in the method pool.
 class ASTDeclContextNameLookupTrait {
   ASTWriter &Writer;
+  llvm::SmallVector<DeclID, 64> DeclIDs;
 
 public:
   typedef DeclarationNameKey key_type;
   typedef key_type key_type_ref;
 
-  typedef DeclContext::lookup_result data_type;
+  /// A start and end index into DeclIDs, representing a sequence of decls.
+  typedef std::pair<unsigned, unsigned> data_type;
   typedef const data_type& data_type_ref;
 
   typedef unsigned hash_value_type;
@@ -3390,10 +3394,40 @@ public:
 
   explicit ASTDeclContextNameLookupTrait(ASTWriter &Writer) : Writer(Writer) { }
 
+  template<typename Coll>
+  data_type getData(const Coll &Decls) {
+    unsigned Start = DeclIDs.size();
+    for (NamedDecl *D : Decls) {
+      DeclIDs.push_back(
+          Writer.GetDeclRef(getDeclForLocalLookup(Writer.getLangOpts(), D)));
+    }
+    return std::make_pair(Start, DeclIDs.size());
+  }
+
+  data_type ImportData(const reader::ASTDeclContextNameLookupTrait::data_type &FromReader) {
+    unsigned Start = DeclIDs.size();
+    for (auto ID : FromReader)
+      DeclIDs.push_back(ID);
+    return std::make_pair(Start, DeclIDs.size());
+  }
+
+  static bool EqualKey(key_type_ref a, key_type_ref b) {
+    return a == b;
+  }
+
   hash_value_type ComputeHash(DeclarationNameKey Name) {
     return Name.getHash();
   }
 
+  void EmitFileRef(raw_ostream &Out, ModuleFile *F) const {
+    assert(Writer.hasChain() &&
+           "have reference to loaded module file but no chain?");
+
+    using namespace llvm::support;
+    endian::Writer<little>(Out)
+        .write<uint32_t>(Writer.getChain()->getModuleFileID(F));
+  }
+
   std::pair<unsigned, unsigned> EmitKeyDataLength(raw_ostream &Out,
                                                   DeclarationNameKey Name,
                                                   data_type_ref Lookup) {
@@ -3420,7 +3454,9 @@ public:
     LE.write<uint16_t>(KeyLen);
 
     // 4 bytes for each DeclID.
-    unsigned DataLen = 4 * Lookup.size();
+    unsigned DataLen = 4 * (Lookup.second - Lookup.first);
+    assert(uint16_t(DataLen) == DataLen &&
+           "too many decls for serialized lookup result");
     LE.write<uint16_t>(DataLen);
 
     return std::make_pair(KeyLen, DataLen);
@@ -3460,11 +3496,8 @@ public:
     using namespace llvm::support;
     endian::Writer<little> LE(Out);
     uint64_t Start = Out.tell(); (void)Start;
-    for (DeclContext::lookup_iterator I = Lookup.begin(), E = Lookup.end();
-         I != E; ++I)
-      LE.write<uint32_t>(
-          Writer.GetDeclRef(getDeclForLocalLookup(Writer.getLangOpts(), *I)));
-
+    for (unsigned I = Lookup.first, N = Lookup.second; I != N; ++I)
+      LE.write<uint32_t>(DeclIDs[I]);
     assert(Out.tell() - Start == DataLen && "Data length is wrong");
   }
 };
@@ -3484,7 +3517,7 @@ bool ASTWriter::isLookupResultEntirelyEx
   return true;
 }
 
-uint32_t
+void
 ASTWriter::GenerateNameLookupTable(const DeclContext *ConstDC,
                                    llvm::SmallVectorImpl<char> &LookupTable) {
   assert(!ConstDC->HasLazyLocalLexicalLookups &&
@@ -3496,8 +3529,8 @@ ASTWriter::GenerateNameLookupTable(const
   assert(DC == DC->getPrimaryContext() && "only primary DC has lookup table");
 
   // Create the on-disk hash table representation.
-  llvm::OnDiskChainedHashTableGenerator<ASTDeclContextNameLookupTrait>
-      Generator;
+  MultiOnDiskHashTableGenerator<reader::ASTDeclContextNameLookupTrait,
+                                ASTDeclContextNameLookupTrait> Generator;
   ASTDeclContextNameLookupTrait Trait(*this);
 
   // The first step is to collect the declaration names which we need to
@@ -3632,7 +3665,7 @@ ASTWriter::GenerateNameLookupTable(const
 
     switch (Name.getNameKind()) {
     default:
-      Generator.insert(Name, Result, Trait);
+      Generator.insert(Name, Trait.getData(Result), Trait);
       break;
 
     case DeclarationName::CXXConstructorName:
@@ -3650,17 +3683,15 @@ ASTWriter::GenerateNameLookupTable(const
   // the key, only the kind of name is used.
   if (!ConstructorDecls.empty())
     Generator.insert(ConstructorDecls.front()->getDeclName(),
-                     DeclContext::lookup_result(ConstructorDecls), Trait);
+                     Trait.getData(ConstructorDecls), Trait);
   if (!ConversionDecls.empty())
     Generator.insert(ConversionDecls.front()->getDeclName(),
-                     DeclContext::lookup_result(ConversionDecls), Trait);
+                     Trait.getData(ConversionDecls), Trait);
 
-  // Create the on-disk hash table in a buffer.
-  llvm::raw_svector_ostream Out(LookupTable);
-  // Make sure that no bucket is at offset 0
-  using namespace llvm::support;
-  endian::Writer<little>(Out).write<uint32_t>(0);
-  return Generator.Emit(Out, Trait);
+  // Create the on-disk hash table. Also emit the existing imported and
+  // merged table if there is one.
+  auto *Lookups = Chain ? Chain->getLoadedLookupTables(DC) : nullptr;
+  Generator.emit(LookupTable, Trait, Lookups ? &Lookups->Table : nullptr);
 }
 
 /// \brief Write the block containing all of the declaration IDs
@@ -3743,12 +3774,11 @@ uint64_t ASTWriter::WriteDeclContextVisi
 
   // Create the on-disk hash table in a buffer.
   SmallString<4096> LookupTable;
-  uint32_t BucketOffset = GenerateNameLookupTable(DC, LookupTable);
+  GenerateNameLookupTable(DC, LookupTable);
 
   // Write the lookup table
   RecordData Record;
   Record.push_back(DECL_CONTEXT_VISIBLE);
-  Record.push_back(BucketOffset);
   Stream.EmitRecordWithBlob(DeclContextVisibleLookupAbbrev, Record,
                             LookupTable);
   ++NumVisibleDeclContexts;
@@ -3771,7 +3801,7 @@ void ASTWriter::WriteDeclContextVisibleU
 
   // Create the on-disk hash table in a buffer.
   SmallString<4096> LookupTable;
-  uint32_t BucketOffset = GenerateNameLookupTable(DC, LookupTable);
+  GenerateNameLookupTable(DC, LookupTable);
 
   // If we're updating a namespace, select a key declaration as the key for the
   // update record; those are the only ones that will be checked on reload.
@@ -3782,7 +3812,6 @@ void ASTWriter::WriteDeclContextVisibleU
   RecordData Record;
   Record.push_back(UPDATE_VISIBLE);
   Record.push_back(getDeclID(cast<Decl>(DC)));
-  Record.push_back(BucketOffset);
   Stream.EmitRecordWithBlob(UpdateVisibleAbbrev, Record, LookupTable);
 }
 
@@ -4246,7 +4275,6 @@ void ASTWriter::WriteASTCore(Sema &SemaR
   Abv = new llvm::BitCodeAbbrev();
   Abv->Add(llvm::BitCodeAbbrevOp(UPDATE_VISIBLE));
   Abv->Add(llvm::BitCodeAbbrevOp(llvm::BitCodeAbbrevOp::VBR, 6));
-  Abv->Add(llvm::BitCodeAbbrevOp(llvm::BitCodeAbbrevOp::Fixed, 32));
   Abv->Add(llvm::BitCodeAbbrevOp(llvm::BitCodeAbbrevOp::Blob));
   UpdateVisibleAbbrev = Stream.EmitAbbrev(Abv);
   WriteDeclContextVisibleUpdate(TU);

Modified: cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriterDecl.cpp?rev=246582&r1=246581&r2=246582&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTWriterDecl.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTWriterDecl.cpp Tue Sep  1 15:35:42 2015
@@ -2049,7 +2049,6 @@ void ASTWriter::WriteDeclAbbrevs() {
 
   Abv = new BitCodeAbbrev();
   Abv->Add(BitCodeAbbrevOp(serialization::DECL_CONTEXT_VISIBLE));
-  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 32));
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob));
   DeclContextVisibleLookupAbbrev = Stream.EmitAbbrev(Abv);
 }

Modified: cfe/trunk/lib/Serialization/Module.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/Module.cpp?rev=246582&r1=246581&r2=246582&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/Module.cpp (original)
+++ cfe/trunk/lib/Serialization/Module.cpp Tue Sep  1 15:35:42 2015
@@ -45,13 +45,6 @@ ModuleFile::ModuleFile(ModuleKind Kind,
 {}
 
 ModuleFile::~ModuleFile() {
-  for (DeclContextInfosMap::iterator I = DeclContextInfos.begin(),
-       E = DeclContextInfos.end();
-       I != E; ++I) {
-    if (I->second.NameLookupTableData)
-      delete I->second.NameLookupTableData;
-  }
-  
   delete static_cast<ASTIdentifierLookupTable *>(IdentifierLookupTable);
   delete static_cast<HeaderFileInfoLookupTable *>(HeaderFileInfoTable);
   delete static_cast<ASTSelectorLookupTable *>(SelectorLookupTable);

Copied: cfe/trunk/lib/Serialization/MultiOnDiskHashTable.h (from r246545, cfe/trunk/lib/Serialization/MultiOnDiskHashTable.h)
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/MultiOnDiskHashTable.h?p2=cfe/trunk/lib/Serialization/MultiOnDiskHashTable.h&p1=cfe/trunk/lib/Serialization/MultiOnDiskHashTable.h&r1=246545&r2=246582&rev=246582&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/MultiOnDiskHashTable.h (original)
+++ cfe/trunk/lib/Serialization/MultiOnDiskHashTable.h Tue Sep  1 15:35:42 2015
@@ -102,20 +102,27 @@ private:
 
   /// \brief Delete all our current on-disk tables.
   void clear() {
-    if (auto *M = getMergedTable())
-      delete M;
     for (auto *T : tables())
       delete T;
+    if (auto *M = getMergedTable())
+      delete M;
+    Tables.clear();
   }
 
   void removeOverriddenTables() {
     llvm::DenseSet<file_type> Files;
     Files.insert(PendingOverrides.begin(), PendingOverrides.end());
-    Tables.erase(
-        std::remove_if(tables().begin().getCurrent(), Tables.end(), [&](void *T) -> bool {
-          auto *ODT = Table::getFromOpaqueValue(T).template get<OnDiskTable*>();
-          return Files.count(ODT->File);
-        }), Tables.end());
+    // Explicitly capture Files to work around an MSVC 2015 rejects-valid bug.
+    auto ShouldRemove = [&Files](void *T) -> bool {
+      auto *ODT = Table::getFromOpaqueValue(T).template get<OnDiskTable *>();
+      bool Remove = Files.count(ODT->File);
+      if (Remove)
+        delete ODT;
+      return Remove;
+    };
+    Tables.erase(std::remove_if(tables().begin().getCurrent(), Tables.end(),
+                                ShouldRemove),
+                 Tables.end());
     PendingOverrides.clear();
   }
 

Modified: cfe/trunk/test/Modules/cxx-templates.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/cxx-templates.cpp?rev=246582&r1=246581&r2=246582&view=diff
==============================================================================
--- cfe/trunk/test/Modules/cxx-templates.cpp (original)
+++ cfe/trunk/test/Modules/cxx-templates.cpp Tue Sep  1 15:35:42 2015
@@ -28,8 +28,8 @@ void g() {
   f<double>(1.0);
   f<int>();
   f(); // expected-error {{no matching function}}
-  // expected-note at Inputs/cxx-templates-b.h:3 {{couldn't infer template argument}}
-  // expected-note at Inputs/cxx-templates-b.h:4 {{requires single argument}}
+  // expected-note at Inputs/cxx-templates-a.h:3 {{couldn't infer template argument}}
+  // expected-note at Inputs/cxx-templates-a.h:4 {{requires 1 argument}}
 
   N::f(0);
   N::f<double>(1.0);
@@ -179,10 +179,14 @@ namespace Std {
 
 // CHECK-GLOBAL:      DeclarationName 'f'
 // CHECK-GLOBAL-NEXT: |-FunctionTemplate {{.*}} 'f'
+// CHECK-GLOBAL-NEXT: |-FunctionTemplate {{.*}} 'f'
+// CHECK-GLOBAL-NEXT: |-FunctionTemplate {{.*}} 'f'
 // CHECK-GLOBAL-NEXT: `-FunctionTemplate {{.*}} 'f'
 
 // 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

Modified: cfe/trunk/test/Modules/merge-using-decls.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/merge-using-decls.cpp?rev=246582&r1=246581&r2=246582&view=diff
==============================================================================
--- cfe/trunk/test/Modules/merge-using-decls.cpp (original)
+++ cfe/trunk/test/Modules/merge-using-decls.cpp Tue Sep  1 15:35:42 2015
@@ -31,6 +31,8 @@ template int UseAll<YA>();
 template int UseAll<YB>();
 template int UseAll<Y>();
 
+// Which of these two sets of diagnostics is chosen is not important. It's OK
+// if this varies with ORDER, but it must be consistent across runs.
 #if ORDER == 1
 // Here, we're instantiating the definition from 'A' and merging the definition
 // from 'B' into it.




More information about the cfe-commits mailing list