Re: [PATCH][Middle-end][Version 3]Add a new option to control inlining only on static functions

[Martin Sebor]

On 09/18/2018 12:58 PM, Qing Zhao wrote:
Hi,
this is the 3rd version of the patch, the major change is to address Andrew’s 
concern on the documentation part.

I updated the documentation of this option as following:

'-finline-only-static'
     By default, GCC inlines functions without considering whether they
     are static or not.  This flag guides inliner to only inline static
     functions.  This option has any effect only when inlining itself is
     turned on by the -finline-functions or -finline-small-fiunctions.

     Off by default.

all other changes keep the same as version 2.

please take a look again. and let me know any comment and suggestion.

Just a few minor spelling issues:


thanks.

Qing

gcc/ChangeLog

+2018-09-18  Qing Zhao  <qing.z...@oracle.com>
+
+       * cif-code.def (FUNCTION_EXTERN): New CIFCODE.
+       * common.opt (-finline-only-static): New option.
+       * doc/invoke.texi: Document -finline-only-static.
+       * ipa-inline.c (can_inline_edge_p): Control inlining based on
+       function's visibility.

Probably "linkage" would be a more fitting term here.


gcc/testsuite/ChangeLog

+2018-09-18  Qing Zhao  <qing.z...@oracle.com>
+
+       * gcc.dg/inline_only_static.c: New test.
+


diff --git a/gcc/cif-code.def b/gcc/cif-code.def
index 19a7621..64b2b1a 100644
--- a/gcc/cif-code.def
+++ b/gcc/cif-code.def
@@ -132,6 +132,12 @@
 DEFCIFCODE(USES_COMDAT_LOCAL, CIF_FINAL_ERROR,
 DEFCIFCODE(ATTRIBUTE_MISMATCH, CIF_FINAL_ERROR,
           N_("function attribute mismatch"))

+/* We can't inline because the user requests only inlining static 
function
+   but the function is external visible.  */

I suspect you meant: "only static functions" (plural) and
"the function has external linkage" (as defined in the C and
C++ standards).

+DEFCIFCODE(FUNCTION_EXTERN, CIF_FINAL_ERROR,
+          N_("function is external visible when the user requests only"
+             " inlining static"))
+

Here as well: either "function has external linkage" or "function
is extern."

=======================================
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index ec12711..b6b0db5 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@ -8066,6 +8067,15 @@
 having large chains of nested wrapper functions.

 Enabled by default.

+@item -finline-only-static
+@opindex finline-only-static
+By default, GCC inlines functions without considering whether they are 
static
+or not. This flag guides inliner to only inline static functions.

Guides "the inliner" (missing article).

+This option has any effect only when inlining itself is turned on by the
+-finline-functions or -finline-small-fiunctions.

"by the -f... options."  (Missing "options") and
-finline-small-functions (note the spelling of functions).

+
+Off by default.

I think the customary way to word it is: "Disabled by default."
or "The finline-only-static option/flag is disabled/off by default"

Martin