> On Sep 27, 2018, at 2:45 AM, Richard Biener <rguent...@suse.de> wrote: > > On Wed, 26 Sep 2018, Qing Zhao wrote: > >> >>> On Sep 26, 2018, at 6:07 PM, Alexander Monakov <amona...@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!).
Yes, limit additional unnecessary inlining might be better/ > 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). right. > > 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? Yes, if we can have -fease-live-patching, that will be even better for the live patch purpose. > > 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? Okay. > 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. Yes, this option might be also helpful for livepatching. we can do it at the next step. Qing > > Thanks, > Richard.
