On 20/11/18 09:32 -0600, Qing Zhao wrote:
Hi, this is the newest version of the patch. major changes: 1. format fixes raised by Martin; 2. output error when disabled IPA optimizations are turned on explicitly by the user with -flive-patching at the same time. based on Honza’s suggestions. the new changes have been bootstrapped and tested on both aarch64 and x86, no regressions. Okay for commit? thanks. Qing. =========== gcc/ChangeLog: 2018-11-20 qing zhao <qing.zhao@xxxxxxxxxx> * cif-code.def (EXTERN_LIVE_ONLY_STATIC): New CIF code. * common.opt: Add -flive-patching flag. * doc/invoke.texi: Document -flive-patching. * flag-types.h (enum live_patching_level): New enum. * ipa-inline.c (can_inline_edge_p): Disable external functions from inlining when flag_live_patching is LIVE_PATCHING_INLINE_ONLY_STATIC. * opts.c (control_options_for_live_patching): New function. (finish_options): Make flag_live_patching incompatible with flag_lto. Control IPA optimizations based on different levels of flag_live_patching. gcc/testsuite/ChangeLog: 2018-11-20 qing zhao <qing.zhao@xxxxxxxxxx> * gcc.dg/live-patching-1.c: New test. * gcc.dg/live-patching-2.c: New test. * gcc.dg/live-patching-3.c: New test. * gcc.dg/tree-ssa/writeonly-3.c: New test. * gcc.target/i386/ipa-stack-alignment-2.c: New test.
Hi Qing Zhao, I was looking at the new docs for -flive-patching and have some questions.
--- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -416,6 +416,7 @@ Objective-C and Objective-C++ Dialects}. -fipa-bit-cp -fipa-vrp @gol -fipa-pta -fipa-profile -fipa-pure-const -fipa-reference -fipa-reference-addressable @gol -fipa-stack-alignment -fipa-icf -fira-algorithm=@var{algorithm} @gol +-flive-patching=@var{level} @gol -fira-region=@var{region} -fira-hoist-pressure @gol -fira-loop-pressure -fno-ira-share-save-slots @gol -fno-ira-share-spill-slots @gol @@ -9045,6 +9046,65 @@ equivalences that are found only by GCC and equivalences found only by Gold. This flag is enabled by default at @option{-O2} and @option{-Os}. +@item -flive-patching=@var{level} +@opindex flive-patching +Control GCC's optimizations to provide a safe compilation for live-patching.
"provide a safe compilation" isn't very clear to me. I don't know what it means to "provide a compilation", let alone a safe one. Could we say something like "Control GCC’s optimizations to produce output suitable 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
decided or determined?
+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.
I don't know what the second sentence is saying. I can read it two different ways: 1) Those are examples of interprocedural optimizations which participate in the decision making, but the actual details of how the decisions are made are not specified here. 2) Performing those optimizations causes a function to be impacted. If 1) is the intended meaning, then I think it should say "For example, <INS>when</INS> inlining a function into its caller, ..." If 2) is the intended meaning, then I think it should say "For example, <INS>a caller is impacted when</INS> inlining a function into its caller ...". Does either of those suggestions match the intended meaning? Or do you have a better way to rephrase it?
+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,
Should be "and more easily compute the list of impacted functions".
+we provide control to partially enable IPA optimizations on two different +levels.
We don't usually say "we provide" like this. I suggest simply "IPA optimizations can be partially enabled at two different levels."
+ +The @var{level} argument should be one of the following: + +@table @samp + +@item inline-clone + +Only enable inlining and cloning optimizations, which includes inlining, +cloning, interprocedural scalar replacement of aggregates and partial inlining. +As a result, when patching a function, all its callers and its clones' +callers need to be patched as well.
Since you've defined the term "impacted" could this just say "all its callers and its clones' callers are impacted."?
+@option{-flive-patching=inline-clone} disables the following optimization flags: +@gccoptlist{-fwhole-program -fipa-pta -fipa-reference -fipa-ra @gol +-fipa-icf -fipa-icf-functions -fipa-icf-variables @gol +-fipa-bit-cp -fipa-vrp -fipa-pure-const -fipa-reference-addressable @gol +-fipa-stack-alignment} + +@item inline-only-static + +Only enable inlining of static functions. +As a result, when patching a static function, all its callers need to be +patches as well.
"Typo: "patches" should be "patched", but I'd suggest "are impacted" here too.
+In addition to all the flags that -flive-patching=inline-clone disables, +@option{-flive-patching=inline-only-static} disables the following additional +optimization flags: +@gccoptlist{-fipa-cp-clone -fipa-sra -fpartial-inlining -fipa-cp} + +@end table + +When -flive-patching specified without any value, the default value +is "inline-clone".
This should use @option{} and @var{} and is missing the word "is".
+This flag is disabled by default. + +Note that -flive-patching is not supported with link-time optimizer.
s/optimizer./optimization/
+(@option{-flto}). + @item -fisolate-erroneous-paths-dereference @opindex fisolate-erroneous-paths-dereference Detect paths that trigger erroneous or undefined behavior due to
The attached patch makes some of these changes, but I'd like to know if my changes preserve the intended meaning.
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 73a7789d879..1e9f0fa7230 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -9367,7 +9367,7 @@ This flag is enabled by default at @option{-O2} and @option{-Os}. @item -flive-patching=@var{level} @opindex flive-patching -Control GCC's optimizations to provide a safe compilation for live-patching. +Control GCC's optimizations to produce output suitable 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 @@ -9375,16 +9375,16 @@ 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. +optimizations. For example, a caller is impacted when 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. +impacted functions and more easily compute the list of impacted function, +IPA optimizations can be partially enabled at two different levels. The @var{level} argument should be one of the following: @@ -9407,21 +9407,22 @@ callers need to be patched as well. Only enable inlining of static functions. As a result, when patching a static function, all its callers need to be -patches as well. +patched as well. -In addition to all the flags that -flive-patching=inline-clone disables, +In addition to all the flags that @option{-flive-patching=inline-clone} +disables, @option{-flive-patching=inline-only-static} disables the following additional optimization flags: @gccoptlist{-fipa-cp-clone -fipa-sra -fpartial-inlining -fipa-cp} @end table -When -flive-patching specified without any value, the default value -is "inline-clone". +When @option{-flive-patching} is specified without any value, the default value +is @var{inline-clone}. This flag is disabled by default. -Note that -flive-patching is not supported with link-time optimizer. +Note that @option{-flive-patching} is not supported with link-time optimization (@option{-flto}). @item -fisolate-erroneous-paths-dereference