[PATCH] Keep track of canonical decls in Redeclarable.

[Manuel Klimek]

Yep, we see a roughly 10x speedup on that test (11 seconds -> 1.1 second)

On Wed, Mar 25, 2015 at 2:51 PM Rafael Espíndola <rafael.espindola at gmail.com>
wrote:

> I think this fixes
>
> https://llvm.org/bugs/show_bug.cgi?id=10651
>
> My previous attempt was to invert the link order, but that is a lot
> more involved.
>
>
> An interesting testcase:
>
> #define M extern int a;
>     #define M2 M M
>     #define M4 M2 M2
>     #define M8 M4 M4
>     #define M16 M8 M8
>     #define M32 M16 M16
>     #define M64 M32 M32
>     #define M128 M64 M64
>     #define M256 M128 M128
>     #define M512 M256 M256
>     #define M1024 M512 M512
>     #define M2048 M1024 M1024
>     #define M4096 M2048 M2048
>     #define M8192 M4096 M4096
>     #define M16384 M8192 M8192
>     M16384
>
>
> On 25 March 2015 at 17:45, Manuel Klimek <klimek at google.com> wrote:
> > Hi rsmith,
> >
> > More than 2x speedup on modules builds with large redecl chains.
> > Between 2-3% cost in memory on large TUs.
> >
> > http://reviews.llvm.org/D8619
> >
> > Files:
> >   include/clang/AST/Decl.h
> >   include/clang/AST/Redeclarable.h
> >   lib/Serialization/ASTReaderDecl.cpp
> >
> > Index: include/clang/AST/Decl.h
> > ===================================================================
> > --- include/clang/AST/Decl.h
> > +++ include/clang/AST/Decl.h
> > @@ -3738,8 +3738,6 @@
> >    assert(RedeclLink.NextIsLatest() &&
> >           "setPreviousDecl on a decl already in a redeclaration chain");
> >
> > -  decl_type *First;
> > -
> >    if (PrevDecl) {
> >      // Point to previous. Make sure that this is actually the most
> recent
> >      // redeclaration, or we can build invalid chains. If the most recent
> > Index: include/clang/AST/Redeclarable.h
> > ===================================================================
> > --- include/clang/AST/Redeclarable.h
> > +++ include/clang/AST/Redeclarable.h
> > @@ -121,14 +121,15 @@
> >    ///
> >    /// If there is only one declaration, it is <pointer to self, true>
> >    DeclLink RedeclLink;
> > +  decl_type *First;
> >
> >    decl_type *getNextRedeclaration() const {
> >      return RedeclLink.getNext(static_cast<const decl_type *>(this));
> >    }
> >
> >  public:
> > -  Redeclarable(const ASTContext &Ctx)
> > -      : RedeclLink(LatestDeclLink(Ctx)) {}
> > + Redeclarable(const ASTContext &Ctx)
> > +     : RedeclLink(LatestDeclLink(Ctx)), First(static_cast<decl_type
> *>(this)) {}
> >
> >    /// \brief Return the previous declaration of this declaration or
> NULL if this
> >    /// is the first declaration.
> > @@ -144,21 +145,11 @@
> >
> >    /// \brief Return the first declaration of this declaration or itself
> if this
> >    /// is the only declaration.
> > -  decl_type *getFirstDecl() {
> > -    decl_type *D = static_cast<decl_type*>(this);
> > -    while (D->getPreviousDecl())
> > -      D = D->getPreviousDecl();
> > -    return D;
> > -  }
> > +  decl_type *getFirstDecl() { return First; }
> >
> >    /// \brief Return the first declaration of this declaration or itself
> if this
> >    /// is the only declaration.
> > -  const decl_type *getFirstDecl() const {
> > -    const decl_type *D = static_cast<const decl_type*>(this);
> > -    while (D->getPreviousDecl())
> > -      D = D->getPreviousDecl();
> > -    return D;
> > -  }
> > +  const decl_type *getFirstDecl() const { return First; }
> >
> >    /// \brief True if this is the first declaration in its redeclaration
> chain.
> >    bool isFirstDecl() const { return RedeclLink.NextIsLatest(); }
> > Index: lib/Serialization/ASTReaderDecl.cpp
> > ===================================================================
> > --- lib/Serialization/ASTReaderDecl.cpp
> > +++ lib/Serialization/ASTReaderDecl.cpp
> > @@ -2106,6 +2106,7 @@
> >      // which is the one that matters and mark the real previous DeclID
> to be
> >      // loaded & attached later on.
> >      D->RedeclLink = Redeclarable<T>::PreviousDeclLink(FirstDecl);
> > +    D->First = FirstDecl->getCanonicalDecl();
> >    }
> >
> >    // Note that this declaration has been deserialized.
> > @@ -2209,6 +2210,7 @@
> >      // of the existing declaration, so that this declaration has the
> >      // appropriate canonical declaration.
> >      D->RedeclLink = Redeclarable<T>::PreviousDeclLink(ExistingCanon);
> > +    D->First = ExistingCanon;
> >
> >      // When we merge a namespace, update its pointer to the first
> namespace.
> >      // We cannot have loaded any redeclarations of this declaration
> yet, so
> > @@ -2828,6 +2830,7 @@
> >                                             Redeclarable<DeclT> *D,
> >                                             Decl *Previous, Decl *Canon)
> {
> >    D->RedeclLink.setPrevious(cast<DeclT>(Previous));
> > +  D->First = cast<DeclT>(Previous)->First;
> >  }
> >  namespace clang {
> >  template<>
> > @@ -2838,6 +2841,7 @@
> >    FunctionDecl *PrevFD = cast<FunctionDecl>(Previous);
> >
> >    FD->RedeclLink.setPrevious(PrevFD);
> > +  FD->First = PrevFD->First;
> >
> >    // If the previous declaration is an inline function declaration,
> then this
> >    // declaration is too.
> >
> > EMAIL PREFERENCES
> >   http://reviews.llvm.org/settings/panel/emailpreferences/
> >
> > _______________________________________________
> > 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/20150325/a968d0c2/attachment.html>