On 10/04/19 13:55 -0500, Qing Zhao wrote:
Hi, Jonathan,
thanks for your review on the documentation change for -flive-patching option.
--- 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.” ?
yes, this is better.
+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?
determined is better.
+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 …".
2) is the intended meaining.
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”.
this is good.
+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.”
Okay.
+
+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.”?
I think that the following might be better:
when patching a function, all its callers and its clones’ callers are impacted, therefore need to be patched as well.
Agreed.
+@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.
Okay.
+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”.
Okay.
+This flag is disabled by default.
+
+Note that -flive-patching is not supported with link-time optimizer.
s/optimizer./optimization/
Okay.
+(@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.
the changes in the patch looks good to me.
thanks a lot.
Thanks!
I've attached an updated patch with your suggestions.
Reviewers, is this OK for trunk?
commit 0b9a201fb80fb1e708d83566df50f1555cf80e10
Author: nickc <nickc@138bc75d-0d04-0410-961f-82ee72b054a4>
Date: Wed Apr 10 14:44:47 2019 +0000
Clarify documentation for -flive-patching
* doc/invoke.texi (Optimize Options): Clarify -flive-patching docs.
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 755b9f754a1..3a88d8db157 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -9367,24 +9367,24 @@ 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
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.
+The impacted functions are determined by the compiler's interprocedural
+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:
@@ -9395,7 +9395,7 @@ The @var{level} argument should be one of the following:
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.
+callers are impacted, therefore need to be patched as well.
@option{-flive-patching=inline-clone} disables the following optimization flags:
@gccoptlist{-fwhole-program -fipa-pta -fipa-reference -fipa-ra @gol
@@ -9406,22 +9406,23 @@ callers need to be patched as well.
@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.
+As a result, when patching a static function, all its callers are impacted
+and so need to be 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
