[llvm-dev] RFC: We need to explicitly state that some functions are reserved by LLVM

[Hal Finkel via llvm-dev]

On 10/28/2017 11:42 AM, Xinliang David Li wrote:
>
>
> On Fri, Oct 27, 2017 at 11:36 AM, Friedman, Eli via llvm-dev 
> <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote:
>
>     On 10/26/2017 9:01 PM, Hal Finkel via llvm-dev wrote:
>>
>>
>>     On 10/26/2017 10:56 PM, Chris Lattner via llvm-dev wrote:
>>>
>>>>     On Oct 26, 2017, at 8:14 PM, Chandler Carruth via llvm-dev
>>>>     <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote:
>>>>
>>>>
>>>>     One alternative that seems appealing but doesn't actually help
>>>>     would be to make `TargetLibraryInfo` ignore internal functions.
>>>>     That is how the C++ spec seems to handle this for example (C
>>>>     library function names are reserved only when they have
>>>>     linkage). But this doesn't work well for LLVM because we want
>>>>     to be able to LTO an internalized C library. So I think we need
>>>>     the rule for LLVM function names to not rely on linkage here.
>>>
>>>     Oh sorry, (almost) TLDR I didn’t get to this part.  I don’t see
>>>     how this is applicable.  If you’re statically linking in a libc,
>>>     I think it is fine to forgo the optimizations
>>>     that TargetLibraryInfo is all about.
>>>
>>>     If these transformations are important to use in this case, we
>>>     should invent a new attribute, and the thing that turns libc
>>>     symbols into internal ones should add the attribute to the (now
>>>     internal) libc symbols.
>>
>>     I'm not sure; some of the transformations are somewhat special
>>     (e.g., based on mathematical properties, or things like printf ->
>>     puts translation). LTO alone certainly won't give you those kinds
>>     of things via normal IPA, and I doubt we want attributes for all
>>     of them. Also, having LTO essentially disable optimizations isn't
>>     good either.
>
>     Given the way the optimization pipeline works; we can't treat an
>     "internal" function as equivalent to a C library function.  When
>     the linkage of a function becomes "internal", optimizations start
>     kicking in based on the fact that we can see all the users of the
>     function.
>
>     For example, suppose my program has one call to puts with the
>     constant string "foo", and one call to printf which can be
>     transformed into a call to puts, and we LTO the C library into
>     it.  First we run IPSCCP, which will constant-propagate the
>     address of the string into the implementation of puts.  Then
>     instcombine runs and transforms the call to printf into a call to
>     puts.  Now we have a miscompile, because our "puts" can only
>     output "foo".
>
>
>
> Slightly off topic. This particular example probably just shows the 
> issue in implementation. A more robust way to implement this is to 
> 'clone' the original function and privatize it instead of the original 
> function.  The removal of the original function can be delayed till 
> the real link time when linker sees no references to it.  This is also 
> a more flexible scheme as LTO does not need to operate in strict whole 
> program mode, nor does it need to query linker to see if the function 
> is referenced by other library functions not visible to the compiler 
> (in IR form), or there is reference to the symbol through dlopen/dlsym ..

I agree, cloning would be good. I'll mention, however, that there are 
benefits to dropping the unused original functions early that we should 
likely endeavor not to lose (e.g., dropping the unused original function 
may lead to optimization opportunities for global variables and the like).

More-aggressive cloning will also help with our problems optimizing, and 
using IPA on, functions with inline linkage.

  -Hal

>
> David
>
>
>     Given we have mutually exclusive optimizations, we have to pick:
>     either we allow the IPSCCP transform, or we allow the instcombine
>     transform.  The most straightforward way to indicate the
>     difference is to check the linkage: it intuitively has the right
>     meaning, and our existing inter-procedural optimizations already
>     check it.
>
>     -Eli
>
>     -- 
>     Employee of Qualcomm Innovation Center, Inc.
>     Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
>
>
>     _______________________________________________
>     LLVM Developers mailing list
>     llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>
>     http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>     <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>
>
>

-- 
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20171028/0961c728/attachment.html>