Re: [PATCH][Middle-end]Add a new option to finer control inlining based on function's visibility

[Qing Zhao <qing dot zhao at oracle dot com>]

> On Sep 12, 2018, at 10:22 AM, Jeff Law <law@redhat.com> wrote:
> 
> On 9/11/18 9:12 AM, Qing Zhao wrote:
>> Hi, 
>> 
>> This is a simple patch to add a new first-class option 
>> 
>> -finline-visibility={all|extern|static}
>> 
>> to finer control inlining based on function’s visibility. 
>> 
>> '-finline-visibility=[all|extern|static]'
>>     By default, GCC inlines functions without considering their
>>     visibility.  This flag allows finer control of inlining based on
>>     their visibility.
>> 
>>     The value 'extern' tells the compiler to only inline functions with
>>     external visibility.  The value 'static' tells the compiler to only
>>     inline functions with static visibility.  The value 'all' tells the
>>     compilers to inline functions without considering their visibility.
>> 
>>     The default value of '-finline-visibility' is 'all'.
>> 
>> The major purpose of this option is to provide a way for the user 
>> to finer choose the inline candidates based on function’s visibility.
>> For example, some online patching users might want to limit the inlining
>> to only static functions to avoid patching the callers of global functions
>> in order to control the memory consumption caused by online patching. 
>> 
>> let me know any comments and suggestions.
>> 
>> thanks.
>> 
>> Qing
>> 
>> gcc/ChangeLog:
>> 
>> +2018-09-11  Qing Zhao  <qing.zhao@oracle.com>
>> +
>> +       * cif-code.def (FUNCTION_EXTERN): New CIFCODE.
>> +       (FUNCTION_STATIC): Likewise.
>> +       * common.opt (-finline-visibility=): New option.
>> +       * doc/invoke.texi: Document -finline-visibility=.
>> +       * flag-types.h: Add a new enum inline_visibility.
>> +       * ipa-inline.c (can_inline_edge_p): Control inlining based on
>> +       function's visibility. 
>> +
>> 
>> gcc/testsuite/ChangeLog:
>> 
>> +2018-09-11  Qing Zhao  <qing.zhao@oracle.com>
>> +
>> +       * gcc.dg/inline_visibility_1.c: New test.
>> +       * gcc.dg/inline_visibility_2.c: New test.
> Presumably one of the cases where this capability is really helpful is
> things like ksplice.   If you have a function with global scope that has
> been potentially inlined, then it's a lot harder track down those
> inlining points at DTRT.

exactly one of the use cases and motivation for this new functionality.  Thanks!

> 
> We ran into this internally when looking at hot patching some of the
> spinlock code in the kernel.  It would have been real helpful if the
> kernel had been compiled with this kind of option :-)
yes. 

the major purpose of this new option is to disable inlining of global functions to help
online patching.

> 
> So conceptually I can see value in this kind of option.
> 
> Not a review, just wanted to chime in on potential use scenarios…

thank you.

Qing
> 
> jeff