DmitryPolukhin marked an inline comment as done.
DmitryPolukhin added inline comments.


================
Comment at: clang/include/clang/Serialization/ASTBitCodes.h:220
     /// Source range/offset of a preprocessed entity.
     struct DeclOffset {
+      /// Raw source location. The unsigned i.e. 32-bit integer is enough for
----------------
sammccall wrote:
> Is there one of these for every decl in the module? It seems like we're 
> probably giving up a good fraction of the 4% increase for just using absolute 
> 64 bit offsets everywhere :-( Is there still a significant gain from using 
> section-relative elsewhere?
> 
> If there are indeed lots of these, giving up 4 bytes to padding (in addition 
> to the wide offset) seems unfortunate and because we memcpy the structs into 
> the AST file seems like a sad reason :-)
> Can we align this to 4 bytes instead?
> (e.g. by splitting into two fields and encapsulating the few direct accesses, 
> though there's probably a neater way)
Yes, it seems that all referenced decls should have an entry in this table. 4% 
increase was counted on previous diff before I discovered DeclOffsets and 
TypeOffsets. This diff uses relative offsets for previous cases and increase is 
only 2 additional 64-bit integers per file. DeclOffsets are about 4.7% and 
TypeOffsets are about 2.5% of the whole file. With this diff both number will 
be 2x, splitting DeclOffset in two separate arrays could make the increase only 
1.5x for DeclOffsets. If approach from this diff looks fine, I can split array 
of DeclOffset into 2 separate arrays. It could be a bit less efficient because 
of worse memory locality but will save some space.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76594/new/

https://reviews.llvm.org/D76594



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to