[PATCH] Prevent user-supplied libc implementations from premature removal in LTO

Nick Lewycky nlewycky at google.com
Tue Nov 12 19:39:39 PST 2013


On 11 November 2013 14:11, Duncan Exon Smith <dexonsmith at apple.com> wrote:

> Thanks for the new review!  Updated patch attached.  See comments inline.
>
> On Nov 11, 2013, at 12:50 PM, Rafael Espíndola <rafael.espindola at gmail.com>
> wrote:
>
> > +//===-- CodeGen/RuntimeLibcalls.h - Runtime Library Calls --------*-
> > C++ -*-===//
> >
> > Nice cleanup, but unrelated. Please commit it independently.
>
> Removed it.
>
> > +                        const std::vector<llvm::StringRef> &Libcalls,
> >
> > Can you use an ArrayRef?
>
> Fixed.
>
> > +  std::sort(Libcalls.begin(), Libcalls.end());
> > +  Libcalls.erase(std::unique(Libcalls.begin(), Libcalls.end()),
> > +                 Libcalls.end());
> >
> > What is the size of this std::vector? Would a StringSet be better?
>
> I see ~600 entries for RTLIB::Libcall and LibFunc::Func combined.  I don’t
> have a good sense of how StringSet and std::vector compare at this size, so
> I’ll defer to your judgement.  I’ve left it as std::vector for now.
>
> Typically I default to std::vector until I see a performance problem, and <
> http://llvm.org/docs/ProgrammersManual.html#dss-sortedvectorset>
> suggested my pattern was okay.  However, I missed the discussion of
> StringSet/StringMap — should I use StringSet by default for strings?
>
> > +; RUN: FileCheck <%t3 %s -check-prefix=LIBC
> > +; RUN: FileCheck <%t3 %s -check-prefix=RT
> > +; RUN: FileCheck <%t3 %s -check-prefix=BOTH
> > +; RUN: FileCheck <%t3 %s -check-prefix=NEITHER
> >
> > You only need 2 prefixes, one for the keep and one for the NOT.
> > FileCheck also just got support for multiple prefixes in one run, so
> > you should be able to use just one FileCheck invocation with two
> > -check-prefix.
>
> Heh… I’d missed the llvm-nm -no-sort option.  Fixed.
>
> > Nick, what do you think of the idea of using TargetLibraryInfo and
> > TargetLowering?
>
> Nick?
>

Sorry, lost context on this. Sounds good!

Nick
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131112/b9b1df30/attachment.html>


More information about the llvm-commits mailing list