[patch] More precise message with -Winline

[patch] More precise message with -Winline

[Eric Botcazou]

[-- Attachment #1: Type: text/plain, Size: 922 bytes --]

Hi,

for EH cleanups, the branch probability heuristics can consider that edges are 
never taken, i.e. their profile count is set to zero.  In this case, no matter 
how you tweak the inlining parameters, you cannot get function calls inlined, 
but -Winline nevertheless prints the standard message about unlikely calls as 
in the cases when tweaking the inlining parameters can work.

Therefore the attached patch adds a new CIF code with the warning message:

"call is never executed and code size would grow [-Winline]"

for this case, thus signaling the user that he'd better not waste time trying 
to get the call inlined.

Tested on x86_64-suse-linux, OK for the mainline?


2019-07-24  Eric Botcazou  <ebotcazou@adacore.com>

	* cif-code.def (NEVER_CALL): New code.
	* ipa-inline.c (want_inline_small_function_p): Fix formatting issues.
	Set the failure to CIF_NEVER_CALL if the IPA count is zero.

-- 
Eric Botcazou

[-- Attachment #2: p.diff --]
[-- Type: text/x-patch, Size: 2815 bytes --]

Index: cif-code.def
===================================================================
--- cif-code.def	(revision 273480)
+++ cif-code.def	(working copy)
@@ -83,6 +83,10 @@ DEFCIFCODE(RECURSIVE_INLINING, CIF_FINAL_NORMAL,
 DEFCIFCODE(UNLIKELY_CALL, CIF_FINAL_NORMAL,
 	   N_("call is unlikely and code size would grow"))
 
+/* Call is considered never executed.  */
+DEFCIFCODE(NEVER_CALL, CIF_FINAL_NORMAL,
+	   N_("call is never executed and code size would grow"))
+
 /* Function is not declared as inline.  */
 DEFCIFCODE(NOT_DECLARED_INLINED, CIF_FINAL_NORMAL,
 	   N_("function not declared inline and code size would grow"))
Index: ipa-inline.c
===================================================================
--- ipa-inline.c	(revision 273480)
+++ ipa-inline.c	(working copy)
@@ -810,7 +810,7 @@ want_inline_small_function_p (struct cgraph_edge *
 				  | INLINE_HINT_loop_stride))
 		       && !(big_speedup = big_speedup_p (e)))))
 	{
-          e->inline_failed = CIF_MAX_INLINE_INSNS_SINGLE_LIMIT;
+	  e->inline_failed = CIF_MAX_INLINE_INSNS_SINGLE_LIMIT;
 	  want_inline = false;
 	}
       else if (!DECL_DECLARED_INLINE_P (callee->decl)
@@ -818,12 +818,12 @@ want_inline_small_function_p (struct cgraph_edge *
 	       && growth >= PARAM_VALUE (PARAM_MAX_INLINE_INSNS_SMALL))
 	{
 	  /* growth_likely_positive is expensive, always test it last.  */
-          if (growth >= MAX_INLINE_INSNS_SINGLE
+	  if (growth >= MAX_INLINE_INSNS_SINGLE
 	      || growth_likely_positive (callee, growth))
 	    {
-              e->inline_failed = CIF_NOT_DECLARED_INLINED;
+	      e->inline_failed = CIF_NOT_DECLARED_INLINED;
 	      want_inline = false;
- 	    }
+	    }
 	}
       /* Apply MAX_INLINE_INSNS_AUTO limit for functions not declared inline
 	 Upgrade it to MAX_INLINE_INSNS_SINGLE when hints suggests that
@@ -839,12 +839,12 @@ want_inline_small_function_p (struct cgraph_edge *
 	       && !(big_speedup == -1 ? big_speedup_p (e) : big_speedup))
 	{
 	  /* growth_likely_positive is expensive, always test it last.  */
-          if (growth >= MAX_INLINE_INSNS_SINGLE
+	  if (growth >= MAX_INLINE_INSNS_SINGLE
 	      || growth_likely_positive (callee, growth))
 	    {
 	      e->inline_failed = CIF_MAX_INLINE_INSNS_AUTO_LIMIT;
 	      want_inline = false;
- 	    }
+	    }
 	}
       /* If call is cold, do not inline when function body would grow. */
       else if (!e->maybe_hot_p ()
@@ -851,7 +851,10 @@ want_inline_small_function_p (struct cgraph_edge *
 	       && (growth >= MAX_INLINE_INSNS_SINGLE
 		   || growth_likely_positive (callee, growth)))
 	{
-          e->inline_failed = CIF_UNLIKELY_CALL;
+	  if (e->count.ipa () == profile_count::zero ())
+	    e->inline_failed = CIF_NEVER_CALL;
+	  else
+	    e->inline_failed = CIF_UNLIKELY_CALL;
 	  want_inline = false;
 	}
     }

Re: [patch] More precise message with -Winline

[Richard Biener]

On Wed, Jul 24, 2019 at 11:29 AM Eric Botcazou <ebotcazou@adacore.com> wrote:
>
> Hi,
>
> for EH cleanups, the branch probability heuristics can consider that edges are
> never taken, i.e. their profile count is set to zero.  In this case, no matter
> how you tweak the inlining parameters, you cannot get function calls inlined,
> but -Winline nevertheless prints the standard message about unlikely calls as
> in the cases when tweaking the inlining parameters can work.
>
> Therefore the attached patch adds a new CIF code with the warning message:
>
> "call is never executed and code size would grow [-Winline]"
>
> for this case, thus signaling the user that he'd better not waste time trying
> to get the call inlined.
>
> Tested on x86_64-suse-linux, OK for the mainline?

Looks good besides

+         if (e->count.ipa () == profile_count::zero ())
+           e->inline_failed = CIF_NEVER_CALL;

does it actually matter what kind of profile quality e->count.ipa has
compared to profile_count::zero ()?  Also

+/* Call is considered never executed.  */
+DEFCIFCODE(NEVER_CALL, CIF_FINAL_NORMAL,
+          N_("call is never executed and code size would grow"))

suggests the call is never executed, but we only assume that
(or the profile training run never hit it).

Richard.

>
>
> 2019-07-24  Eric Botcazou  <ebotcazou@adacore.com>
>
>         * cif-code.def (NEVER_CALL): New code.
>         * ipa-inline.c (want_inline_small_function_p): Fix formatting issues.
>         Set the failure to CIF_NEVER_CALL if the IPA count is zero.
>
> --
> Eric Botcazou

Re: [patch] More precise message with -Winline

[Eric Botcazou]

> Looks good besides
> 
> +         if (e->count.ipa () == profile_count::zero ())
> +           e->inline_failed = CIF_NEVER_CALL;
> 
> does it actually matter what kind of profile quality e->count.ipa has
> compared to profile_count::zero ()?

I don't really know, the other examples in the function use e->count.ipa too.

> Also
> 
> +/* Call is considered never executed.  */
> +DEFCIFCODE(NEVER_CALL, CIF_FINAL_NORMAL,
> +          N_("call is never executed and code size would grow"))
> 
> suggests the call is never executed, but we only assume that
> (or the profile training run never hit it).

OK, I added "considered" as in the comment above.

-- 
Eric Botcazou

Re: [patch] More precise message with -Winline

[Andi Kleen]

Eric Botcazou <ebotcazou@adacore.com> writes:

> Hi,
>
> for EH cleanups, the branch probability heuristics can consider that edges are 
> never taken, i.e. their profile count is set to zero.  In this case, no matter 
> how you tweak the inlining parameters, you cannot get function calls inlined, 
> but -Winline nevertheless prints the standard message about unlikely calls as 
> in the cases when tweaking the inlining parameters can work.
>
> Therefore the attached patch adds a new CIF code with the warning message:
>
> "call is never executed and code size would grow [-Winline]"

That seems misleading, unless the exception is really never thrown.

Maybe s/never/rarely/


-Andi

Re: [patch] More precise message with -Winline

[Eric Botcazou]

> That seems misleading, unless the exception is really never thrown.

See my earlier answer to Richard.

-- 
Eric Botcazou