On Tue, 13 Nov 2018, Qing Zhao wrote: > Hi, > > > On Nov 13, 2018, at 1:18 PM, Miroslav Benes <mbenes@xxxxxxx> wrote: > > > >> Attached is the patch for new -flive-patching=[inline-only-static | inline-clone] master option. > >> > >> '-flive-patching=LEVEL' > >> Control GCC's optimizations to provide a safe compilation for > >> live-patching. Provides multiple-level control on how many of the > >> optimizations are enabled by users' request. The LEVEL argument > >> should be one of the following: > >> > >> 'inline-only-static' > >> > >> Only enable inlining of static functions, disable all other > >> ipa optimizations/analyses. As a result, when patching a > >> static routine, all its callers need to be patches as well. > >> > >> 'inline-clone' > >> > >> Only enable inlining and all optimizations that internally > >> create clone, for example, cloning, ipa-sra, partial inlining, > >> etc.; disable all other ipa optimizations/analyses. As a > >> result, when patching a routine, all its callers and its > >> clones' callers need to be patched as well. > > > > Based on our previous discussion I assume that "clone" optimizations are > > safe (for LP) and the others are not. Anyway I'd welcome a note mentioning > > that disabled optimizations are dangerous for LP. > > actually, I don’t think that those disabled optimizations are “dangerous” for live-patching. one of the major reasons we disable them > is because that currently the compiler does NOT provide a good way to compute the impacted function list for those optimizations. > therefore, we disable them at this time. > > many of them could be enabled too if the compiler can report the impacted function list accurately in the future. Yes, you can formulate it like that. On the other hand, I (we) have always tried to keep a set of patched functions as small as possible. So some cost-benefit analysis would have to be done. However, that's another problem and we can discuss it later. > > > > I know it may be the same for you, but it is not for me as a GCC user. > > "internally create clone" sounds very... well, internal. It does not > > describe the option much for ordinary user whow has no knowledge about GCC > > internals. > > > > So could you rephrase it a bit, please? > > I tried to make this clear. please see the following: > > '-flive-patching=LEVEL' > Control GCC's optimizations to provide a safe compilation for > live-patching. > > If the compiler's optimization uses a function's body or > information extracted from its body to optimize/change another > function, the latter is called an impacted function of the former. > If a function is patched, its impacted functions should be patched > too. > > The impacted functions are decided by the compiler's > interprocedural optimizations. For example, inlining a function > into its caller, cloning a function and changing its caller to call > this new clone, or extracting a function's pureness/constness > information to optimize its direct or indirect callers, etc. > > Usually, the more ipa optimizations enabled, the larger the number > of impacted functions for each function. In order to control the > number of impacted functions and computed the list of impacted > function easily, we provide control to partially enable ipa > optimizations on two different levels. > > The LEVEL argument should be one of the following: > > 'inline-only-static' > > Only enable inlining of static functions, disable all other > interprocedural optimizations/analyses. As a result, when > patching a static routine, all its callers need to be patches > as well. > > 'inline-clone' > > Only enable inlining and cloning optimizations, which includes > inlining, cloning, interprocedural scalar replacement of > aggregates and partial inlining. Disable all other > interprocedural optimizations/analyses. As a result, when > patching a routine, all its callers and its clones' callers > need to be patched as well. > > When -flive-patching specified without any value, the default value > is "inline-clone". > > This flag is disabled by default. Sounds better. Thanks. Miroslav
