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

[Duncan Exon Smith]

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?

-------------- next part --------------
A non-text attachment was scrubbed...
Name: protect-runtime-library-functions-2.patch
Type: application/octet-stream
Size: 6633 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131111/c7c0a68c/attachment.obj>