IPA: support -flto + -flive-patching=inline-clone

  There's no fundamental reason why -flive-patching=inline-clone can't
coexist with -flto. Yes, one can theoretically have many more clone
function that includes a live patch. It is pretty much the same
as in-module inlining.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ready to be installed?
Thanks,
Martin

gcc/ChangeLog:

	* opts.cc (finish_options): Print sorry message only
	for -flive-patching=inline-only-static.

gcc/testsuite/ChangeLog:

	* gcc.dg/live-patching-2.c: Update scanned pattern.
	* gcc.dg/live-patching-5.c: New test.
---
 gcc/opts.cc                            | 5 +++--
 gcc/testsuite/gcc.dg/live-patching-2.c | 4 ++--
 gcc/testsuite/gcc.dg/live-patching-5.c | 8 ++++++++
 3 files changed, 13 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/live-patching-5.c
  

Hi, Martin:


I have two questions on this:

1.  What’s the motivation to enable -flive-patching with -flto? Is there any application that will try -flive-patching with -flto now?

2. Why only enable -flive-patching=inline-clone with -flto?

thanks.

Qing

> On Oct 5, 2022, at 7:41 AM, Martin Liška <mliska@suse.cz> wrote:
> 
> There's no fundamental reason why -flive-patching=inline-clone can't
> coexist with -flto. Yes, one can theoretically have many more clone
> function that includes a live patch. It is pretty much the same
> as in-module inlining.
> 
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> 
> Ready to be installed?
> Thanks,
> Martin
> 
> gcc/ChangeLog:
> 
> 	* opts.cc (finish_options): Print sorry message only
> 	for -flive-patching=inline-only-static.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.dg/live-patching-2.c: Update scanned pattern.
> 	* gcc.dg/live-patching-5.c: New test.
> ---
> gcc/opts.cc                            | 5 +++--
> gcc/testsuite/gcc.dg/live-patching-2.c | 4 ++--
> gcc/testsuite/gcc.dg/live-patching-5.c | 8 ++++++++
> 3 files changed, 13 insertions(+), 4 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/live-patching-5.c
> 
> diff --git a/gcc/opts.cc b/gcc/opts.cc
> index eb5db01de17..ae079fcd20e 100644
> --- a/gcc/opts.cc
> +++ b/gcc/opts.cc
> @@ -1288,8 +1288,9 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set,
> 	   "%<-fsanitize=kernel-address%>");
> 
>   /* Currently live patching is not support for LTO.  */
> -  if (opts->x_flag_live_patching && opts->x_flag_lto)
> -    sorry ("live patching is not supported with LTO");
> +  if (opts->x_flag_live_patching == LIVE_PATCHING_INLINE_ONLY_STATIC && opts->x_flag_lto)
> +    sorry ("live patching (with %qs) is not supported with LTO",
> +	   "inline-only-static");
> 
>   /* Currently vtable verification is not supported for LTO */
>   if (opts->x_flag_vtable_verify && opts->x_flag_lto)
> diff --git a/gcc/testsuite/gcc.dg/live-patching-2.c b/gcc/testsuite/gcc.dg/live-patching-2.c
> index 0dde4e9e0c0..1c4f9229b82 100644
> --- a/gcc/testsuite/gcc.dg/live-patching-2.c
> +++ b/gcc/testsuite/gcc.dg/live-patching-2.c
> @@ -1,10 +1,10 @@
> /* { dg-do compile } */
> /* { dg-require-effective-target lto } */
> -/* { dg-options "-O2 -flive-patching -flto" } */
> +/* { dg-options "-O2 -flive-patching=inline-only-static -flto" } */
> 
> int main()
> {
>   return 0;
> }
> 
> -/* { dg-message "sorry, unimplemented: live patching is not supported with LTO" "-flive-patching and -flto together" { target *-*-* } 0 } */
> +/* { dg-message "sorry, unimplemented: live patching \\(with 'inline-only-static'\\) is not supported with LTO" "-flive-patching and -flto together" { target *-*-* } 0 } */
> diff --git a/gcc/testsuite/gcc.dg/live-patching-5.c b/gcc/testsuite/gcc.dg/live-patching-5.c
> new file mode 100644
> index 00000000000..098047a36cd
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/live-patching-5.c
> @@ -0,0 +1,8 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target lto } */
> +/* { dg-options "-O2 -flive-patching -flto" } */
> +
> +int main()
> +{
> +  return 0;
> +}
> -- 
> 2.37.3
>
  

On 10/5/22 16:50, Qing Zhao wrote:
> I have two questions on this:

Hello.

> 
> 1.  What’s the motivation to enable -flive-patching with -flto? Is there any application that will try -flive-patching with -flto now?

We're planning supporting GCC LTO Linux kernel support, so that's one motivation. And the second one is a possible
use in user-space livepatching. Note majority of modern distros default to -flto (openSUSE, Fedora, Debian, Ubuntu, ...).

> 
> 2. Why only enable -flive-patching=inline-clone with -flto?

Because the inline-only-static level (which you added/requested) would have to properly
block inter-procedural inlining that happens in LTO (can_inline_edge_by_limits_p) and
I'm not sure it would be properly blocked. So, feel free to extend my patch if you want?

Martin

> 
> thanks.
  

> On Oct 5, 2022, at 1:36 PM, Martin Liška <mliska@suse.cz> wrote:
> 
> On 10/5/22 16:50, Qing Zhao wrote:
>> I have two questions on this:
> 
> Hello.
> 
>> 
>> 1.  What’s the motivation to enable -flive-patching with -flto? Is there any application that will try -flive-patching with -flto now?
> 
> We're planning supporting GCC LTO Linux kernel support, so that's one motivation. And the second one is a possible
> use in user-space livepatching. Note majority of modern distros default to -flto (openSUSE, Fedora, Debian, Ubuntu, ...).

Okay, I see. That’s reasonable.
> 
>> 
>> 2. Why only enable -flive-patching=inline-clone with -flto?
> 
> Because the inline-only-static level (which you added/requested) would have to properly
> block inter-procedural inlining that happens in LTO (can_inline_edge_by_limits_p) and
> I'm not sure it would be properly blocked. So, feel free to extend my patch if you want?

-flive-patching=inline-only-static

Only enable static functions inlining,  all the inlining of external visible functions are blocked, So, LTO should be compatible with this naturally without any issue, I think.

i.e, when "-flive-patching=inline-only-static -flto"  present together, all the inter-procedural inlining should be automatically blocked by -flive-patching=inline-only-static already.

Do I miss anything here?

thanks.

Qing

> 
> Martin
> 
>> 
>> thanks.
>
  

On Wed, Oct 5, 2022 at 8:18 PM Qing Zhao via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
>
>
> > On Oct 5, 2022, at 1:36 PM, Martin Liška <mliska@suse.cz> wrote:
> >
> > On 10/5/22 16:50, Qing Zhao wrote:
> >> I have two questions on this:
> >
> > Hello.
> >
> >>
> >> 1.  What’s the motivation to enable -flive-patching with -flto? Is there any application that will try -flive-patching with -flto now?
> >
> > We're planning supporting GCC LTO Linux kernel support, so that's one motivation. And the second one is a possible
> > use in user-space livepatching. Note majority of modern distros default to -flto (openSUSE, Fedora, Debian, Ubuntu, ...).
>
> Okay, I see. That’s reasonable.
> >
> >>
> >> 2. Why only enable -flive-patching=inline-clone with -flto?
> >
> > Because the inline-only-static level (which you added/requested) would have to properly
> > block inter-procedural inlining that happens in LTO (can_inline_edge_by_limits_p) and
> > I'm not sure it would be properly blocked. So, feel free to extend my patch if you want?
>
> -flive-patching=inline-only-static
>
> Only enable static functions inlining,  all the inlining of external visible functions are blocked, So, LTO should be compatible with this naturally without any issue, I think.
>
> i.e, when "-flive-patching=inline-only-static -flto"  present together, all the inter-procedural inlining should be automatically blocked by -flive-patching=inline-only-static already.
>
> Do I miss anything here?

WPA will promote externally visible functions static when all accesses
are from LTO IR, I don't think we preserve
the "original" visibility for IPA inlining heuristics.

OTOH inline-only-static could disable WPA inlining and do all inlining early ...

Richard,

>
> thanks.
>
> Qing
>
> >
> > Martin
> >
> >>
> >> thanks.
> >
>
  

On 10/6/22 10:29, Richard Biener wrote:
> On Wed, Oct 5, 2022 at 8:18 PM Qing Zhao via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>>
>>
>>
>>> On Oct 5, 2022, at 1:36 PM, Martin Liška <mliska@suse.cz> wrote:
>>>
>>> On 10/5/22 16:50, Qing Zhao wrote:
>>>> I have two questions on this:
>>>
>>> Hello.
>>>
>>>>
>>>> 1.  What’s the motivation to enable -flive-patching with -flto? Is there any application that will try -flive-patching with -flto now?
>>>
>>> We're planning supporting GCC LTO Linux kernel support, so that's one motivation. And the second one is a possible
>>> use in user-space livepatching. Note majority of modern distros default to -flto (openSUSE, Fedora, Debian, Ubuntu, ...).
>>
>> Okay, I see. That’s reasonable.
>>>
>>>>
>>>> 2. Why only enable -flive-patching=inline-clone with -flto?
>>>
>>> Because the inline-only-static level (which you added/requested) would have to properly
>>> block inter-procedural inlining that happens in LTO (can_inline_edge_by_limits_p) and
>>> I'm not sure it would be properly blocked. So, feel free to extend my patch if you want?
>>
>> -flive-patching=inline-only-static
>>
>> Only enable static functions inlining,  all the inlining of external visible functions are blocked, So, LTO should be compatible with this naturally without any issue, I think.
>>
>> i.e, when "-flive-patching=inline-only-static -flto"  present together, all the inter-procedural inlining should be automatically blocked by -flive-patching=inline-only-static already.
>>
>> Do I miss anything here?
> 
> WPA will promote externally visible functions static when all accesses
> are from LTO IR, I don't think we preserve
> the "original" visibility for IPA inlining heuristics.

That said, can we go with my original patch?

> 
> OTOH inline-only-static could disable WPA inlining and do all inlining early ...

And this can be extended by Oracle folks if they are interested in -flive-patching=inline-only-static
combined with LTO.

Martin

> 
> Richard,
> 
>>
>> thanks.
>>
>> Qing
>>
>>>
>>> Martin
>>>
>>>>
>>>> thanks.
>>>
>>
  

> On Oct 6, 2022, at 4:29 AM, Richard Biener <richard.guenther@gmail.com> wrote:
> 
> On Wed, Oct 5, 2022 at 8:18 PM Qing Zhao via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>> 
>> 
>> 
>>> On Oct 5, 2022, at 1:36 PM, Martin Liška <mliska@suse.cz> wrote:
>>> 
>>> On 10/5/22 16:50, Qing Zhao wrote:
>>>> I have two questions on this:
>>> 
>>> Hello.
>>> 
>>>> 
>>>> 1.  What’s the motivation to enable -flive-patching with -flto? Is there any application that will try -flive-patching with -flto now?
>>> 
>>> We're planning supporting GCC LTO Linux kernel support, so that's one motivation. And the second one is a possible
>>> use in user-space livepatching. Note majority of modern distros default to -flto (openSUSE, Fedora, Debian, Ubuntu, ...).
>> 
>> Okay, I see. That’s reasonable.
>>> 
>>>> 
>>>> 2. Why only enable -flive-patching=inline-clone with -flto?
>>> 
>>> Because the inline-only-static level (which you added/requested) would have to properly
>>> block inter-procedural inlining that happens in LTO (can_inline_edge_by_limits_p) and
>>> I'm not sure it would be properly blocked. So, feel free to extend my patch if you want?
>> 
>> -flive-patching=inline-only-static
>> 
>> Only enable static functions inlining,  all the inlining of external visible functions are blocked, So, LTO should be compatible with this naturally without any issue, I think.
>> 
>> i.e, when "-flive-patching=inline-only-static -flto"  present together, all the inter-procedural inlining should be automatically blocked by -flive-patching=inline-only-static already.
>> 
>> Do I miss anything here?
> 
> WPA will promote externally visible functions static when all accesses
> are from LTO IR, I don't think we preserve
> the "original" visibility for IPA inlining heuristics.

WPA is Whole Program Analysis?
Okay, then  It will promote all static function to extern functions. That’s reasonable.

Is it hard to preserve the original “static” visibility in the IR?
> 
> OTOH inline-only-static could disable WPA inlining and do all inlining early ...

Inline-only-static ONLY inlines static functions, how can it disable WPA inlining? Don’t quite understand here.

thanks.

Qing
> 
> Richard,
> 
>> 
>> thanks.
>> 
>> Qing
>> 
>>> 
>>> Martin
>>> 
>>>> 
>>>> thanks.
  

On Thu, Oct 6, 2022 at 3:18 PM Qing Zhao <qing.zhao@oracle.com> wrote:
>
>
>
> > On Oct 6, 2022, at 4:29 AM, Richard Biener <richard.guenther@gmail.com> wrote:
> >
> > On Wed, Oct 5, 2022 at 8:18 PM Qing Zhao via Gcc-patches
> > <gcc-patches@gcc.gnu.org> wrote:
> >>
> >>
> >>
> >>> On Oct 5, 2022, at 1:36 PM, Martin Liška <mliska@suse.cz> wrote:
> >>>
> >>> On 10/5/22 16:50, Qing Zhao wrote:
> >>>> I have two questions on this:
> >>>
> >>> Hello.
> >>>
> >>>>
> >>>> 1.  What’s the motivation to enable -flive-patching with -flto? Is there any application that will try -flive-patching with -flto now?
> >>>
> >>> We're planning supporting GCC LTO Linux kernel support, so that's one motivation. And the second one is a possible
> >>> use in user-space livepatching. Note majority of modern distros default to -flto (openSUSE, Fedora, Debian, Ubuntu, ...).
> >>
> >> Okay, I see. That’s reasonable.
> >>>
> >>>>
> >>>> 2. Why only enable -flive-patching=inline-clone with -flto?
> >>>
> >>> Because the inline-only-static level (which you added/requested) would have to properly
> >>> block inter-procedural inlining that happens in LTO (can_inline_edge_by_limits_p) and
> >>> I'm not sure it would be properly blocked. So, feel free to extend my patch if you want?
> >>
> >> -flive-patching=inline-only-static
> >>
> >> Only enable static functions inlining,  all the inlining of external visible functions are blocked, So, LTO should be compatible with this naturally without any issue, I think.
> >>
> >> i.e, when "-flive-patching=inline-only-static -flto"  present together, all the inter-procedural inlining should be automatically blocked by -flive-patching=inline-only-static already.
> >>
> >> Do I miss anything here?
> >
> > WPA will promote externally visible functions static when all accesses
> > are from LTO IR, I don't think we preserve
> > the "original" visibility for IPA inlining heuristics.
>
> WPA is Whole Program Analysis?

Yes.

> Okay, then  It will promote all static function to extern functions. That’s reasonable.

No, all extern functions to static functions.

> Is it hard to preserve the original “static” visibility in the IR?

Probably not hard, and the IPA pass adjusting visbility could as well
mark the functions
as not to be inlined with -flive-patching=inline-only-static.

> >
> > OTOH inline-only-static could disable WPA inlining and do all inlining early ...
>
> Inline-only-static ONLY inlines static functions, how can it disable WPA inlining? Don’t quite understand here.

it's a flag so it can be used to control other things

> thanks.
>
> Qing
> >
> > Richard,
> >
> >>
> >> thanks.
> >>
> >> Qing
> >>
> >>>
> >>> Martin
> >>>
> >>>>
> >>>> thanks.
>
  

> > WPA is Whole Program Analysis?
> 
> Yes.
> 
> > Okay, then  It will promote all static function to extern functions. That’s reasonable.
> 
> No, all extern functions to static functions.
> 
> > Is it hard to preserve the original “static” visibility in the IR?
> 
> Probably not hard, and the IPA pass adjusting visbility could as well
> mark the functions
> as not to be inlined with -flive-patching=inline-only-static.
> 
> > >
> > > OTOH inline-only-static could disable WPA inlining and do all inlining early ...
> >
> > Inline-only-static ONLY inlines static functions, how can it disable WPA inlining? Don’t quite understand here.
> 
> it's a flag so it can be used to control other things

GCC has two inliners
 1) ealry inlininer which happens at compile time and is quite
 restricted only to obvious cases (always_inline, flatten and very small
 functions)
 2) IPA inlining happening at link-time (WPA) which is using greedy
 algorithm and makes more complicated code size/speed tradeoffs
Indeed betwen 1 and 2 previously global functions may become static by
resolution info (they won't currently with kernel since we do
incremental linking).  We could easily keep track of originally static
functions and promoted to static functions and make IPA inlining to
honnor the patch.

I however wonder how much LTO optimization would remain. If we disable
all inter-module inlining and with live patching we also disable most of
other optimization, I think basically only unreachable code removal will
remain and possibly some propagation of "coldness" across the code.

I can implement this incrementally.
Martin, if live patching is happy about some symbols being promoted
static, the patch is OK.
Honza
  

> On Oct 7, 2022, at 2:34 AM, Richard Biener <richard.guenther@gmail.com> wrote:
> 
> On Thu, Oct 6, 2022 at 3:18 PM Qing Zhao <qing.zhao@oracle.com> wrote:
>> 
>> 
>> 
>>> On Oct 6, 2022, at 4:29 AM, Richard Biener <richard.guenther@gmail.com> wrote:
>>> 
>>> On Wed, Oct 5, 2022 at 8:18 PM Qing Zhao via Gcc-patches
>>> <gcc-patches@gcc.gnu.org> wrote:
>>>> 
>>>> 
>>>> 
>>>>> On Oct 5, 2022, at 1:36 PM, Martin Liška <mliska@suse.cz> wrote:
>>>>> 
>>>>> On 10/5/22 16:50, Qing Zhao wrote:
>>>>>> I have two questions on this:
>>>>> 
>>>>> Hello.
>>>>> 
>>>>>> 
>>>>>> 1.  What’s the motivation to enable -flive-patching with -flto? Is there any application that will try -flive-patching with -flto now?
>>>>> 
>>>>> We're planning supporting GCC LTO Linux kernel support, so that's one motivation. And the second one is a possible
>>>>> use in user-space livepatching. Note majority of modern distros default to -flto (openSUSE, Fedora, Debian, Ubuntu, ...).
>>>> 
>>>> Okay, I see. That’s reasonable.
>>>>> 
>>>>>> 
>>>>>> 2. Why only enable -flive-patching=inline-clone with -flto?
>>>>> 
>>>>> Because the inline-only-static level (which you added/requested) would have to properly
>>>>> block inter-procedural inlining that happens in LTO (can_inline_edge_by_limits_p) and
>>>>> I'm not sure it would be properly blocked. So, feel free to extend my patch if you want?
>>>> 
>>>> -flive-patching=inline-only-static
>>>> 
>>>> Only enable static functions inlining,  all the inlining of external visible functions are blocked, So, LTO should be compatible with this naturally without any issue, I think.
>>>> 
>>>> i.e, when "-flive-patching=inline-only-static -flto"  present together, all the inter-procedural inlining should be automatically blocked by -flive-patching=inline-only-static already.
>>>> 
>>>> Do I miss anything here?
>>> 
>>> WPA will promote externally visible functions static when all accesses
>>> are from LTO IR, I don't think we preserve
>>> the "original" visibility for IPA inlining heuristics.
>> 
>> WPA is Whole Program Analysis?
> 
> Yes.
> 
>> Okay, then  It will promote all static function to extern functions. That’s reasonable.
> 
> No, all extern functions to static functions.

Oh, really? Why change “extern” to “static”?  (I recall that studio compiler promote static to extern for inter-procedural inlining)
> 
>> Is it hard to preserve the original “static” visibility in the IR?
> 
> Probably not hard, and the IPA pass adjusting visbility could as well
> mark the functions
> as not to be inlined with -flive-patching=inline-only-static.
Okay, then the implementation should be doable? 
> 
>>> 
>>> OTOH inline-only-static could disable WPA inlining and do all inlining early ...
>> 
>> Inline-only-static ONLY inlines static functions, how can it disable WPA inlining? Don’t quite understand here.
> 
> it's a flag so it can be used to control other things. 

When -flive-patching=inline-only-static is specified by the user, user explicitly request ONLY inlining static functions. 
Even when LTO is enabled, if only static function inlining is enabled, user gets what he/she want. So I didn’t see any issue here. 

Let me know if I still miss anything here.

Thanks.

Qing


> 
>> thanks.
>> 
>> Qing
>>> 
>>> Richard,
>>> 
>>>> 
>>>> thanks.
>>>> 
>>>> Qing
>>>> 
>>>>> 
>>>>> Martin
>>>>> 
>>>>>> 
>>>>>> thanks.
  

On 10/7/22 15:04, Qing Zhao wrote:
> 
> 
>> On Oct 7, 2022, at 2:34 AM, Richard Biener <richard.guenther@gmail.com> wrote:
>>
>> On Thu, Oct 6, 2022 at 3:18 PM Qing Zhao <qing.zhao@oracle.com> wrote:
>>>
>>>
>>>
>>>> On Oct 6, 2022, at 4:29 AM, Richard Biener <richard.guenther@gmail.com> wrote:
>>>>
>>>> On Wed, Oct 5, 2022 at 8:18 PM Qing Zhao via Gcc-patches
>>>> <gcc-patches@gcc.gnu.org> wrote:
>>>>>
>>>>>
>>>>>
>>>>>> On Oct 5, 2022, at 1:36 PM, Martin Liška <mliska@suse.cz> wrote:
>>>>>>
>>>>>> On 10/5/22 16:50, Qing Zhao wrote:
>>>>>>> I have two questions on this:
>>>>>>
>>>>>> Hello.
>>>>>>
>>>>>>>
>>>>>>> 1.  What’s the motivation to enable -flive-patching with -flto? Is there any application that will try -flive-patching with -flto now?
>>>>>>
>>>>>> We're planning supporting GCC LTO Linux kernel support, so that's one motivation. And the second one is a possible
>>>>>> use in user-space livepatching. Note majority of modern distros default to -flto (openSUSE, Fedora, Debian, Ubuntu, ...).
>>>>>
>>>>> Okay, I see. That’s reasonable.
>>>>>>
>>>>>>>
>>>>>>> 2. Why only enable -flive-patching=inline-clone with -flto?
>>>>>>
>>>>>> Because the inline-only-static level (which you added/requested) would have to properly
>>>>>> block inter-procedural inlining that happens in LTO (can_inline_edge_by_limits_p) and
>>>>>> I'm not sure it would be properly blocked. So, feel free to extend my patch if you want?
>>>>>
>>>>> -flive-patching=inline-only-static
>>>>>
>>>>> Only enable static functions inlining,  all the inlining of external visible functions are blocked, So, LTO should be compatible with this naturally without any issue, I think.
>>>>>
>>>>> i.e, when "-flive-patching=inline-only-static -flto"  present together, all the inter-procedural inlining should be automatically blocked by -flive-patching=inline-only-static already.
>>>>>
>>>>> Do I miss anything here?
>>>>
>>>> WPA will promote externally visible functions static when all accesses
>>>> are from LTO IR, I don't think we preserve
>>>> the "original" visibility for IPA inlining heuristics.
>>>
>>> WPA is Whole Program Analysis?
>>
>> Yes.
>>
>>> Okay, then  It will promote all static function to extern functions. That’s reasonable.
>>
>> No, all extern functions to static functions.
> 
> Oh, really? Why change “extern” to “static”?  (I recall that studio compiler promote static to extern for inter-procedural inlining)

Because the linker LTO plug-in can get information about symbols are really
and the thus some of them can before static and not visible.

>>
>>> Is it hard to preserve the original “static” visibility in the IR?
>>
>> Probably not hard, and the IPA pass adjusting visbility could as well
>> mark the functions
>> as not to be inlined with -flive-patching=inline-only-static.
> Okay, then the implementation should be doable?
>>
>>>>
>>>> OTOH inline-only-static could disable WPA inlining and do all inlining early ...
>>>
>>> Inline-only-static ONLY inlines static functions, how can it disable WPA inlining? Don’t quite understand here.
>>
>> it's a flag so it can be used to control other things.
> 
> When -flive-patching=inline-only-static is specified by the user, user explicitly request ONLY inlining static functions.
> Even when LTO is enabled, if only static function inlining is enabled, user gets what he/she want. So I didn’t see any issue here.

Please see Honza's previous email about the second inliner.

Anyway, I'm going to install my patch that handles inline-clone option value.

Martin

> 
> Let me know if I still miss anything here> 
> Thanks.
> 
> Qing
> 
> 
>>
>>> thanks.
>>>
>>> Qing
>>>>
>>>> Richard,
>>>>
>>>>>
>>>>> thanks.
>>>>>
>>>>> Qing
>>>>>
>>>>>>
>>>>>> Martin
>>>>>>
>>>>>>>
>>>>>>> thanks.
>
  

> On Oct 7, 2022, at 9:03 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
> 
>>> WPA is Whole Program Analysis?
>> 
>> Yes.
>> 
>>> Okay, then  It will promote all static function to extern functions. That’s reasonable.
>> 
>> No, all extern functions to static functions.
>> 
>>> Is it hard to preserve the original “static” visibility in the IR?
>> 
>> Probably not hard, and the IPA pass adjusting visbility could as well
>> mark the functions
>> as not to be inlined with -flive-patching=inline-only-static.
>> 
>>>> 
>>>> OTOH inline-only-static could disable WPA inlining and do all inlining early ...
>>> 
>>> Inline-only-static ONLY inlines static functions, how can it disable WPA inlining? Don’t quite understand here.
>> 
>> it's a flag so it can be used to control other things
> 
> GCC has two inliners
> 1) ealry inlininer which happens at compile time and is quite
> restricted only to obvious cases (always_inline, flatten and very small
> functions)
> 2) IPA inlining happening at link-time (WPA) which is using greedy
> algorithm and makes more complicated code size/speed tradeoffs
> Indeed betwen 1 and 2 previously global functions may become static by
> resolution info (they won't currently with kernel since we do
> incremental linking).  We could easily keep track of originally static
> functions and promoted to static functions and make IPA inlining to
> honnor the patch.

Yes, this is similar as Studio compiler (an early inliner and a IPA inliner) 
But I still don’t quite understand why during IPA inlining, extern functions need to be changed to static functions?
 (in Studio compiler, it’s opposite, static functions are all promoted to extern functions to enable inter-procedural inlining)
Is there a file I can read to understand more details on this?

> 
> I however wonder how much LTO optimization would remain. If we disable
> all inter-module inlining

Oh, wait,  so, demoting “extern” functions to “static” functions in GCC’s IPA inlining is to disable inter-module inlining? 
Why? Is there any technical issue with inter-module inlining in GCC? 


> and with live patching we also disable most of
> other optimization,

Yes, with live-patching, most of the IPA optimization need to be disabled. But this functionality is needed, right? When user requests live-patching support, 
They should know that most of IPA optimization will be disabled.

Qing

> I think basically only unreachable code removal will
> remain and possibly some propagation of "coldness" across the code.
> 
> I can implement this incrementally.
> Martin, if live patching is happy about some symbols being promoted
> static, the patch is OK.
> Honza
  

> >> Probably not hard, and the IPA pass adjusting visbility could as well
> >> mark the functions
> >> as not to be inlined with -flive-patching=inline-only-static.
> >> 
> >>>> 
> >>>> OTOH inline-only-static could disable WPA inlining and do all inlining early ...
> >>> 
> >>> Inline-only-static ONLY inlines static functions, how can it disable WPA inlining? Don’t quite understand here.
> >> 
> >> it's a flag so it can be used to control other things
> > 
> > GCC has two inliners
> > 1) ealry inlininer which happens at compile time and is quite
> > restricted only to obvious cases (always_inline, flatten and very small
> > functions)
> > 2) IPA inlining happening at link-time (WPA) which is using greedy
> > algorithm and makes more complicated code size/speed tradeoffs
> > Indeed betwen 1 and 2 previously global functions may become static by
> > resolution info (they won't currently with kernel since we do
> > incremental linking).  We could easily keep track of originally static
> > functions and promoted to static functions and make IPA inlining to
> > honnor the patch.
	       ^^^^ I mean -flive-patching=inline-only-static flag
> 
> Yes, this is similar as Studio compiler (an early inliner and a IPA inliner) 
> But I still don’t quite understand why during IPA inlining, extern functions need to be changed to static functions?
>  (in Studio compiler, it’s opposite, static functions are all promoted to extern functions to enable inter-procedural inlining)
> Is there a file I can read to understand more details on this?

In GCC WPA stage takes all compilation units and create a new combined
translation unit.  This has same kind of symbol table as if it came all
from a single source file.  So if resolution file tells us that a given
symbol is not used outside the LTO bytecode (when one links final binary
linker knows if there was other use and similar for shared libraries if
symbol is hidden) we promote symbol to static.  This lets us to optimize
it better: change calling conventions, remove offline copy if all calls
was inlined, propagate various information about it.

Since from this moment on the whole translation unit behaves as single
source file it is not problem to inline static functions cross-modle.

Later we partition the combined unit to do rest of compilation in
parallel and at this stage some static symbols can be changed to hidden
symbols if they are used by multiple partitioning.
> 
> > 
> > I however wonder how much LTO optimization would remain. If we disable
> > all inter-module inlining
> 
> Oh, wait,  so, demoting “extern” functions to “static” functions in GCC’s IPA inlining is to disable inter-module inlining? 
> Why? Is there any technical issue with inter-module inlining in GCC? 

No I suppose it is just different organization of LTO from Studio
compiler.  Perhaps studio compiler still combine according to original
source-level compilation units and then cross-module inlining of
function A may cause function B to be promoted to public in a case
originally both A and B were in same compilation unit and B was static?
> 
> 
> > and with live patching we also disable most of
> > other optimization,
> 
> Yes, with live-patching, most of the IPA optimization need to be disabled. But this functionality is needed, right? When user requests live-patching support, 
> They should know that most of IPA optimization will be disabled.

Question is how much useful is to biuld with -flto then.  I woudl say that
close to 90% of performance benefits from LTO originates from cross-module
inlining. Most of code size benefits are due to 
 - removing unreachable code,
 - inlining functions called once (with whole program knowledge)
 - removing offline function bodies if all calls has been inlined. and
 - identical code folding

Other IPA optimizations we implement (function flags discovery, mod-ref
etc.) accounts for smaller portion of perofmance & size.

If you block cross-module inlininig and all kind of inter-procedural
optimizations you lose, I believe, all of the goodies above except for
removal of unreachable code.

While this is important for some C++ code with a lot of template
instantiations, I am not sure how many completely dead functions kernel has.
So what use cases you expect for -flto -flive-patching=inline-only-static?
Honza
> 
> Qing
> 
> > I think basically only unreachable code removal will
> > remain and possibly some propagation of "coldness" across the code.
> > 
> > I can implement this incrementally.
> > Martin, if live patching is happy about some symbols being promoted
> > static, the patch is OK.
> > Honza
>
  

> On Oct 7, 2022, at 10:43 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
> 
>>>> Probably not hard, and the IPA pass adjusting visbility could as well
>>>> mark the functions
>>>> as not to be inlined with -flive-patching=inline-only-static.
>>>> 
>>>>>> 
>>>>>> OTOH inline-only-static could disable WPA inlining and do all inlining early ...
>>>>> 
>>>>> Inline-only-static ONLY inlines static functions, how can it disable WPA inlining? Don’t quite understand here.
>>>> 
>>>> it's a flag so it can be used to control other things
>>> 
>>> GCC has two inliners
>>> 1) ealry inlininer which happens at compile time and is quite
>>> restricted only to obvious cases (always_inline, flatten and very small
>>> functions)
>>> 2) IPA inlining happening at link-time (WPA) which is using greedy
>>> algorithm and makes more complicated code size/speed tradeoffs
>>> Indeed betwen 1 and 2 previously global functions may become static by
>>> resolution info (they won't currently with kernel since we do
>>> incremental linking).  We could easily keep track of originally static
>>> functions and promoted to static functions and make IPA inlining to
>>> honnor the patch.
> 	       ^^^^ I mean -flive-patching=inline-only-static flag
>> 
>> Yes, this is similar as Studio compiler (an early inliner and a IPA inliner) 
>> But I still don’t quite understand why during IPA inlining, extern functions need to be changed to static functions?
>> (in Studio compiler, it’s opposite, static functions are all promoted to extern functions to enable inter-procedural inlining)
>> Is there a file I can read to understand more details on this?
> 
> In GCC WPA stage takes all compilation units and create a new combined
> translation unit.  This has same kind of symbol table as if it came all
> from a single source file.

Okay, I see now.. (should take some time to familiar with the GCC LTO framework a little more, I guess-:)

>  So if resolution file tells us that a given
> symbol is not used outside the LTO bytecode (when one links final binary
> linker knows if there was other use and similar for shared libraries if
> symbol is hidden) we promote symbol to static.

Okay, that’s reasonable. 

>  This lets us to optimize
> it better: change calling conventions, remove offline copy if all calls
> was inlined, propagate various information about it.
> 
> Since from this moment on the whole translation unit behaves as single
> source file it is not problem to inline static functions cross-modle.
Yes, makes sense.


> 
> Later we partition the combined unit to do rest of compilation in
> parallel and at this stage some static symbols can be changed to hidden
> symbols if they are used by multiple partitioning.
Okay. 
So, original “extern” functions will be kept as “static” if they are not used by multiple partitioning?

>> 
>>> 
>>> I however wonder how much LTO optimization would remain. If we disable
>>> all inter-module inlining
>> 
>> Oh, wait,  so, demoting “extern” functions to “static” functions in GCC’s IPA inlining is to disable inter-module inlining? 
>> Why? Is there any technical issue with inter-module inlining in GCC? 
> 
> No I suppose it is just different organization of LTO from Studio
> compiler.  Perhaps studio compiler still combine according to original
> source-level compilation units and then cross-module inlining of
> function A may cause function B to be promoted to public in a case
> originally both A and B were in same compilation unit and B was static?
I think the biggest difference between IPO (inter-procedural optimization) of Studio compiler
 and LTO of GCC is, GCC merges all IR files together into one single IR file. But Studio still 
kept multiple IR files. So, For GCC, LTO inlining is just similar as a IN-Module inlining, and
external functions are better to be demoted to static functions to enable better optimizations. 

For Studio compiler, IPO analysis and optimization still based on multiple IRs. So in order to
enable cross-file inlining between different modules, static functions need to be promoted to
external function the exactly reason as you mentioned above. (Though Studio compiler could 
do better to only promote parts of the static functions to external by demand)

>> 
>> 
>>> and with live patching we also disable most of
>>> other optimization,
>> 
>> Yes, with live-patching, most of the IPA optimization need to be disabled. But this functionality is needed, right? When user requests live-patching support, 
>> They should know that most of IPA optimization will be disabled.
> 
> Question is how much useful is to biuld with -flto then.  I woudl say that
> close to 90% of performance benefits from LTO originates from cross-module
> inlining.
> Most of code size benefits are due to 
> - removing unreachable code,
> - inlining functions called once (with whole program knowledge)
> - removing offline function bodies if all calls has been inlined. and
> - identical code folding

Yes. 

> 
> Other IPA optimizations we implement (function flags discovery, mod-ref
> etc.) accounts for smaller portion of perofmance & size.
Yes, this was also the case for studio compiler. 
> 
> If you block cross-module inlininig and all kind of inter-procedural
> optimizations you lose, I believe, all of the goodies above except for
> removal of unreachable code.
> 
> While this is important for some C++ code with a lot of template
> instantiations, I am not sure how many completely dead functions kernel has.
> So what use cases you expect for -flto -flive-patching=inline-only-static?

Okay, So, your major concern of the combination of “-flto -flive-patching=inline-only-static” is: not very  useful. 
If so, Yes, I agree with this. 

My major point was, if technically simple and doable, it’s better to support it for feature completeness. 
It’s user’s choice to use it or not. (We can provide more details in documentation to warn the user about the
Performance impact).  On the other hand, if it’s too complicate to implement, I agree not doing it is better.

Thanks a lot for your patience and detailed explanation.

Qing


> Honza
>> 
>> Qing
>> 
>>> I think basically only unreachable code removal will
>>> remain and possibly some propagation of "coldness" across the code.
>>> 
>>> I can implement this incrementally.
>>> Martin, if live patching is happy about some symbols being promoted
>>> static, the patch is OK.
>>> Honza