Re: [PATCH][Middle-end][Version 4]Add a new option to control inlining only on static functions

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

> On Sep 26, 2018, at 12:16 PM, Jan Hubicka <hubicka@ucw.cz <mailto:hubicka@ucw.cz>> wrote:
> 
>>> 
>>> On Sep 26, 2018, at 10:06 AM, Jan Hubicka <hubicka@ucw.cz <mailto:hubicka@ucw.cz>> wrote:
>>> 
>>>> On Wed, Sep 26, 2018 at 10:45 AM, Jeff Law <law@redhat.com <mailto:law@redhat.com>> wrote:
>>>>> On 9/26/18 7:38 AM, Jason Merrill wrote:
>>>>>> On Wed, Sep 26, 2018 at 9:24 AM, Richard Biener <rguenther@suse.de <mailto:rguenther@suse.de>> wrote:
>>>>>>> IIRC he explicitely wanted 'static' not 'hidden' linkage.  Not sure
>>>>>>> what 'internal' would mean in this context.
>>>>>> 
>>>>>> I mean internal linkage as in the C and C++ standards.
>>>> 
>>>>> Since this is primarily for kernel hot patching, I think we're looking
>>>>> to restrict inlining to functions that have visibility limited to a
>>>>> compilation unit.
>>>> 
>>>> Right, which is internal linkage.
>>>> 
>>>> C11: "Within one translation unit, each declaration of an identifier
>>>> with internal linkage denotes the same object or function."
>>>> C++17: "When a name has internal linkage, the entity it denotes can be
>>>> referred to by names from other scopes in the same translation unit."
>>>> 
>>>> Or perhaps we want to say "not external linkage", i.e. !TREE_PUBLIC as
>>>> richi suggested.
>>> 
>>> I am not quite sure if we can relate visibility flags we have at this stage
>>> to visibility in source languge in very coherent way.  They change a lot with
>>> LTO and we may want to make this option incompatible with LTO, but even w/o
>>> we can turn function static that previously wasn’t.
>> 
>> Looks like both LTO and whole_program need to be made incompatible with -finline-only-static. 
>> from my study of the function “cgraph_externally_visible_p”, comdat functions can ONLY be turned into
>> static when either “in_lto_p” or “whole_program” is true. 
> 
> This is not quite the case, because, we can still clone them to static functions
> or derive fact about their side effects (such that information if they write/read
> memory, particular global var etc).  All this inter-procedural propagation
> is guarded by get_availability predicate returning at least AVAILABLE.

Okay, I see.

> 
> If you make this to be INTERPOSABLE (which means it can be replaced by different
> implementation by linker and that is probably what we want for live patching)
> then also inliner, ipa-sra and other optimization will give up on these.

do you suggest that to set the global function as AVAIL_INTERPOSABLE when -finline-only-static 
is present? then we should avoid all issues?

Qing


> Honza
>> 
>> 
>>> For example comdat that was cloned by IPA-SRA. See can_be_local_p and
>>> comdat_can_be_unshared_p predicates.  Similar problem happens to clones created
>>> by ipa-cp.
>>> 
>>> I guess we want to disable localization and cloning in this case as well.
>>> I wonder what else.
>> 
>> Yes, I think we should make -finline-only-static incompatible with cloning and tree-sra too.
>> 
>> Qing
>>> 
>>> Honza