[PATCH] Keep track of canonical decls in Redeclarable.

[Rafael Espíndola]

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
>