xbolva00 added inline comments.
================ Comment at: llvm/include/llvm/ADT/FoldingSet.h:328 /// Add* - Add various data types to Bit data. - void AddPointer(const void *Ptr); - void AddInteger(signed I); - void AddInteger(unsigned I); - void AddInteger(long I); - void AddInteger(unsigned long I); - void AddInteger(long long I); - void AddInteger(unsigned long long I); + void AddPointer(const void *Ptr) { + // Note: this adds pointers to the hash using sizes and endianness that ---------------- yurai007 wrote: > serge-sans-paille wrote: > > Concerning that inlined part, I expect LTO to close the gap instead of > > moving everything to headers. Do we have a policy on that topic? > I'm not aware of any LLVM Coding Guidlines policy, probably most related is > just general rule: > https://llvm.org/docs/CodingStandards.html#include-as-little-as-possible I > agree LTO is great when enabled. I just tried to move only small part which > matters. > Could you please elaborate what are you concerning about? > > The only potential risks which come to my mind right now are: > * increased binary size, however I noticed Clang binary grows only below > +0.1% which is acceptable I think. > * moving to header part of implementation which is often changed, however > AddPointer/AddInteger/ComputeHash were touched last time in 2012. > * compile time impact on Clang build time. I confess I didn't compare Clang > build times before and after change, but if you like I can. > * reduced I-cache hit rate. This is something also I didn't check under perf > but I can if you like (not sure how important it is given that we get drops > of other metrics). Is LLVM / Clang in distro releases even built with LTO / LTO + PGO? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118385/new/ https://reviews.llvm.org/D118385 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits