[PATCH] Prevent user-supplied libc implementations from premature removal in LTO
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>
> > +//===-- 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?
> > + 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 <
> 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?
Sorry, lost context on this. Sounds good!
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the llvm-commits