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

[Martin Sebor <msebor at gmail dot com>]

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.zhao@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.zhao@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