Re: [PATCH][Version 3]Come up with -flive-patching master option.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux Kernel]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux SCSI]

  Powered by Linux