<div dir="ltr">Yep, we see a roughly 10x speedup on that test (11 seconds -> 1.1 second)<br></div><br><div class="gmail_quote">On Wed, Mar 25, 2015 at 2:51 PM Rafael Espíndola <<a href="mailto:rafael.espindola@gmail.com">rafael.espindola@gmail.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I think this fixes<br>
<br>
<a href="https://llvm.org/bugs/show_bug.cgi?id=10651" target="_blank">https://llvm.org/bugs/show_<u></u>bug.cgi?id=10651</a><br>
<br>
My previous attempt was to invert the link order, but that is a lot<br>
more involved.<br>
<br>
<br>
An interesting testcase:<br>
<br>
#define M extern int a;<br>
#define M2 M M<br>
#define M4 M2 M2<br>
#define M8 M4 M4<br>
#define M16 M8 M8<br>
#define M32 M16 M16<br>
#define M64 M32 M32<br>
#define M128 M64 M64<br>
#define M256 M128 M128<br>
#define M512 M256 M256<br>
#define M1024 M512 M512<br>
#define M2048 M1024 M1024<br>
#define M4096 M2048 M2048<br>
#define M8192 M4096 M4096<br>
#define M16384 M8192 M8192<br>
M16384<br>
<br>
<br>
On 25 March 2015 at 17:45, Manuel Klimek <<a href="mailto:klimek@google.com" target="_blank">klimek@google.com</a>> wrote:<br>
> Hi rsmith,<br>
><br>
> More than 2x speedup on modules builds with large redecl chains.<br>
> Between 2-3% cost in memory on large TUs.<br>
><br>
> <a href="http://reviews.llvm.org/D8619" target="_blank">http://reviews.llvm.org/D8619</a><br>
><br>
> Files:<br>
> include/clang/AST/Decl.h<br>
> include/clang/AST/<u></u>Redeclarable.h<br>
> lib/Serialization/<u></u>ASTReaderDecl.cpp<br>
><br>
> Index: include/clang/AST/Decl.h<br>
> ==============================<u></u>==============================<u></u>=======<br>
> --- include/clang/AST/Decl.h<br>
> +++ include/clang/AST/Decl.h<br>
> @@ -3738,8 +3738,6 @@<br>
> assert(RedeclLink.<u></u>NextIsLatest() &&<br>
> "setPreviousDecl on a decl already in a redeclaration chain");<br>
><br>
> - decl_type *First;<br>
> -<br>
> if (PrevDecl) {<br>
> // Point to previous. Make sure that this is actually the most recent<br>
> // redeclaration, or we can build invalid chains. If the most recent<br>
> Index: include/clang/AST/<u></u>Redeclarable.h<br>
> ==============================<u></u>==============================<u></u>=======<br>
> --- include/clang/AST/<u></u>Redeclarable.h<br>
> +++ include/clang/AST/<u></u>Redeclarable.h<br>
> @@ -121,14 +121,15 @@<br>
> ///<br>
> /// If there is only one declaration, it is <pointer to self, true><br>
> DeclLink RedeclLink;<br>
> + decl_type *First;<br>
><br>
> decl_type *getNextRedeclaration() const {<br>
> return RedeclLink.getNext(static_<u></u>cast<const decl_type *>(this));<br>
> }<br>
><br>
> public:<br>
> - Redeclarable(const ASTContext &Ctx)<br>
> - : RedeclLink(LatestDeclLink(Ctx)<u></u>) {}<br>
> + Redeclarable(const ASTContext &Ctx)<br>
> + : RedeclLink(LatestDeclLink(Ctx)<u></u>), First(static_cast<decl_type *>(this)) {}<br>
><br>
> /// \brief Return the previous declaration of this declaration or NULL if this<br>
> /// is the first declaration.<br>
> @@ -144,21 +145,11 @@<br>
><br>
> /// \brief Return the first declaration of this declaration or itself if this<br>
> /// is the only declaration.<br>
> - decl_type *getFirstDecl() {<br>
> - decl_type *D = static_cast<decl_type*>(this);<br>
> - while (D->getPreviousDecl())<br>
> - D = D->getPreviousDecl();<br>
> - return D;<br>
> - }<br>
> + decl_type *getFirstDecl() { return First; }<br>
><br>
> /// \brief Return the first declaration of this declaration or itself if this<br>
> /// is the only declaration.<br>
> - const decl_type *getFirstDecl() const {<br>
> - const decl_type *D = static_cast<const decl_type*>(this);<br>
> - while (D->getPreviousDecl())<br>
> - D = D->getPreviousDecl();<br>
> - return D;<br>
> - }<br>
> + const decl_type *getFirstDecl() const { return First; }<br>
><br>
> /// \brief True if this is the first declaration in its redeclaration chain.<br>
> bool isFirstDecl() const { return RedeclLink.NextIsLatest(); }<br>
> Index: lib/Serialization/<u></u>ASTReaderDecl.cpp<br>
> ==============================<u></u>==============================<u></u>=======<br>
> --- lib/Serialization/<u></u>ASTReaderDecl.cpp<br>
> +++ lib/Serialization/<u></u>ASTReaderDecl.cpp<br>
> @@ -2106,6 +2106,7 @@<br>
> // which is the one that matters and mark the real previous DeclID to be<br>
> // loaded & attached later on.<br>
> D->RedeclLink = Redeclarable<T>::<u></u>PreviousDeclLink(FirstDecl);<br>
> + D->First = FirstDecl->getCanonicalDecl();<br>
> }<br>
><br>
> // Note that this declaration has been deserialized.<br>
> @@ -2209,6 +2210,7 @@<br>
> // of the existing declaration, so that this declaration has the<br>
> // appropriate canonical declaration.<br>
> D->RedeclLink = Redeclarable<T>::<u></u>PreviousDeclLink(<u></u>ExistingCanon);<br>
> + D->First = ExistingCanon;<br>
><br>
> // When we merge a namespace, update its pointer to the first namespace.<br>
> // We cannot have loaded any redeclarations of this declaration yet, so<br>
> @@ -2828,6 +2830,7 @@<br>
> Redeclarable<DeclT> *D,<br>
> Decl *Previous, Decl *Canon) {<br>
> D->RedeclLink.setPrevious(<u></u>cast<DeclT>(Previous));<br>
> + D->First = cast<DeclT>(Previous)->First;<br>
> }<br>
> namespace clang {<br>
> template<><br>
> @@ -2838,6 +2841,7 @@<br>
> FunctionDecl *PrevFD = cast<FunctionDecl>(Previous);<br>
><br>
> FD->RedeclLink.setPrevious(<u></u>PrevFD);<br>
> + FD->First = PrevFD->First;<br>
><br>
> // If the previous declaration is an inline function declaration, then this<br>
> // declaration is too.<br>
><br>
> EMAIL PREFERENCES<br>
> <a href="http://reviews.llvm.org/settings/panel/emailpreferences/" target="_blank">http://reviews.llvm.org/<u></u>settings/panel/<u></u>emailpreferences/</a><br>
><br>
> ______________________________<u></u>_________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/cfe-commits</a><br>
><br>
</blockquote></div>