serge-sans-paille 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
----------------
xbolva00 wrote:
> 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?
In fedora, LLVM is compiled with LTO. But I agree that's not an assumption we
should make, and thus moving some functions to the header looks ok.
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
