[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