jkorous updated this revision to Diff 194604. jkorous added a comment. Second attempt on reducing the cache size and number of operations.
I try in this order 1. cache lookup for the specific provided declaration 2. try to find comment attached to the specific provided declaration without using cache (and cache it using the specific declaration as a key) 3. cache lookup using the canonical declaration (which would return comment from any redeclaration - see the next step) 4. try to find comment attached to any redeclaration (and cache it using the canonical declaration as a key) 5. give up, return nullptr CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60432/new/ https://reviews.llvm.org/D60432 Files: clang/include/clang/AST/ASTContext.h clang/lib/AST/ASTContext.cpp
Index: clang/lib/AST/ASTContext.cpp =================================================================== --- clang/lib/AST/ASTContext.cpp +++ clang/lib/AST/ASTContext.cpp @@ -370,70 +370,69 @@ const Decl **OriginalDecl) const { D = adjustDeclToTemplate(D); - // Check whether we have cached a comment for this declaration already. - { + auto CacheLookup = [&](const Decl *SpecificD) -> const RawComment * { llvm::DenseMap<const Decl *, RawCommentAndCacheFlags>::iterator Pos = - RedeclComments.find(D); + RedeclComments.find(SpecificD); if (Pos != RedeclComments.end()) { - const RawCommentAndCacheFlags &Raw = Pos->second; - if (Raw.getKind() != RawCommentAndCacheFlags::NoCommentInDecl) { - if (OriginalDecl) - *OriginalDecl = Raw.getOriginalDecl(); - return Raw.getRaw(); - } + const RawCommentAndCacheFlags &CachedComment = Pos->second; + if (OriginalDecl) + *OriginalDecl = CachedComment.getOriginalDecl(); + return CachedComment.getRaw(); } - } + return nullptr; + }; - // Search for comments attached to declarations in the redeclaration chain. - const RawComment *RC = nullptr; - const Decl *OriginalDeclForRC = nullptr; - for (auto I : D->redecls()) { - llvm::DenseMap<const Decl *, RawCommentAndCacheFlags>::iterator Pos = - RedeclComments.find(I); - if (Pos != RedeclComments.end()) { - const RawCommentAndCacheFlags &Raw = Pos->second; - if (Raw.getKind() != RawCommentAndCacheFlags::NoCommentInDecl) { - RC = Raw.getRaw(); - OriginalDeclForRC = Raw.getOriginalDecl(); - break; - } - } else { - RC = getRawCommentForDeclNoCache(I); - OriginalDeclForRC = I; - RawCommentAndCacheFlags Raw; - if (RC) { - // Call order swapped to work around ICE in VS2015 RTM (Release Win32) - // https://connect.microsoft.com/VisualStudio/feedback/details/1741530 - Raw.setKind(RawCommentAndCacheFlags::FromDecl); - Raw.setRaw(RC); - } else - Raw.setKind(RawCommentAndCacheFlags::NoCommentInDecl); - Raw.setOriginalDecl(I); - RedeclComments[I] = Raw; - if (RC) - break; + auto GetCommentNoCache = [&](const Decl *SpecificD) -> const llvm::Optional<RawCommentAndCacheFlags> { + if (const RawComment *RC = getRawCommentForDeclNoCache(SpecificD)) { + // If we find a comment, it should be a documentation comment. + assert(RC->isDocumentation() || LangOpts.CommentOpts.ParseAllComments); + + RawCommentAndCacheFlags NewCachedComment; + // Call order swapped to work around ICE in VS2015 RTM (Release Win32) + // https://connect.microsoft.com/VisualStudio/feedback/details/1741530 + NewCachedComment.setRaw(RC); + NewCachedComment.setOriginalDecl(SpecificD); + + if (OriginalDecl) + *OriginalDecl = SpecificD; + + return NewCachedComment; } - } + return llvm::None; + }; - // If we found a comment, it should be a documentation comment. - assert(!RC || RC->isDocumentation() || LangOpts.CommentOpts.ParseAllComments); + // Check whether we have cached a comment for this specific declaration. + if (auto * CachedComment = CacheLookup(D)) + return CachedComment; - if (OriginalDecl) - *OriginalDecl = OriginalDeclForRC; + // Try to find comment for this specific declaration. + if (llvm::Optional<RawCommentAndCacheFlags> OptCommentAndFlags = GetCommentNoCache(D)) { + const RawCommentAndCacheFlags& CommentAndFlags = OptCommentAndFlags.getValue(); + RedeclComments[D] = CommentAndFlags; + return CommentAndFlags.getRaw(); + } + + // In case this is the canonical Decl we're done. + auto CanonicalD = D->getCanonicalDecl(); + if (!CanonicalD) + return nullptr; - // Update cache for every declaration in the redeclaration chain. - RawCommentAndCacheFlags Raw; - Raw.setRaw(RC); - Raw.setKind(RawCommentAndCacheFlags::FromRedecl); - Raw.setOriginalDecl(OriginalDeclForRC); + // Check whether we have cached a comment for canonical declaration of this declaration. + if (auto * CachedComment = CacheLookup(CanonicalD)) + return CachedComment; + // We don't have comment for neither D, nor it's canonical declaration in cache and D doesn't have any comment attached to itself. + // Search for any comment attached to redeclarations of D and cache it for canonical declaration of D. for (auto I : D->redecls()) { - RawCommentAndCacheFlags &R = RedeclComments[I]; - if (R.getKind() == RawCommentAndCacheFlags::NoCommentInDecl) - R = Raw; + if (llvm::Optional<RawCommentAndCacheFlags> OptCommentAndFlags = GetCommentNoCache(I)) { + const RawCommentAndCacheFlags& CommentAndFlags = OptCommentAndFlags.getValue(); + RedeclComments[CanonicalD] = CommentAndFlags; + return CommentAndFlags.getRaw(); + } } - return RC; + // We didn't find any comment attached to any redeclaration of D. + return nullptr; } static void addRedeclaredMethods(const ObjCMethodDecl *ObjCMethod, Index: clang/include/clang/AST/ASTContext.h =================================================================== --- clang/include/clang/AST/ASTContext.h +++ clang/include/clang/AST/ASTContext.h @@ -727,43 +727,12 @@ class RawCommentAndCacheFlags { public: - enum Kind { - /// We searched for a comment attached to the particular declaration, but - /// didn't find any. - /// - /// getRaw() == 0. - NoCommentInDecl = 0, - - /// We have found a comment attached to this particular declaration. - /// - /// getRaw() != 0. - FromDecl, - - /// This declaration does not have an attached comment, and we have - /// searched the redeclaration chain. - /// - /// If getRaw() == 0, the whole redeclaration chain does not have any - /// comments. - /// - /// If getRaw() != 0, it is a comment propagated from other - /// redeclaration. - FromRedecl - }; - - Kind getKind() const LLVM_READONLY { - return Data.getInt(); - } - - void setKind(Kind K) { - Data.setInt(K); - } - const RawComment *getRaw() const LLVM_READONLY { - return Data.getPointer(); + return Comment; } void setRaw(const RawComment *RC) { - Data.setPointer(RC); + Comment = RC; } const Decl *getOriginalDecl() const LLVM_READONLY { @@ -775,11 +744,11 @@ } private: - llvm::PointerIntPair<const RawComment *, 2, Kind> Data; + const RawComment * Comment; const Decl *OriginalDecl; }; - /// Mapping from declarations to comments attached to any + /// Mapping from canonical declarations to comments attached to any /// redeclaration. /// /// Raw comments are owned by Comments list. This mapping is populated
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits