public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fix inlining checks wrt optimize attribute
@ 2015-01-22 10:23 Richard Biener
  2015-01-22 10:36 ` Christian Bruel
  2015-01-22 19:42 ` Jan Hubicka
  0 siblings, 2 replies; 6+ messages in thread
From: Richard Biener @ 2015-01-22 10:23 UTC (permalink / raw)
  To: gcc-patches; +Cc: Jan Hubicka


As said in the other thread - this makes sure we don't perform inlining
that might end up generating invalid code.  It also preserves
user-provided optimize attributes more properly.

Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.

Richard.

2015-01-22  Richard Biener  <rguenther@suse.de>

	* ipa-inline.c (can_inline_edge_p): Disable inlining of edges
	with IL incompatible options.  Properly honor user optimize
	attributes.

Index: gcc/ipa-inline.c
===================================================================
--- gcc/ipa-inline.c	(revision 219929)
+++ gcc/ipa-inline.c	(working copy)
@@ -404,17 +404,56 @@ can_inline_edge_p (struct cgraph_edge *e
      optimization attribute.  */
   else if (caller_tree != callee_tree)
     {
-      /* gcc.dg/pr43564.c.  Look at forced inline even in -O0.  */
-      if (DECL_DISREGARD_INLINE_LIMITS (callee->decl))
+      /* There are some options that change IL semantics which means
+         we cannot inline in these cases for correctness reason.
+	 Not even for always_inline declared functions.  */
+      /* Strictly speaking only when the callee contains signed integer
+         math where overflow is undefined.  */
+      if ((opt_for_fn (e->caller->decl, flag_strict_overflow)
+	   != opt_for_fn (e->caller->decl, flag_strict_overflow))
+	  || (opt_for_fn (e->caller->decl, flag_wrapv)
+	      != opt_for_fn (e->caller->decl, flag_wrapv))
+	  || (opt_for_fn (e->caller->decl, flag_trapv)
+	      != opt_for_fn (e->caller->decl, flag_trapv))
+	  /* Strictly speaking only when the callee contains memory
+	     accesses that are not using alias-set zero anyway.  */
+	  || (opt_for_fn (e->caller->decl, flag_strict_aliasing)
+	      != opt_for_fn (e->caller->decl, flag_strict_aliasing))
+	  /* Strictly speaking only when the callee uses FP math.  */
+	  || (opt_for_fn (e->caller->decl, flag_rounding_math)
+	      != opt_for_fn (e->caller->decl, flag_rounding_math))
+	  || (opt_for_fn (e->caller->decl, flag_trapping_math)
+	      != opt_for_fn (e->caller->decl, flag_trapping_math))
+	  || (opt_for_fn (e->caller->decl, flag_unsafe_math_optimizations)
+	      != opt_for_fn (e->caller->decl, flag_unsafe_math_optimizations))
+	  || (opt_for_fn (e->caller->decl, flag_finite_math_only)
+	      != opt_for_fn (e->caller->decl, flag_finite_math_only))
+	  || (opt_for_fn (e->caller->decl, flag_signaling_nans)
+	      != opt_for_fn (e->caller->decl, flag_signaling_nans))
+	  || (opt_for_fn (e->caller->decl, flag_cx_limited_range)
+	      != opt_for_fn (e->caller->decl, flag_cx_limited_range))
+	  || (opt_for_fn (e->caller->decl, flag_signed_zeros)
+	      != opt_for_fn (e->caller->decl, flag_signed_zeros))
+	  || (opt_for_fn (e->caller->decl, flag_associative_math)
+	      != opt_for_fn (e->caller->decl, flag_associative_math))
+	  || (opt_for_fn (e->caller->decl, flag_reciprocal_math)
+	      != opt_for_fn (e->caller->decl, flag_reciprocal_math))
+	  /* Strictly speaking only when the callee contains function
+	     calls that may end up setting errno.  */
+	  || (opt_for_fn (e->caller->decl, flag_errno_math)
+	      != opt_for_fn (e->caller->decl, flag_errno_math)))
+	{
+	  e->inline_failed = CIF_OPTIMIZATION_MISMATCH;
+	  inlinable = false;
+	}
+      /* gcc.dg/pr43564.c.  Apply user-forced inline even at -O0.  */
+      else if (DECL_DISREGARD_INLINE_LIMITS (callee->decl)
+	       && lookup_attribute ("always_inline",
+				    DECL_ATTRIBUTES (callee->decl)))
 	;
-      /* When user added an attribute, honnor it.  */
-      else if ((lookup_attribute ("optimize", DECL_ATTRIBUTES (caller->decl))
-		|| lookup_attribute ("optimize",
-				     DECL_ATTRIBUTES (callee->decl)))
-	       && ((opt_for_fn (caller->decl, optimize)
-		   > opt_for_fn (callee->decl, optimize))
-		   || (opt_for_fn (caller->decl, optimize_size)
-		       != opt_for_fn (callee->decl, optimize_size))))
+      /* When user added an attribute to the callee honor it.  */
+      else if (lookup_attribute ("optimize", DECL_ATTRIBUTES (callee->decl))
+	       && opts_for_fn (caller->decl) != opts_for_fn (callee->decl))
 	{
 	  e->inline_failed = CIF_OPTIMIZATION_MISMATCH;
 	  inlinable = false;

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix inlining checks wrt optimize attribute
  2015-01-22 10:23 [PATCH] Fix inlining checks wrt optimize attribute Richard Biener
@ 2015-01-22 10:36 ` Christian Bruel
  2015-01-22 11:03   ` Richard Biener
  2015-01-22 19:42 ` Jan Hubicka
  1 sibling, 1 reply; 6+ messages in thread
From: Christian Bruel @ 2015-01-22 10:36 UTC (permalink / raw)
  To: ramana Radhakrishnan, Richard Biener; +Cc: gcc-patches

Hi Richard,

I thought one of my current issue would be solved by this patch, but it 
is not : I have some inlining failures with the attribute target on ARM. 
(e.g inline-3.c) where obvious early inline fails with because we fail 
into the last can_inline_edge_p case:

opt_for_fn (callee->decl, optimize)
		   >= opt_for_fn (caller->decl, optimize)))

when callee and caller are both -O2 and 
targetm.target_option.can_inline_p was true, they should be inlined as 
in the general case (no DECL_FUNCTION_SPECIFIC_OPTIMIZATION)

I'm currently testing this additional change:

Index: ipa-inline.c
===================================================================
--- ipa-inline.c	(revision 219989)
+++ ipa-inline.c	(working copy)
@@ -489,7 +489,7 @@
        else if (opt_for_fn (callee->decl, optimize_size)
  	       < opt_for_fn (caller->decl, optimize_size)
  	       || (opt_for_fn (callee->decl, optimize)
-		   >= opt_for_fn (caller->decl, optimize)))
+		   > opt_for_fn (caller->decl, optimize)))
  	{
  	  if (estimate_edge_time (e)
  	      >= 20 + inline_edge_summary (e)->call_stmt_time)

Since this is a hot topic for you, I though you would have useful 
comments on this before I ask for a commit (when stage 4 close) ?

Cheers

Christian





On 01/22/2015 10:24 AM, Richard Biener wrote:
>
> As said in the other thread - this makes sure we don't perform inlining
> that might end up generating invalid code.  It also preserves
> user-provided optimize attributes more properly.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.
>
> Richard.
>
> 2015-01-22  Richard Biener  <rguenther@suse.de>
>
> 	* ipa-inline.c (can_inline_edge_p): Disable inlining of edges
> 	with IL incompatible options.  Properly honor user optimize
> 	attributes.
>
> Index: gcc/ipa-inline.c
> ===================================================================
> --- gcc/ipa-inline.c	(revision 219929)
> +++ gcc/ipa-inline.c	(working copy)
> @@ -404,17 +404,56 @@ can_inline_edge_p (struct cgraph_edge *e
>        optimization attribute.  */
>     else if (caller_tree != callee_tree)
>       {
> -      /* gcc.dg/pr43564.c.  Look at forced inline even in -O0.  */
> -      if (DECL_DISREGARD_INLINE_LIMITS (callee->decl))
> +      /* There are some options that change IL semantics which means
> +         we cannot inline in these cases for correctness reason.
> +	 Not even for always_inline declared functions.  */
> +      /* Strictly speaking only when the callee contains signed integer
> +         math where overflow is undefined.  */
> +      if ((opt_for_fn (e->caller->decl, flag_strict_overflow)
> +	   != opt_for_fn (e->caller->decl, flag_strict_overflow))
> +	  || (opt_for_fn (e->caller->decl, flag_wrapv)
> +	      != opt_for_fn (e->caller->decl, flag_wrapv))
> +	  || (opt_for_fn (e->caller->decl, flag_trapv)
> +	      != opt_for_fn (e->caller->decl, flag_trapv))
> +	  /* Strictly speaking only when the callee contains memory
> +	     accesses that are not using alias-set zero anyway.  */
> +	  || (opt_for_fn (e->caller->decl, flag_strict_aliasing)
> +	      != opt_for_fn (e->caller->decl, flag_strict_aliasing))
> +	  /* Strictly speaking only when the callee uses FP math.  */
> +	  || (opt_for_fn (e->caller->decl, flag_rounding_math)
> +	      != opt_for_fn (e->caller->decl, flag_rounding_math))
> +	  || (opt_for_fn (e->caller->decl, flag_trapping_math)
> +	      != opt_for_fn (e->caller->decl, flag_trapping_math))
> +	  || (opt_for_fn (e->caller->decl, flag_unsafe_math_optimizations)
> +	      != opt_for_fn (e->caller->decl, flag_unsafe_math_optimizations))
> +	  || (opt_for_fn (e->caller->decl, flag_finite_math_only)
> +	      != opt_for_fn (e->caller->decl, flag_finite_math_only))
> +	  || (opt_for_fn (e->caller->decl, flag_signaling_nans)
> +	      != opt_for_fn (e->caller->decl, flag_signaling_nans))
> +	  || (opt_for_fn (e->caller->decl, flag_cx_limited_range)
> +	      != opt_for_fn (e->caller->decl, flag_cx_limited_range))
> +	  || (opt_for_fn (e->caller->decl, flag_signed_zeros)
> +	      != opt_for_fn (e->caller->decl, flag_signed_zeros))
> +	  || (opt_for_fn (e->caller->decl, flag_associative_math)
> +	      != opt_for_fn (e->caller->decl, flag_associative_math))
> +	  || (opt_for_fn (e->caller->decl, flag_reciprocal_math)
> +	      != opt_for_fn (e->caller->decl, flag_reciprocal_math))
> +	  /* Strictly speaking only when the callee contains function
> +	     calls that may end up setting errno.  */
> +	  || (opt_for_fn (e->caller->decl, flag_errno_math)
> +	      != opt_for_fn (e->caller->decl, flag_errno_math)))
> +	{
> +	  e->inline_failed = CIF_OPTIMIZATION_MISMATCH;
> +	  inlinable = false;
> +	}
> +      /* gcc.dg/pr43564.c.  Apply user-forced inline even at -O0.  */
> +      else if (DECL_DISREGARD_INLINE_LIMITS (callee->decl)
> +	       && lookup_attribute ("always_inline",
> +				    DECL_ATTRIBUTES (callee->decl)))
>   	;
> -      /* When user added an attribute, honnor it.  */
> -      else if ((lookup_attribute ("optimize", DECL_ATTRIBUTES (caller->decl))
> -		|| lookup_attribute ("optimize",
> -				     DECL_ATTRIBUTES (callee->decl)))
> -	       && ((opt_for_fn (caller->decl, optimize)
> -		   > opt_for_fn (callee->decl, optimize))
> -		   || (opt_for_fn (caller->decl, optimize_size)
> -		       != opt_for_fn (callee->decl, optimize_size))))
> +      /* When user added an attribute to the callee honor it.  */
> +      else if (lookup_attribute ("optimize", DECL_ATTRIBUTES (callee->decl))
> +	       && opts_for_fn (caller->decl) != opts_for_fn (callee->decl))
>   	{
>   	  e->inline_failed = CIF_OPTIMIZATION_MISMATCH;
>   	  inlinable = false;
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix inlining checks wrt optimize attribute
  2015-01-22 10:36 ` Christian Bruel
@ 2015-01-22 11:03   ` Richard Biener
  2015-04-17  6:51     ` Christian Bruel
  0 siblings, 1 reply; 6+ messages in thread
From: Richard Biener @ 2015-01-22 11:03 UTC (permalink / raw)
  To: Christian Bruel; +Cc: ramana Radhakrishnan, gcc-patches

On Thu, 22 Jan 2015, Christian Bruel wrote:

> Hi Richard,
> 
> I thought one of my current issue would be solved by this patch, but it is not
> : I have some inlining failures with the attribute target on ARM. (e.g
> inline-3.c) where obvious early inline fails with because we fail into the
> last can_inline_edge_p case:
> 
> opt_for_fn (callee->decl, optimize)
> 		   >= opt_for_fn (caller->decl, optimize)))
> 
> when callee and caller are both -O2 and targetm.target_option.can_inline_p was
> true, they should be inlined as in the general case (no
> DECL_FUNCTION_SPECIFIC_OPTIMIZATION)
> 
> I'm currently testing this additional change:
> 
> Index: ipa-inline.c
> ===================================================================
> --- ipa-inline.c	(revision 219989)
> +++ ipa-inline.c	(working copy)
> @@ -489,7 +489,7 @@
>        else if (opt_for_fn (callee->decl, optimize_size)
>  	       < opt_for_fn (caller->decl, optimize_size)
>  	       || (opt_for_fn (callee->decl, optimize)
> -		   >= opt_for_fn (caller->decl, optimize)))
> +		   > opt_for_fn (caller->decl, optimize)))
>  	{
>  	  if (estimate_edge_time (e)
>  	      >= 20 + inline_edge_summary (e)->call_stmt_time)
> 
> Since this is a hot topic for you, I though you would have useful comments on
> this before I ask for a commit (when stage 4 close) ?

Yeah - the above looks like an obvious change to me.  Thus,
approved if it passes bootstrap/regtest.

Thanks,
Richard.

> Cheers
> 
> Christian
> 
> 
> 
> 
> 
> On 01/22/2015 10:24 AM, Richard Biener wrote:
> > 
> > As said in the other thread - this makes sure we don't perform inlining
> > that might end up generating invalid code.  It also preserves
> > user-provided optimize attributes more properly.
> > 
> > Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.
> > 
> > Richard.
> > 
> > 2015-01-22  Richard Biener  <rguenther@suse.de>
> > 
> > 	* ipa-inline.c (can_inline_edge_p): Disable inlining of edges
> > 	with IL incompatible options.  Properly honor user optimize
> > 	attributes.
> > 
> > Index: gcc/ipa-inline.c
> > ===================================================================
> > --- gcc/ipa-inline.c	(revision 219929)
> > +++ gcc/ipa-inline.c	(working copy)
> > @@ -404,17 +404,56 @@ can_inline_edge_p (struct cgraph_edge *e
> >        optimization attribute.  */
> >     else if (caller_tree != callee_tree)
> >       {
> > -      /* gcc.dg/pr43564.c.  Look at forced inline even in -O0.  */
> > -      if (DECL_DISREGARD_INLINE_LIMITS (callee->decl))
> > +      /* There are some options that change IL semantics which means
> > +         we cannot inline in these cases for correctness reason.
> > +	 Not even for always_inline declared functions.  */
> > +      /* Strictly speaking only when the callee contains signed integer
> > +         math where overflow is undefined.  */
> > +      if ((opt_for_fn (e->caller->decl, flag_strict_overflow)
> > +	   != opt_for_fn (e->caller->decl, flag_strict_overflow))
> > +	  || (opt_for_fn (e->caller->decl, flag_wrapv)
> > +	      != opt_for_fn (e->caller->decl, flag_wrapv))
> > +	  || (opt_for_fn (e->caller->decl, flag_trapv)
> > +	      != opt_for_fn (e->caller->decl, flag_trapv))
> > +	  /* Strictly speaking only when the callee contains memory
> > +	     accesses that are not using alias-set zero anyway.  */
> > +	  || (opt_for_fn (e->caller->decl, flag_strict_aliasing)
> > +	      != opt_for_fn (e->caller->decl, flag_strict_aliasing))
> > +	  /* Strictly speaking only when the callee uses FP math.  */
> > +	  || (opt_for_fn (e->caller->decl, flag_rounding_math)
> > +	      != opt_for_fn (e->caller->decl, flag_rounding_math))
> > +	  || (opt_for_fn (e->caller->decl, flag_trapping_math)
> > +	      != opt_for_fn (e->caller->decl, flag_trapping_math))
> > +	  || (opt_for_fn (e->caller->decl, flag_unsafe_math_optimizations)
> > +	      != opt_for_fn (e->caller->decl, flag_unsafe_math_optimizations))
> > +	  || (opt_for_fn (e->caller->decl, flag_finite_math_only)
> > +	      != opt_for_fn (e->caller->decl, flag_finite_math_only))
> > +	  || (opt_for_fn (e->caller->decl, flag_signaling_nans)
> > +	      != opt_for_fn (e->caller->decl, flag_signaling_nans))
> > +	  || (opt_for_fn (e->caller->decl, flag_cx_limited_range)
> > +	      != opt_for_fn (e->caller->decl, flag_cx_limited_range))
> > +	  || (opt_for_fn (e->caller->decl, flag_signed_zeros)
> > +	      != opt_for_fn (e->caller->decl, flag_signed_zeros))
> > +	  || (opt_for_fn (e->caller->decl, flag_associative_math)
> > +	      != opt_for_fn (e->caller->decl, flag_associative_math))
> > +	  || (opt_for_fn (e->caller->decl, flag_reciprocal_math)
> > +	      != opt_for_fn (e->caller->decl, flag_reciprocal_math))
> > +	  /* Strictly speaking only when the callee contains function
> > +	     calls that may end up setting errno.  */
> > +	  || (opt_for_fn (e->caller->decl, flag_errno_math)
> > +	      != opt_for_fn (e->caller->decl, flag_errno_math)))
> > +	{
> > +	  e->inline_failed = CIF_OPTIMIZATION_MISMATCH;
> > +	  inlinable = false;
> > +	}
> > +      /* gcc.dg/pr43564.c.  Apply user-forced inline even at -O0.  */
> > +      else if (DECL_DISREGARD_INLINE_LIMITS (callee->decl)
> > +	       && lookup_attribute ("always_inline",
> > +				    DECL_ATTRIBUTES (callee->decl)))
> >   	;
> > -      /* When user added an attribute, honnor it.  */
> > -      else if ((lookup_attribute ("optimize", DECL_ATTRIBUTES
> > (caller->decl))
> > -		|| lookup_attribute ("optimize",
> > -				     DECL_ATTRIBUTES (callee->decl)))
> > -	       && ((opt_for_fn (caller->decl, optimize)
> > -		   > opt_for_fn (callee->decl, optimize))
> > -		   || (opt_for_fn (caller->decl, optimize_size)
> > -		       != opt_for_fn (callee->decl, optimize_size))))
> > +      /* When user added an attribute to the callee honor it.  */
> > +      else if (lookup_attribute ("optimize", DECL_ATTRIBUTES
> > (callee->decl))
> > +	       && opts_for_fn (caller->decl) != opts_for_fn (callee->decl))
> >   	{
> >   	  e->inline_failed = CIF_OPTIMIZATION_MISMATCH;
> >   	  inlinable = false;
> > 
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Jennifer Guild,
Dilip Upmanyu, Graham Norton HRB 21284 (AG Nuernberg)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix inlining checks wrt optimize attribute
  2015-01-22 10:23 [PATCH] Fix inlining checks wrt optimize attribute Richard Biener
  2015-01-22 10:36 ` Christian Bruel
@ 2015-01-22 19:42 ` Jan Hubicka
  2015-01-23 10:05   ` Richard Biener
  1 sibling, 1 reply; 6+ messages in thread
From: Jan Hubicka @ 2015-01-22 19:42 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches, Jan Hubicka

> 
> As said in the other thread - this makes sure we don't perform inlining
> that might end up generating invalid code.  It also preserves
> user-provided optimize attributes more properly.
> 
> Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.
> 
> Richard.
> 
> 2015-01-22  Richard Biener  <rguenther@suse.de>
> 
> 	* ipa-inline.c (can_inline_edge_p): Disable inlining of edges
> 	with IL incompatible options.  Properly honor user optimize
> 	attributes.

Thanks, in fact I had similar patch in queue, just wanted to catch you on IRC
and discuss posibility of making this a flag in common.opt

Honza
> 
> Index: gcc/ipa-inline.c
> ===================================================================
> --- gcc/ipa-inline.c	(revision 219929)
> +++ gcc/ipa-inline.c	(working copy)
> @@ -404,17 +404,56 @@ can_inline_edge_p (struct cgraph_edge *e
>       optimization attribute.  */
>    else if (caller_tree != callee_tree)
>      {
> -      /* gcc.dg/pr43564.c.  Look at forced inline even in -O0.  */
> -      if (DECL_DISREGARD_INLINE_LIMITS (callee->decl))
> +      /* There are some options that change IL semantics which means
> +         we cannot inline in these cases for correctness reason.
> +	 Not even for always_inline declared functions.  */
> +      /* Strictly speaking only when the callee contains signed integer
> +         math where overflow is undefined.  */
> +      if ((opt_for_fn (e->caller->decl, flag_strict_overflow)
> +	   != opt_for_fn (e->caller->decl, flag_strict_overflow))
> +	  || (opt_for_fn (e->caller->decl, flag_wrapv)
> +	      != opt_for_fn (e->caller->decl, flag_wrapv))
> +	  || (opt_for_fn (e->caller->decl, flag_trapv)
> +	      != opt_for_fn (e->caller->decl, flag_trapv))
> +	  /* Strictly speaking only when the callee contains memory
> +	     accesses that are not using alias-set zero anyway.  */
> +	  || (opt_for_fn (e->caller->decl, flag_strict_aliasing)
> +	      != opt_for_fn (e->caller->decl, flag_strict_aliasing))
> +	  /* Strictly speaking only when the callee uses FP math.  */
> +	  || (opt_for_fn (e->caller->decl, flag_rounding_math)
> +	      != opt_for_fn (e->caller->decl, flag_rounding_math))
> +	  || (opt_for_fn (e->caller->decl, flag_trapping_math)
> +	      != opt_for_fn (e->caller->decl, flag_trapping_math))
> +	  || (opt_for_fn (e->caller->decl, flag_unsafe_math_optimizations)
> +	      != opt_for_fn (e->caller->decl, flag_unsafe_math_optimizations))
> +	  || (opt_for_fn (e->caller->decl, flag_finite_math_only)
> +	      != opt_for_fn (e->caller->decl, flag_finite_math_only))
> +	  || (opt_for_fn (e->caller->decl, flag_signaling_nans)
> +	      != opt_for_fn (e->caller->decl, flag_signaling_nans))
> +	  || (opt_for_fn (e->caller->decl, flag_cx_limited_range)
> +	      != opt_for_fn (e->caller->decl, flag_cx_limited_range))
> +	  || (opt_for_fn (e->caller->decl, flag_signed_zeros)
> +	      != opt_for_fn (e->caller->decl, flag_signed_zeros))
> +	  || (opt_for_fn (e->caller->decl, flag_associative_math)
> +	      != opt_for_fn (e->caller->decl, flag_associative_math))
> +	  || (opt_for_fn (e->caller->decl, flag_reciprocal_math)
> +	      != opt_for_fn (e->caller->decl, flag_reciprocal_math))
> +	  /* Strictly speaking only when the callee contains function
> +	     calls that may end up setting errno.  */
> +	  || (opt_for_fn (e->caller->decl, flag_errno_math)
> +	      != opt_for_fn (e->caller->decl, flag_errno_math)))
> +	{
> +	  e->inline_failed = CIF_OPTIMIZATION_MISMATCH;
> +	  inlinable = false;
> +	}
> +      /* gcc.dg/pr43564.c.  Apply user-forced inline even at -O0.  */
> +      else if (DECL_DISREGARD_INLINE_LIMITS (callee->decl)
> +	       && lookup_attribute ("always_inline",
> +				    DECL_ATTRIBUTES (callee->decl)))
>  	;
> -      /* When user added an attribute, honnor it.  */
> -      else if ((lookup_attribute ("optimize", DECL_ATTRIBUTES (caller->decl))
> -		|| lookup_attribute ("optimize",
> -				     DECL_ATTRIBUTES (callee->decl)))
> -	       && ((opt_for_fn (caller->decl, optimize)
> -		   > opt_for_fn (callee->decl, optimize))
> -		   || (opt_for_fn (caller->decl, optimize_size)
> -		       != opt_for_fn (callee->decl, optimize_size))))
> +      /* When user added an attribute to the callee honor it.  */
> +      else if (lookup_attribute ("optimize", DECL_ATTRIBUTES (callee->decl))
> +	       && opts_for_fn (caller->decl) != opts_for_fn (callee->decl))
>  	{
>  	  e->inline_failed = CIF_OPTIMIZATION_MISMATCH;
>  	  inlinable = false;

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix inlining checks wrt optimize attribute
  2015-01-22 19:42 ` Jan Hubicka
@ 2015-01-23 10:05   ` Richard Biener
  0 siblings, 0 replies; 6+ messages in thread
From: Richard Biener @ 2015-01-23 10:05 UTC (permalink / raw)
  To: Jan Hubicka; +Cc: gcc-patches

On Thu, 22 Jan 2015, Jan Hubicka wrote:

> > 
> > As said in the other thread - this makes sure we don't perform inlining
> > that might end up generating invalid code.  It also preserves
> > user-provided optimize attributes more properly.
> > 
> > Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.
> > 
> > Richard.
> > 
> > 2015-01-22  Richard Biener  <rguenther@suse.de>
> > 
> > 	* ipa-inline.c (can_inline_edge_p): Disable inlining of edges
> > 	with IL incompatible options.  Properly honor user optimize
> > 	attributes.
> 
> Thanks, in fact I had similar patch in queue, just wanted to catch you on IRC
> and discuss posibility of making this a flag in common.opt

Sure - I briefly considered this but we're in stage4 now so I was just
fixing the obvious correctness issues.

We also have to fix

  /* Don't inline if the callee can throw non-call exceptions but the
     caller cannot.
     FIXME: this is obviously wrong for LTO where STRUCT_FUNCTION is 
missing.
     Move the flag into cgraph node or mirror it in the inline summary.  
*/
  else if (callee_fun && callee_fun->can_throw_non_call_exceptions
           && !(caller_fun && caller_fun->can_throw_non_call_exceptions))
    {
      e->inline_failed = CIF_NON_CALL_EXCEPTIONS;
      inlinable = false;
    }

in some way (eventually we can just use opt_for_fn 
()->flag_non_call_exceptions).

Richard.

> Honza
> > 
> > Index: gcc/ipa-inline.c
> > ===================================================================
> > --- gcc/ipa-inline.c	(revision 219929)
> > +++ gcc/ipa-inline.c	(working copy)
> > @@ -404,17 +404,56 @@ can_inline_edge_p (struct cgraph_edge *e
> >       optimization attribute.  */
> >    else if (caller_tree != callee_tree)
> >      {
> > -      /* gcc.dg/pr43564.c.  Look at forced inline even in -O0.  */
> > -      if (DECL_DISREGARD_INLINE_LIMITS (callee->decl))
> > +      /* There are some options that change IL semantics which means
> > +         we cannot inline in these cases for correctness reason.
> > +	 Not even for always_inline declared functions.  */
> > +      /* Strictly speaking only when the callee contains signed integer
> > +         math where overflow is undefined.  */
> > +      if ((opt_for_fn (e->caller->decl, flag_strict_overflow)
> > +	   != opt_for_fn (e->caller->decl, flag_strict_overflow))
> > +	  || (opt_for_fn (e->caller->decl, flag_wrapv)
> > +	      != opt_for_fn (e->caller->decl, flag_wrapv))
> > +	  || (opt_for_fn (e->caller->decl, flag_trapv)
> > +	      != opt_for_fn (e->caller->decl, flag_trapv))
> > +	  /* Strictly speaking only when the callee contains memory
> > +	     accesses that are not using alias-set zero anyway.  */
> > +	  || (opt_for_fn (e->caller->decl, flag_strict_aliasing)
> > +	      != opt_for_fn (e->caller->decl, flag_strict_aliasing))
> > +	  /* Strictly speaking only when the callee uses FP math.  */
> > +	  || (opt_for_fn (e->caller->decl, flag_rounding_math)
> > +	      != opt_for_fn (e->caller->decl, flag_rounding_math))
> > +	  || (opt_for_fn (e->caller->decl, flag_trapping_math)
> > +	      != opt_for_fn (e->caller->decl, flag_trapping_math))
> > +	  || (opt_for_fn (e->caller->decl, flag_unsafe_math_optimizations)
> > +	      != opt_for_fn (e->caller->decl, flag_unsafe_math_optimizations))
> > +	  || (opt_for_fn (e->caller->decl, flag_finite_math_only)
> > +	      != opt_for_fn (e->caller->decl, flag_finite_math_only))
> > +	  || (opt_for_fn (e->caller->decl, flag_signaling_nans)
> > +	      != opt_for_fn (e->caller->decl, flag_signaling_nans))
> > +	  || (opt_for_fn (e->caller->decl, flag_cx_limited_range)
> > +	      != opt_for_fn (e->caller->decl, flag_cx_limited_range))
> > +	  || (opt_for_fn (e->caller->decl, flag_signed_zeros)
> > +	      != opt_for_fn (e->caller->decl, flag_signed_zeros))
> > +	  || (opt_for_fn (e->caller->decl, flag_associative_math)
> > +	      != opt_for_fn (e->caller->decl, flag_associative_math))
> > +	  || (opt_for_fn (e->caller->decl, flag_reciprocal_math)
> > +	      != opt_for_fn (e->caller->decl, flag_reciprocal_math))
> > +	  /* Strictly speaking only when the callee contains function
> > +	     calls that may end up setting errno.  */
> > +	  || (opt_for_fn (e->caller->decl, flag_errno_math)
> > +	      != opt_for_fn (e->caller->decl, flag_errno_math)))
> > +	{
> > +	  e->inline_failed = CIF_OPTIMIZATION_MISMATCH;
> > +	  inlinable = false;
> > +	}
> > +      /* gcc.dg/pr43564.c.  Apply user-forced inline even at -O0.  */
> > +      else if (DECL_DISREGARD_INLINE_LIMITS (callee->decl)
> > +	       && lookup_attribute ("always_inline",
> > +				    DECL_ATTRIBUTES (callee->decl)))
> >  	;
> > -      /* When user added an attribute, honnor it.  */
> > -      else if ((lookup_attribute ("optimize", DECL_ATTRIBUTES (caller->decl))
> > -		|| lookup_attribute ("optimize",
> > -				     DECL_ATTRIBUTES (callee->decl)))
> > -	       && ((opt_for_fn (caller->decl, optimize)
> > -		   > opt_for_fn (callee->decl, optimize))
> > -		   || (opt_for_fn (caller->decl, optimize_size)
> > -		       != opt_for_fn (callee->decl, optimize_size))))
> > +      /* When user added an attribute to the callee honor it.  */
> > +      else if (lookup_attribute ("optimize", DECL_ATTRIBUTES (callee->decl))
> > +	       && opts_for_fn (caller->decl) != opts_for_fn (callee->decl))
> >  	{
> >  	  e->inline_failed = CIF_OPTIMIZATION_MISMATCH;
> >  	  inlinable = false;
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Jennifer Guild,
Dilip Upmanyu, Graham Norton HRB 21284 (AG Nuernberg)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix inlining checks wrt optimize attribute
  2015-01-22 11:03   ` Richard Biener
@ 2015-04-17  6:51     ` Christian Bruel
  0 siblings, 0 replies; 6+ messages in thread
From: Christian Bruel @ 2015-04-17  6:51 UTC (permalink / raw)
  To: Richard Biener; +Cc: ramana Radhakrishnan, gcc-patches

On 01/22/2015 11:33 AM, Richard Biener wrote:
> On Thu, 22 Jan 2015, Christian Bruel wrote:
> 
>> Hi Richard,
>>
>> I thought one of my current issue would be solved by this patch, but it is not
>> : I have some inlining failures with the attribute target on ARM. (e.g
>> inline-3.c) where obvious early inline fails with because we fail into the
>> last can_inline_edge_p case:
>>
>> opt_for_fn (callee->decl, optimize)
>> 		   >= opt_for_fn (caller->decl, optimize)))
>>
>> when callee and caller are both -O2 and targetm.target_option.can_inline_p was
>> true, they should be inlined as in the general case (no
>> DECL_FUNCTION_SPECIFIC_OPTIMIZATION)
>>
>> I'm currently testing this additional change:
>>
>> Index: ipa-inline.c
>> ===================================================================
>> --- ipa-inline.c	(revision 219989)
>> +++ ipa-inline.c	(working copy)
>> @@ -489,7 +489,7 @@
>>        else if (opt_for_fn (callee->decl, optimize_size)
>>  	       < opt_for_fn (caller->decl, optimize_size)
>>  	       || (opt_for_fn (callee->decl, optimize)
>> -		   >= opt_for_fn (caller->decl, optimize)))
>> +		   > opt_for_fn (caller->decl, optimize)))
>>  	{
>>  	  if (estimate_edge_time (e)
>>  	      >= 20 + inline_edge_summary (e)->call_stmt_time)
>>
>> Since this is a hot topic for you, I though you would have useful comments on
>> this before I ask for a commit (when stage 4 close) ?
> 
> Yeah - the above looks like an obvious change to me.  Thus,
> approved if it passes bootstrap/regtest.
> 
> Thanks,
> Richard.
> 

thanks, sorry for the delay (stage1 blocked)

committed with the Changelog:

* ipa-inline.c (can_inline_edge_p): Allow inlining of functions with
same attributes.

This frees the road for

 - [PATCH, x86] [PR target/64835] Add
TARGET_OVERRIDE_OPTIONS_AFTER_CHANGE hook
 https://gcc.gnu.org/ml/gcc-patches/2015-04/msg00594.html

 and

 - [PATCH, ARM] attribute target (thumb,arm) [0-6]
  https://gcc.gnu.org/ml/gcc-patches/2015-04/msg00706.html

with new regressions tests.



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2015-04-17  6:51 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-22 10:23 [PATCH] Fix inlining checks wrt optimize attribute Richard Biener
2015-01-22 10:36 ` Christian Bruel
2015-01-22 11:03   ` Richard Biener
2015-04-17  6:51     ` Christian Bruel
2015-01-22 19:42 ` Jan Hubicka
2015-01-23 10:05   ` Richard Biener

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).