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

[Richard Biener <rguenther at suse dot de>]

On Wed, 26 Sep 2018, Qing Zhao wrote:

> 
> > On Sep 26, 2018, at 6:07 PM, Alexander Monakov <amonakov@ispras.ru> wrote:
> > 
> > On Wed, 26 Sep 2018, Qing Zhao wrote:
> >> The request is for application developers who want to use gcc's online
> >>   patching feature.
> >> 
> >> Today, developers can turn off inlining and deliver just the patched routine.  They
> >>   can also allow all inlining and deliver the patched routine and all the routines
> >>   that the patched routine was inlined into.
> >> 
> >> completely turning off inlining will sacrifice too much run-time performance. completely
> >> enable inlining, on the other hand, will have the potential issues with code size, complexity and
> >> debuggability for the online patching.
> >> 
> >> the proposed option provides a compromised solution for the above issues. and enable more
> >> developers to utilize gcc’s online patching feature.
> > 
> > From this explanation it sounds to me that what you really need is -Os-like
> > behavior for IPA passes, without enabling -Os for gimple/rtl passes, as I
> > mentioned in my previous email. Honza, how does that sound?
> 
> I don’t think a -Os-like option will do the job.
> 
> As Jeff already mentioned in a very previous email:
> 
> “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.
> 
> 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 :-)
> 
> So conceptually I can see value in this kind of option.
> “
> 
> so, specially control inlining on static/global will be helpful to online patch.

But as Honza said this gets you only sofar.  IIRC for our kernel 
livepatching we turn off most IPA passes because while we can "easily"
figure what and where things were inlined spotting the effects of
IPA analysis and transform is almost impossible.

So there's two parts of the knob - one is to make the live-patch size
not explode (do less inlining where it doesn't hurt performance - that
eventually also applies to static functions called once inlining!).
The other is to make it possible to conservatively compute the set
of functions you have to replace (the set of functions that are
affected by a patch).

Having an option to _that_ effect might indeed be useful (to avoid
people chasing all the flags they need to disable).  So shouldn't this
be a -fease-live-patching option rather that -finline-only-static
which doesn't really capture the intention nor the effect?

That is, -fease-live-patching would guarantee that if you source-patch
function X then, if you replace all functions which debuginfo tells you
X was inlined to, the result will be semantically equivalent with
replacing the whole program?  We might even add sth like
-fpatch-symbol-list=FOO,BAR that outputs a list of symbols into BAR
that are affected this way when functions FOO are changed (you
run that on unpatched code of course).  Or we add sth to the
cgraph dumpfile that for each function lists the set of symbols
it was affected by.

Thanks,
Richard.