[PATCH] D52268: [AST] Squeeze some bits in LinkageComputer

2018-09-25 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL342973: [AST] Squeeze some bits in LinkageComputer::QueryType (authored by brunoricci, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D52268?v

[PATCH] D52268: [AST] Squeeze some bits in LinkageComputer

2018-09-21 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv accepted this revision. george.burgess.iv added a comment. LGTM too. Thanks again! Repository: rC Clang https://reviews.llvm.org/D52268 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailm

[PATCH] D52268: [AST] Squeeze some bits in LinkageComputer

2018-09-21 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. In https://reviews.llvm.org/D52268#1241793, @riccibruno wrote: > In https://reviews.llvm.org/D52268#1241033, @rjmccall wrote: > > > `LinkageComputer` isn't actually persisted anywhere, righ

[PATCH] D52268: [AST] Squeeze some bits in LinkageComputer

2018-09-21 Thread Bruno Ricci via Phabricator via cfe-commits
riccibruno added a comment. In https://reviews.llvm.org/D52268#1241033, @rjmccall wrote: > `LinkageComputer` isn't actually persisted anywhere, right? And there's > maybe one computer active at once? So this compression is theoretically > saving one pointer of stack space but forcing a bunch

[PATCH] D52268: [AST] Squeeze some bits in LinkageComputer

2018-09-20 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. `LinkageComputer` isn't actually persisted anywhere, right? And there's maybe one computer active at once? So this compression is theoretically saving one pointer of stack space but forcing a bunch of bit-manipulation every time these fields are accessed. Repositor

[PATCH] D52268: [AST] Squeeze some bits in LinkageComputer

2018-09-20 Thread Bruno Ricci via Phabricator via cfe-commits
riccibruno updated this revision to Diff 166264. riccibruno marked 3 inline comments as done. riccibruno added a comment. Removed the superfluous static_assert. Repository: rC Clang https://reviews.llvm.org/D52268 Files: lib/AST/Linkage.h Index: lib/AST/Linkage.h

[PATCH] D52268: [AST] Squeeze some bits in LinkageComputer

2018-09-19 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv added a comment. Thanks for this! LGTM after erichkeane's comments are resolved. > I did a little digging on this, and it seems to be to keep track of a > declarations linkage for caching sake Yeah, otherwise, we get exponential behavior on some pathological template-y patter

[PATCH] D52268: [AST] Squeeze some bits in LinkageComputer

2018-09-19 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added subscribers: gbiv, george.burgess.iv. erichkeane added a comment. In https://reviews.llvm.org/D52268#1239557, @riccibruno wrote: > In https://reviews.llvm.org/D52268#1239538, @erichkeane wrote: > > > Does this still work with 32 bit hosts? Does PointerIntPair have 3 bits in > >

[PATCH] D52268: [AST] Squeeze some bits in LinkageComputer

2018-09-19 Thread Bruno Ricci via Phabricator via cfe-commits
riccibruno added a comment. In https://reviews.llvm.org/D52268#1239538, @erichkeane wrote: > Does this still work with 32 bit hosts? Does PointerIntPair have 3 bits in > that case? Is the alignof static_assert valid in that case? I think it does since Decl is manually over-aligned to 8 bytes

[PATCH] D52268: [AST] Squeeze some bits in LinkageComputer

2018-09-19 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. Does this still work with 32 bit hosts? Does PointerIntPair have 3 bits in that case? Is the alignof static_assert valid in that case? Comment at: lib/AST/Linkage.h:40 + enum { NumLVComputationKindBits = 3 }; + I'm not sure how

[PATCH] D52268: [AST] Squeeze some bits in LinkageComputer

2018-09-19 Thread Bruno Ricci via Phabricator via cfe-commits
riccibruno created this revision. riccibruno added reviewers: erichkeane, rjmccall. riccibruno added a project: clang. Herald added a subscriber: cfe-commits. Since Decls are already aligned explicitly to 8 bytes, stash the 3 bits representing an LVComputationKind into the lower 3 bits of the Name