r241999 - [modules] Improve performance when there is a local declaration of an entity

Manuel Klimek klimek at google.com
Mon Jul 13 04:07:18 PDT 2015


On Mon, Jul 13, 2015 at 1:45 AM Richard Smith <richard-llvm at metafoo.co.uk>
wrote:

> Author: rsmith
> Date: Sun Jul 12 18:43:21 2015
> New Revision: 241999
>
> URL: http://llvm.org/viewvc/llvm-project?rev=241999&view=rev
> Log:
> [modules] Improve performance when there is a local declaration of an
> entity
> before the first imported declaration.
>
> We don't need to track all formerly-canonical declarations of an entity;
> it's sufficient to track those ones for which no other formerly-canonical
> declaration was imported into the same module. We call those ones "key
> declarations", and use them as our starting points for collecting
> redeclarations and performing namespace lookups.
>

Do you have any numbers on how much this speeds it up?


>
> Modified:
>     cfe/trunk/include/clang/Serialization/ASTReader.h
>     cfe/trunk/include/clang/Serialization/ASTWriter.h
>     cfe/trunk/lib/Serialization/ASTReader.cpp
>     cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
>     cfe/trunk/lib/Serialization/ASTWriter.cpp
>     cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
>
> Modified: cfe/trunk/include/clang/Serialization/ASTReader.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTReader.h?rev=241999&r1=241998&r2=241999&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Serialization/ASTReader.h (original)
> +++ cfe/trunk/include/clang/Serialization/ASTReader.h Sun Jul 12 18:43:21
> 2015
> @@ -976,12 +976,14 @@ private:
>        MergedLookups;
>
>    typedef llvm::DenseMap<Decl *, SmallVector<serialization::DeclID, 2> >
> -    MergedDeclsMap;
> +    KeyDeclsMap;
>
> -  /// \brief A mapping from canonical declarations to the set of
> additional
> -  /// (global, previously-canonical) declaration IDs that have been
> merged with
> -  /// that canonical declaration.
> -  MergedDeclsMap MergedDecls;
> +  /// \brief A mapping from canonical declarations to the set of global
> +  /// declaration IDs for key declaration that have been merged with that
> +  /// canonical declaration. A key declaration is a formerly-canonical
> +  /// declaration whose module did not import any other key declaration
> for that
> +  /// entity. These are the IDs that we use as keys when finding redecl
> chains.
> +  KeyDeclsMap KeyDecls;
>
>    /// \brief A mapping from DeclContexts to the semantic DeclContext that
> we
>    /// are treating as the definition of the entity. This is used, for
> instance,
> @@ -1054,6 +1056,36 @@ public:
>    void ResolveImportedPath(ModuleFile &M, std::string &Filename);
>    static void ResolveImportedPath(std::string &Filename, StringRef
> Prefix);
>
> +  /// \brief Returns the first key declaration for the given declaration.
> This
> +  /// is one that is formerly-canonical (or still canonical) and whose
> module
> +  /// did not import any other key declaration of the entity.
> +  Decl *getKeyDeclaration(Decl *D) {
>

Should the comment say "Returns the first imported key declaration"?
Otherwise it sounds like there'd be multiple key decls in the current
context.

+    D = D->getCanonicalDecl();
> +    if (D->isFromASTFile())
> +      return D;
>

I think I asked you that in person already 2 months back, and I forgot
again, but why is the isFromASTFile check enough? (I assume the invariant
is that merging makes sure that there is only one canonical decl loaded
from a module, and it's the first one? but then how do we know we never
call this during merging?)
Also, why is the canonical decl that comes from the AST file not also in
the KeyDecls?


> +
> +    auto I = KeyDecls.find(D);
> +    if (I == KeyDecls.end() || I->second.empty())
> +      return D;
> +    return GetExistingDecl(I->second[0]);
> +  }
> +  const Decl *getKeyDeclaration(const Decl *D) {
> +    return getKeyDeclaration(const_cast<Decl*>(D));
> +  }
> +
> +  /// \brief Run a callback on each imported key declaration of \p D.
> +  template <typename Fn>
> +  void forEachImportedKeyDecl(const Decl *D, Fn Visit) {
> +    D = D->getCanonicalDecl();
> +    if (D->isFromASTFile())
> +      Visit(D);
> +
> +    auto It = KeyDecls.find(const_cast<Decl*>(D));
> +    if (It != KeyDecls.end())
> +      for (auto ID : It->second)
> +        Visit(GetExistingDecl(ID));
> +  }
> +
>  private:
>    struct ImportedModule {
>      ModuleFile *Mod;
> @@ -1124,18 +1156,6 @@ private:
>    /// merged into its redecl chain.
>    Decl *getMostRecentExistingDecl(Decl *D);
>
> -  template <typename Fn>
> -  void forEachFormerlyCanonicalImportedDecl(const Decl *D, Fn Visit) {
> -    D = D->getCanonicalDecl();
> -    if (D->isFromASTFile())
> -      Visit(D);
> -
> -    auto It = MergedDecls.find(const_cast<Decl*>(D));
> -    if (It != MergedDecls.end())
> -      for (auto ID : It->second)
> -        Visit(GetExistingDecl(ID));
> -  }
> -
>    RecordLocation DeclCursorForID(serialization::DeclID ID,
>                                   unsigned &RawLocation);
>    void loadDeclUpdateRecords(serialization::DeclID ID, Decl *D);
>
> Modified: cfe/trunk/include/clang/Serialization/ASTWriter.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTWriter.h?rev=241999&r1=241998&r2=241999&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Serialization/ASTWriter.h (original)
> +++ cfe/trunk/include/clang/Serialization/ASTWriter.h Sun Jul 12 18:43:21
> 2015
> @@ -398,7 +398,7 @@ private:
>
>    /// \brief The set of declarations that may have redeclaration chains
> that
>    /// need to be serialized.
> -  llvm::SmallSetVector<Decl *, 4> Redeclarations;
> +  llvm::SmallVector<const Decl *, 16> Redeclarations;
>
>    /// \brief Statements that we've encountered while serializing a
>    /// declaration or type.
>
> Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=241999&r1=241998&r2=241999&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
> +++ cfe/trunk/lib/Serialization/ASTReader.cpp Sun Jul 12 18:43:21 2015
> @@ -6071,7 +6071,7 @@ Decl *ASTReader::GetExistingDecl(DeclID
>      if (D) {
>        // Track that we have merged the declaration with ID \p ID into the
>        // pre-existing predefined declaration \p D.
> -      auto &Merged = MergedDecls[D->getCanonicalDecl()];
> +      auto &Merged = KeyDecls[D->getCanonicalDecl()];
>        if (Merged.empty())
>          Merged.push_back(ID);
>      }
> @@ -6413,10 +6413,10 @@ ASTReader::FindExternalVisibleDeclsByNam
>    Contexts.push_back(DC);
>
>    if (DC->isNamespace()) {
> -    auto Merged = MergedDecls.find(const_cast<Decl *>(cast<Decl>(DC)));
> -    if (Merged != MergedDecls.end()) {
> -      for (unsigned I = 0, N = Merged->second.size(); I != N; ++I)
> -        Contexts.push_back(cast<DeclContext>(GetDecl(Merged->second[I])));
> +    auto 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])));
>      }
>    }
>
> @@ -6533,11 +6533,11 @@ void ASTReader::completeVisibleDeclsMap(
>    Contexts.push_back(DC);
>
>    if (DC->isNamespace()) {
> -    MergedDeclsMap::iterator Merged
> -      = MergedDecls.find(const_cast<Decl *>(cast<Decl>(DC)));
> -    if (Merged != MergedDecls.end()) {
> -      for (unsigned I = 0, N = Merged->second.size(); I != N; ++I)
> -        Contexts.push_back(cast<DeclContext>(GetDecl(Merged->second[I])));
> +    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])));
>      }
>    }
>
>
> Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=241999&r1=241998&r2=241999&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)
> +++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Sun Jul 12 18:43:21 2015
> @@ -128,20 +128,22 @@ namespace clang {
>        GlobalDeclID FirstID;
>        Decl *MergeWith;
>        mutable bool Owning;
> +      bool IsKeyDecl;
>        Decl::Kind DeclKind;
>
>        void operator=(RedeclarableResult &) = delete;
>
>      public:
>        RedeclarableResult(ASTReader &Reader, GlobalDeclID FirstID,
> -                         Decl *MergeWith, Decl::Kind DeclKind)
> +                         Decl *MergeWith, Decl::Kind DeclKind,
> +                         bool IsKeyDecl)
>          : Reader(Reader), FirstID(FirstID), MergeWith(MergeWith),
> -          Owning(true), DeclKind(DeclKind) {}
> +          Owning(true), IsKeyDecl(IsKeyDecl), DeclKind(DeclKind) {}
>
>        RedeclarableResult(RedeclarableResult &&Other)
>          : Reader(Other.Reader), FirstID(Other.FirstID),
>            MergeWith(Other.MergeWith), Owning(Other.Owning),
> -          DeclKind(Other.DeclKind) {
> +          IsKeyDecl(Other.IsKeyDecl), DeclKind(Other.DeclKind) {
>          Other.Owning = false;
>        }
>
> @@ -156,6 +158,9 @@ namespace clang {
>        /// \brief Retrieve the first ID.
>        GlobalDeclID getFirstID() const { return FirstID; }
>
> +      /// \brief Is this declaration the key declaration?
> +      bool isKeyDecl() const { return IsKeyDecl; }
> +
>        /// \brief Get a known declaration that this should be merged with,
> if
>        /// any.
>        Decl *getKnownMergeTarget() const { return MergeWith; }
> @@ -348,7 +353,7 @@ namespace clang {
>
>      void mergeTemplatePattern(RedeclarableTemplateDecl *D,
>                                RedeclarableTemplateDecl *Existing,
> -                              DeclID DsID);
> +                              DeclID DsID, bool IsKeyDecl);
>
>      ObjCTypeParamList *ReadObjCTypeParamList();
>
> @@ -2173,12 +2178,16 @@ ASTDeclReader::RedeclarableResult
>  ASTDeclReader::VisitRedeclarable(Redeclarable<T> *D) {
>    DeclID FirstDeclID = ReadDeclID(Record, Idx);
>    Decl *MergeWith = nullptr;
> +  bool IsKeyDecl = ThisDeclID == FirstDeclID;
>
>    // 0 indicates that this declaration was the only declaration of its
> entity,
>    // and is used for space optimization.
> -  if (FirstDeclID == 0)
> +  if (FirstDeclID == 0) {
>      FirstDeclID = ThisDeclID;
> -  else if (unsigned N = Record[Idx++]) {
> +    IsKeyDecl = true;
> +  } else if (unsigned N = Record[Idx++]) {
> +    IsKeyDecl = false;
> +
>      // We have some declarations that must be before us in our
> redeclaration
>      // chain. Read them now, and remember that we ought to merge with one
> of
>      // them.
> @@ -2204,7 +2213,7 @@ ASTDeclReader::VisitRedeclarable(Redecla
>    // The result structure takes care to note that we need to load the
>    // other declaration chains for this ID.
>    return RedeclarableResult(Reader, FirstDeclID, MergeWith,
> -                            static_cast<T *>(D)->getKind());
> +                            static_cast<T *>(D)->getKind(), IsKeyDecl);
>  }
>
>  /// \brief Attempts to merge the given declaration (D) with another
> declaration
> @@ -2243,11 +2252,12 @@ template<typename T> static T assert_cas
>  /// declarations.
>  void ASTDeclReader::mergeTemplatePattern(RedeclarableTemplateDecl *D,
>                                           RedeclarableTemplateDecl
> *Existing,
> -                                         DeclID DsID) {
> +                                         DeclID DsID, bool IsKeyDecl) {
>    auto *DPattern = D->getTemplatedDecl();
>    auto *ExistingPattern = Existing->getTemplatedDecl();
>    RedeclarableResult Result(Reader,
> DPattern->getCanonicalDecl()->getGlobalID(),
> -                            /*MergeWith*/ExistingPattern,
> DPattern->getKind());
> +                            /*MergeWith*/ExistingPattern,
> DPattern->getKind(),
> +                            IsKeyDecl);
>
>    if (auto *DClass = dyn_cast<CXXRecordDecl>(DPattern)) {
>      // Merge with any existing definition.
> @@ -2310,11 +2320,11 @@ void ASTDeclReader::mergeRedeclarable(Re
>      if (auto *DTemplate = dyn_cast<RedeclarableTemplateDecl>(D))
>        mergeTemplatePattern(
>            DTemplate,
> assert_cast<RedeclarableTemplateDecl*>(ExistingCanon),
> -          TemplatePatternID);
> +          TemplatePatternID, Redecl.isKeyDecl());
>
> -    // If this declaration was the canonical declaration, make a note of
> that.
> -    if (DCanon == D) {
> -      Reader.MergedDecls[ExistingCanon].push_back(Redecl.getFirstID());
> +    // If this declaration is a key declaration, make a note of that.
> +    if (Redecl.isKeyDecl()) {
> +      Reader.KeyDecls[ExistingCanon].push_back(Redecl.getFirstID());
>        if (Reader.PendingDeclChainsKnown.insert(ExistingCanon).second)
>          Reader.PendingDeclChains.push_back(ExistingCanon);
>      }
> @@ -3468,7 +3478,7 @@ namespace {
>
>          return;
>        }
> -
> +
>        // Dig out all of the redeclarations.
>        unsigned Offset = Result->Offset;
>        unsigned N = M.RedeclarationChains[Offset];
> @@ -3531,9 +3541,9 @@ void ASTReader::loadPendingDeclChain(Dec
>    GlobalDeclID CanonID = CanonDecl->getGlobalID();
>    if (CanonID)
>      SearchDecls.push_back(CanonDecl->getGlobalID()); // Always first.
> -  MergedDeclsMap::iterator MergedPos = MergedDecls.find(CanonDecl);
> -  if (MergedPos != MergedDecls.end())
> -    SearchDecls.append(MergedPos->second.begin(),
> MergedPos->second.end());
> +  KeyDeclsMap::iterator KeyPos = KeyDecls.find(CanonDecl);
> +  if (KeyPos != KeyDecls.end())
> +    SearchDecls.append(KeyPos->second.begin(), KeyPos->second.end());
>
>    // Build up the list of redeclarations.
>    RedeclChainVisitor Visitor(*this, SearchDecls, RedeclsDeserialized,
> CanonID);
>
> Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=241999&r1=241998&r2=241999&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)
> +++ cfe/trunk/lib/Serialization/ASTWriter.cpp Sun Jul 12 18:43:21 2015
> @@ -3634,6 +3634,55 @@ ASTWriter::GenerateNameLookupTable(const
>  /// bitstream, or 0 if no block was written.
>  uint64_t ASTWriter::WriteDeclContextVisibleBlock(ASTContext &Context,
>                                                   DeclContext *DC) {
> +  // If we imported a key declaration of this namespace, write the visible
> +  // lookup results as an update record for it rather than including them
> +  // on this declaration. We will only look at key declarations on reload.
> +  if (isa<NamespaceDecl>(DC) && Chain &&
> +      Chain->getKeyDeclaration(cast<Decl>(DC))->isFromASTFile()) {
> +    // Only do this once, for the first local declaration of the
> namespace.
> +    for (NamespaceDecl *Prev =
> cast<NamespaceDecl>(DC)->getPreviousDecl(); Prev;
> +         Prev = Prev->getPreviousDecl())
> +      if (!Prev->isFromASTFile())
> +        return 0;
> +
> +    // Note that we need to emit an update record for the primary context.
> +    UpdatedDeclContexts.insert(DC->getPrimaryContext());
> +
> +    // Make sure all visible decls are written. They will be recorded
> later. We
> +    // do this using a side data structure so we can sort the names into
> +    // a deterministic order.
> +    StoredDeclsMap *Map = DC->getPrimaryContext()->buildLookup();
> +    SmallVector<std::pair<DeclarationName, DeclContext::lookup_result>,
> 16>
> +        LookupResults;
> +    if (Map) {
> +      LookupResults.reserve(Map->size());
> +      for (auto &Entry : *Map)
> +        LookupResults.push_back(
> +            std::make_pair(Entry.first, Entry.second.getLookupResult()));
> +    }
> +
> +    std::sort(LookupResults.begin(), LookupResults.end(),
> llvm::less_first());
> +    for (auto &NameAndResult : LookupResults) {
> +      DeclarationName Name = NameAndResult.first;
> +      DeclContext::lookup_result Result = NameAndResult.second;
> +      if (Name.getNameKind() == DeclarationName::CXXConstructorName ||
> +          Name.getNameKind() ==
> DeclarationName::CXXConversionFunctionName) {
> +        // We have to work around a name lookup bug here where negative
> lookup
> +        // results for these names get cached in namespace lookup tables
> (these
> +        // names should never be looked up in a namespace).
> +        assert(Result.empty() && "Cannot have a constructor or conversion
> "
> +                                 "function name in a namespace!");
> +        continue;
> +      }
> +
> +      for (NamedDecl *ND : Result)
> +        if (!ND->isFromASTFile())
> +          GetDeclRef(ND);
> +    }
> +
> +    return 0;
> +  }
> +
>    if (DC->getPrimaryContext() != DC)
>      return 0;
>
> @@ -3685,6 +3734,11 @@ void ASTWriter::WriteDeclContextVisibleU
>    SmallString<4096> LookupTable;
>    uint32_t BucketOffset = 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.
> +  if (isa<NamespaceDecl>(DC))
> +    DC = cast<DeclContext>(Chain->getKeyDeclaration(cast<Decl>(DC)));
> +
>    // Write the lookup table
>    RecordData Record;
>    Record.push_back(UPDATE_VISIBLE);
> @@ -3717,11 +3771,17 @@ void ASTWriter::WriteRedeclarations() {
>    SmallVector<serialization::LocalRedeclarationsInfo, 2> LocalRedeclsMap;
>
>    for (unsigned I = 0, N = Redeclarations.size(); I != N; ++I) {
> -    Decl *First = Redeclarations[I];
> -    assert(First->isFirstDecl() && "Not the first declaration?");
> -
> -    Decl *MostRecent = First->getMostRecentDecl();
> -
> +    const Decl *Key = Redeclarations[I];
> +    assert((Chain ? Chain->getKeyDeclaration(Key) == Key
> +                  : Key->isFirstDecl()) &&
> +           "not the key declaration");
> +
> +    const Decl *First = Key->getCanonicalDecl();
> +    const Decl *MostRecent = First->getMostRecentDecl();
> +
> +    assert((getDeclID(First) >= NUM_PREDEF_DECL_IDS || First == Key) &&
> +           "should not have imported key decls for predefined decl");
> +
>      // If we only have a single declaration, there is no point in storing
>      // a redeclaration chain.
>      if (First == MostRecent)
> @@ -3730,34 +3790,34 @@ void ASTWriter::WriteRedeclarations() {
>      unsigned Offset = LocalRedeclChains.size();
>      unsigned Size = 0;
>      LocalRedeclChains.push_back(0); // Placeholder for the size.
> -
> +
>      // Collect the set of local redeclarations of this declaration.
> -    for (Decl *Prev = MostRecent; Prev != First;
> +    for (const Decl *Prev = MostRecent; Prev;
>           Prev = Prev->getPreviousDecl()) {
> -      if (!Prev->isFromASTFile()) {
> +      if (!Prev->isFromASTFile() && Prev != Key) {
>          AddDeclRef(Prev, LocalRedeclChains);
>          ++Size;
>        }
>      }
>
>      LocalRedeclChains[Offset] = Size;
> -
> +
>      // Reverse the set of local redeclarations, so that we store them in
>      // order (since we found them in reverse order).
>      std::reverse(LocalRedeclChains.end() - Size, LocalRedeclChains.end());
> -
> +
>      // Add the mapping from the first ID from the AST to the set of local
>      // declarations.
> -    LocalRedeclarationsInfo Info = { getDeclID(First), Offset };
> +    LocalRedeclarationsInfo Info = { getDeclID(Key), Offset };
>      LocalRedeclsMap.push_back(Info);
> -
> +
>      assert(N == Redeclarations.size() &&
>             "Deserialized a declaration we shouldn't have");
>    }
> -
> +
>    if (LocalRedeclChains.empty())
>      return;
> -
> +
>    // Sort the local redeclarations map by the first declaration ID,
>    // since the reader will be performing binary searches on this
> information.
>    llvm::array_pod_sort(LocalRedeclsMap.begin(), LocalRedeclsMap.end());
> @@ -4053,25 +4113,27 @@ void ASTWriter::WriteASTCore(Sema &SemaR
>    Preprocessor &PP = SemaRef.PP;
>
>    // Set up predefined declaration IDs.
> -  DeclIDs[Context.getTranslationUnitDecl()] =
> PREDEF_DECL_TRANSLATION_UNIT_ID;
> -  if (Context.ObjCIdDecl)
> -    DeclIDs[Context.ObjCIdDecl] = PREDEF_DECL_OBJC_ID_ID;
> -  if (Context.ObjCSelDecl)
> -    DeclIDs[Context.ObjCSelDecl] = PREDEF_DECL_OBJC_SEL_ID;
> -  if (Context.ObjCClassDecl)
> -    DeclIDs[Context.ObjCClassDecl] = PREDEF_DECL_OBJC_CLASS_ID;
> -  if (Context.ObjCProtocolClassDecl)
> -    DeclIDs[Context.ObjCProtocolClassDecl] = PREDEF_DECL_OBJC_PROTOCOL_ID;
> -  if (Context.Int128Decl)
> -    DeclIDs[Context.Int128Decl] = PREDEF_DECL_INT_128_ID;
> -  if (Context.UInt128Decl)
> -    DeclIDs[Context.UInt128Decl] = PREDEF_DECL_UNSIGNED_INT_128_ID;
> -  if (Context.ObjCInstanceTypeDecl)
> -    DeclIDs[Context.ObjCInstanceTypeDecl] =
> PREDEF_DECL_OBJC_INSTANCETYPE_ID;
> -  if (Context.BuiltinVaListDecl)
> -    DeclIDs[Context.getBuiltinVaListDecl()] =
> PREDEF_DECL_BUILTIN_VA_LIST_ID;
> -  if (Context.ExternCContext)
> -    DeclIDs[Context.ExternCContext] = PREDEF_DECL_EXTERN_C_CONTEXT_ID;
> +  auto RegisterPredefDecl = [&] (Decl *D, PredefinedDeclIDs ID) {
> +    if (D) {
> +      assert(D->isCanonicalDecl() && "predefined decl is not canonical");
> +      DeclIDs[D] = ID;
> +      if (D->getMostRecentDecl() != D)
> +        Redeclarations.push_back(D);
> +    }
> +  };
> +  RegisterPredefDecl(Context.getTranslationUnitDecl(),
> +                     PREDEF_DECL_TRANSLATION_UNIT_ID);
> +  RegisterPredefDecl(Context.ObjCIdDecl, PREDEF_DECL_OBJC_ID_ID);
> +  RegisterPredefDecl(Context.ObjCSelDecl, PREDEF_DECL_OBJC_SEL_ID);
> +  RegisterPredefDecl(Context.ObjCClassDecl, PREDEF_DECL_OBJC_CLASS_ID);
> +  RegisterPredefDecl(Context.ObjCProtocolClassDecl,
> +                     PREDEF_DECL_OBJC_PROTOCOL_ID);
> +  RegisterPredefDecl(Context.Int128Decl, PREDEF_DECL_INT_128_ID);
> +  RegisterPredefDecl(Context.UInt128Decl,
> PREDEF_DECL_UNSIGNED_INT_128_ID);
> +  RegisterPredefDecl(Context.ObjCInstanceTypeDecl,
> +                     PREDEF_DECL_OBJC_INSTANCETYPE_ID);
> +  RegisterPredefDecl(Context.BuiltinVaListDecl,
> PREDEF_DECL_BUILTIN_VA_LIST_ID);
> +  RegisterPredefDecl(Context.ExternCContext,
> PREDEF_DECL_EXTERN_C_CONTEXT_ID);
>
>    // Build a record containing all of the tentative definitions in this
> file, in
>    // TentativeDefinitions order.  Generally, this record will be empty for
> @@ -5675,7 +5737,7 @@ void ASTWriter::AddedCXXTemplateSpeciali
>  void ASTWriter::ResolvedExceptionSpec(const FunctionDecl *FD) {
>    assert(!DoneWritingDeclsAndTypes && "Already done writing updates!");
>    if (!Chain) return;
> -  Chain->forEachFormerlyCanonicalImportedDecl(FD, [&](const Decl *D) {
> +  Chain->forEachImportedKeyDecl(FD, [&](const Decl *D) {
>      // If we don't already know the exception specification for this
> redecl
>      // chain, add an update record for it.
>      if (isUnresolvedExceptionSpec(cast<FunctionDecl>(D)
> @@ -5689,7 +5751,7 @@ void ASTWriter::ResolvedExceptionSpec(co
>  void ASTWriter::DeducedReturnType(const FunctionDecl *FD, QualType
> ReturnType) {
>    assert(!WritingAST && "Already writing the AST!");
>    if (!Chain) return;
> -  Chain->forEachFormerlyCanonicalImportedDecl(FD, [&](const Decl *D) {
> +  Chain->forEachImportedKeyDecl(FD, [&](const Decl *D) {
>      DeclUpdates[D].push_back(
>          DeclUpdate(UPD_CXX_DEDUCED_RETURN_TYPE, ReturnType));
>    });
> @@ -5700,7 +5762,7 @@ void ASTWriter::ResolvedOperatorDelete(c
>    assert(!WritingAST && "Already writing the AST!");
>    assert(Delete && "Not given an operator delete");
>    if (!Chain) return;
> -  Chain->forEachFormerlyCanonicalImportedDecl(DD, [&](const Decl *D) {
> +  Chain->forEachImportedKeyDecl(DD, [&](const Decl *D) {
>      DeclUpdates[D].push_back(DeclUpdate(UPD_CXX_RESOLVED_DTOR_DELETE,
> Delete));
>    });
>  }
>
> Modified: cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriterDecl.cpp?rev=241999&r1=241998&r2=241999&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Serialization/ASTWriterDecl.cpp (original)
> +++ cfe/trunk/lib/Serialization/ASTWriterDecl.cpp Sun Jul 12 18:43:21 2015
> @@ -254,6 +254,8 @@ void ASTDeclWriter::VisitDecl(Decl *D) {
>    //
>    // This happens when we instantiate a class with a friend declaration
> or a
>    // function with a local extern declaration, for instance.
> +  //
> +  // FIXME: Can we handle this in AddedVisibleDecl instead?
>    if (D->isOutOfLine()) {
>      auto *DC = D->getDeclContext();
>      while (auto *NS = dyn_cast<NamespaceDecl>(DC->getRedeclContext())) {
> @@ -1005,42 +1007,6 @@ void ASTDeclWriter::VisitNamespaceDecl(N
>      Writer.AddDeclRef(D->getAnonymousNamespace(), Record);
>    Code = serialization::DECL_NAMESPACE;
>
> -  if (Writer.hasChain() && !D->isOriginalNamespace() &&
> -      D->getOriginalNamespace()->isFromASTFile()) {
> -    NamespaceDecl *NS = D->getOriginalNamespace();
> -    Writer.UpdatedDeclContexts.insert(NS);
> -
> -    // Make sure all visible decls are written. They will be recorded
> later. We
> -    // do this using a side data structure so we can sort the names into
> -    // a deterministic order.
> -    StoredDeclsMap *Map = NS->buildLookup();
> -    SmallVector<std::pair<DeclarationName, DeclContext::lookup_result>,
> 16>
> -        LookupResults;
> -    if (Map) {
> -      LookupResults.reserve(Map->size());
> -      for (auto &Entry : *Map)
> -        LookupResults.push_back(
> -            std::make_pair(Entry.first, Entry.second.getLookupResult()));
> -    }
> -
> -    std::sort(LookupResults.begin(), LookupResults.end(),
> llvm::less_first());
> -    for (auto &NameAndResult : LookupResults) {
> -      DeclarationName Name = NameAndResult.first;
> -      DeclContext::lookup_result Result = NameAndResult.second;
> -      if (Name.getNameKind() == DeclarationName::CXXConstructorName ||
> -          Name.getNameKind() ==
> DeclarationName::CXXConversionFunctionName) {
> -        // We have to work around a name lookup bug here where negative
> lookup
> -        // results for these names get cached in namespace lookup tables.
> -        assert(Result.empty() && "Cannot have a constructor or conversion
> "
> -                                 "function name in a namespace!");
> -        continue;
> -      }
> -
> -      for (NamedDecl *ND : Result)
> -        Writer.GetDeclRef(ND);
> -    }
> -  }
> -
>    if (Writer.hasChain() && D->isAnonymousNamespace() &&
>        D == D->getMostRecentDecl()) {
>      // This is a most recent reopening of the anonymous namespace. If its
> parent
> @@ -1512,6 +1478,17 @@ void ASTDeclWriter::VisitDeclContext(Dec
>    Record.push_back(VisibleOffset);
>  }
>
> +/// Determine whether D is the first declaration in its redeclaration
> chain that
> +/// is not from an AST file.
> +template <typename T>
> +static bool isFirstLocalDecl(Redeclarable<T> *D) {
> +  assert(D && !static_cast<T*>(D)->isFromASTFile());
> +  do
> +    D = D->getPreviousDecl();
> +  while (D && static_cast<T*>(D)->isFromASTFile());
> +  return !D;
> +}
> +
>  template <typename T>
>  void ASTDeclWriter::VisitRedeclarable(Redeclarable<T> *D) {
>    T *First = D->getFirstDecl();
> @@ -1520,41 +1497,30 @@ void ASTDeclWriter::VisitRedeclarable(Re
>      assert(isRedeclarableDeclKind(static_cast<T *>(D)->getKind()) &&
>             "Not considered redeclarable?");
>
> -    // There is more than one declaration of this entity, so we will need
> to
> -    // write a redeclaration chain.
>      Writer.AddDeclRef(First, Record);
> -    Writer.Redeclarations.insert(First);
>
> -    auto *Previous = D->getPreviousDecl();
> -
> -    // In a modules build, we can have imported declarations after a local
> -    // canonical declaration. If this is the first local declaration, emit
> -    // a list of all such imported declarations so that we can ensure they
> -    // are loaded before we are. This allows us to rebuild the redecl
> chain
> -    // in the right order on reload (all declarations imported by a module
> -    // should be before all declarations provided by that module).
> -    bool EmitImportedMergedCanonicalDecls = false;
> +    // In a modules build, emit a list of all imported key declarations
> +    // (excluding First, if it was imported), so that we can be sure that
> all
> +    // redeclarations visible to this module are before D in the redecl
> chain.
> +    unsigned I = Record.size();
> +    Record.push_back(0);
>      if (Context.getLangOpts().Modules && Writer.Chain) {
> -      auto *PreviousLocal = Previous;
> -      while (PreviousLocal && PreviousLocal->isFromASTFile())
> -        PreviousLocal = PreviousLocal->getPreviousDecl();
> -      if (!PreviousLocal)
> -        EmitImportedMergedCanonicalDecls = true;
> +      if (isFirstLocalDecl(D)) {
> +        Writer.Chain->forEachImportedKeyDecl(First, [&](const Decl *D) {
> +          if (D != First)
> +            Writer.AddDeclRef(D, Record);
> +        });
> +        Record[I] = Record.size() - I - 1;
> +
> +        // Write a redeclaration chain, attached to the first key decl.
> +
> Writer.Redeclarations.push_back(Writer.Chain->getKeyDeclaration(First));
> +      }
> +    } else if (D == First || D->getPreviousDecl()->isFromASTFile()) {
> +      assert(isFirstLocalDecl(D) && "imported decl after local decl");
> +
> +      // Write a redeclaration chain attached to the first decl.
> +      Writer.Redeclarations.push_back(First);
>      }
> -    if (EmitImportedMergedCanonicalDecls) {
> -      llvm::SmallMapVector<ModuleFile*, Decl*, 16> FirstInModule;
> -      for (auto *Redecl = MostRecent; Redecl;
> -           Redecl = Redecl->getPreviousDecl())
> -        if (Redecl->isFromASTFile())
> -          FirstInModule[Writer.Chain->getOwningModuleFile(Redecl)] =
> Redecl;
> -      // FIXME: If FirstInModule has entries for modules A and B, and B
> imports
> -      // A (directly or indirectly), we don't need to write the entry for
> A.
> -      Record.push_back(FirstInModule.size());
> -      for (auto I = FirstInModule.rbegin(), E = FirstInModule.rend();
> -           I != E; ++I)
> -        Writer.AddDeclRef(I->second, Record);
> -    } else
> -      Record.push_back(0);
>
>      // Make sure that we serialize both the previous and the most-recent
>      // declarations, which (transitively) ensures that all declarations
> in the
> @@ -1562,7 +1528,7 @@ void ASTDeclWriter::VisitRedeclarable(Re
>      //
>      // FIXME: This is not correct; when we reach an imported declaration
> we
>      // won't emit its previous declaration.
> -    (void)Writer.GetDeclRef(Previous);
> +    (void)Writer.GetDeclRef(D->getPreviousDecl());
>      (void)Writer.GetDeclRef(MostRecent);
>    } else {
>      // We use the sentinel value 0 to indicate an only declaration.
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150713/1fad6eb3/attachment.html>


More information about the cfe-commits mailing list