<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Mar 25, 2015 at 5:56 PM, Sean Silva <span dir="ltr"><<a href="mailto:chisophugis@gmail.com" target="_blank">chisophugis@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div>Awesome performance numbers!</div><div><br></div>When we discussed this previously, one alternative (should we ever need it; not sure we do) if the memory overhead is excessive is to use a waymarking scheme to find the canonical decl in constant time (unlike Use <a href="http://www.llvm.org/docs/ProgrammersManual.html#the-waymarking-algorithm" target="_blank">http://www.llvm.org/docs/ProgrammersManual.html#the-waymarking-algorithm</a> , we don't have random access, but we can instead take advantage of the stable address of the canonical decl and directly store bits of the stable address).</div></blockquote><div><br></div><div>D'oh, just saw the review thread and <a href="https://llvm.org/bugs/show_bug.cgi?id=21264">https://llvm.org/bugs/show_bug.cgi?id=21264</a> where Richard suggests a more elaborate (but not guaranteed constant time?) scheme.</div><div><br></div><div>-- Sean Silva</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><span class=""><font color="#888888"><div><br></div><div>-- Sean Silva</div></font></span></div><div class=""><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Mar 25, 2015 at 4:18 PM, Manuel Klimek <span dir="ltr"><<a href="mailto:klimek@google.com" target="_blank">klimek@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Author: klimek<br>
Date: Wed Mar 25 18:18:30 2015<br>
New Revision: 233228<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=233228&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=233228&view=rev</a><br>
Log:<br>
Keep track of canonical decls in Redeclarable.<br>
<br>
More than 2x speedup on modules builds with large redecl chains.<br>
Roughly 15-20% speedup on non-modules builds for very large TUs.<br>
Between 2-3% cost in memory on large TUs.<br>
<br>
Modified:<br>
cfe/trunk/include/clang/AST/Decl.h<br>
cfe/trunk/include/clang/AST/Redeclarable.h<br>
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp<br>
<br>
Modified: cfe/trunk/include/clang/AST/Decl.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=233228&r1=233227&r2=233228&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=233228&r1=233227&r2=233228&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/AST/Decl.h (original)<br>
+++ cfe/trunk/include/clang/AST/Decl.h Wed Mar 25 18:18:30 2015<br>
@@ -3738,8 +3738,6 @@ void Redeclarable<decl_type>::setPreviou<br>
assert(RedeclLink.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>
<br>
Modified: cfe/trunk/include/clang/AST/Redeclarable.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Redeclarable.h?rev=233228&r1=233227&r2=233228&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Redeclarable.h?rev=233228&r1=233227&r2=233228&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/AST/Redeclarable.h (original)<br>
+++ cfe/trunk/include/clang/AST/Redeclarable.h Wed Mar 25 18:18:30 2015<br>
@@ -121,14 +121,15 @@ protected:<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_cast<const decl_type *>(this));<br>
}<br>
<br>
public:<br>
- Redeclarable(const ASTContext &Ctx)<br>
- : RedeclLink(LatestDeclLink(Ctx)) {}<br>
+ Redeclarable(const ASTContext &Ctx)<br>
+ : RedeclLink(LatestDeclLink(Ctx)), 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 @@ public:<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>
<br>
Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=233228&r1=233227&r2=233228&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=233228&r1=233227&r2=233228&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)<br>
+++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Wed Mar 25 18:18:30 2015<br>
@@ -2106,6 +2106,7 @@ ASTDeclReader::VisitRedeclarable(Redecla<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>::PreviousDeclLink(FirstDecl);<br>
+ D->First = FirstDecl->getCanonicalDecl();<br>
}<br>
<br>
// Note that this declaration has been deserialized.<br>
@@ -2209,6 +2210,7 @@ void ASTDeclReader::mergeRedeclarable(Re<br>
// of the existing declaration, so that this declaration has the<br>
// appropriate canonical declaration.<br>
D->RedeclLink = Redeclarable<T>::PreviousDeclLink(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 @@ void ASTDeclReader::attachPreviousDeclIm<br>
Redeclarable<DeclT> *D,<br>
Decl *Previous, Decl *Canon) {<br>
D->RedeclLink.setPrevious(cast<DeclT>(Previous));<br>
+ D->First = cast<DeclT>(Previous)->First;<br>
}<br>
namespace clang {<br>
template<><br>
@@ -2838,6 +2841,7 @@ void ASTDeclReader::attachPreviousDeclIm<br>
FunctionDecl *PrevFD = cast<FunctionDecl>(Previous);<br>
<br>
FD->RedeclLink.setPrevious(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>
<br>
_______________________________________________<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/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div>
</div></div></blockquote></div><br></div></div>