diff mbox

Fix can_inline_edge_p and code marking calls unreachable

Message ID 20150326211237.GA75710@kam.mff.cuni.cz
State New
Headers show

Commit Message

Jan Hubicka March 26, 2015, 9:12 p.m. UTC
Hi,
this patch missed hunk adding CIF code that I commited now
	* cif-code.def (CILK_SPAWN): New code.

Comments

Christophe Lyon March 26, 2015, 10:58 p.m. UTC | #1
On 26 March 2015 at 22:12, Jan Hubicka <hubicka@ucw.cz> wrote:
> Hi,
> this patch missed hunk adding CIF code that I commited now
>         * cif-code.def (CILK_SPAWN): New code

Hi,
After this fix, I can see build failures in glibc:
key_call.c:574:1: internal compiler error: in inline_call, at
ipa-inline-transform.c:386
 }
 ^
0x10eb475 inline_call(cgraph_edge*, bool, vec<cgraph_edge*, va_heap,
vl_ptr>*, int*, bool, bool*)
        /tmp/9065933_3.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/ipa-inline-transform.c:381
0x10e43ed inline_small_functions
        /tmp/9065933_3.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/ipa-inline.c:1942
0x10e4ead ipa_inline
        /tmp/9065933_3.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/ipa-inline.c:2345

See http://abe.tcwglab.linaro.org/logs/validations/cross-validation/gcc-build/trunk/221710/build.html
for more detailed logs.

Thanks

Christophe

>
> --- trunk/gcc/cif-code.def      2015/03/26 20:37:53     221709
> +++ trunk/gcc/cif-code.def      2015/03/26 21:10:28     221710
> @@ -124,6 +124,10 @@
>  DEFCIFCODE(ATTRIBUTE_MISMATCH, CIF_FINAL_ERROR,
>            N_("function attribute mismatch"))
>
> +/* We can't inline because of mismatched caller/callee attributes.  */
> +DEFCIFCODE(CILK_SPAWN, CIF_FINAL_ERROR,
> +          N_("caller function contains cilk spawn"))
> +
>  /* We proved that the call is unreachable.  */
>  DEFCIFCODE(UNREACHABLE, CIF_FINAL_ERROR,
>            N_("unreachable"))
>
> I also noticed that this breaks one testcase which is cured by the following patch
> I am testing and will commit once it concludes. I apologize for the breakage.
>
> Honza
>
>         * ipa-inline.c (check_maybe_up, check_maybe_down, check_match):
>         New macros.
>         (can_inline_edge_p): Relax option matching for always inline functions.
> Index: ipa-inline.c
> ===================================================================
> --- ipa-inline.c        (revision 221706)
> +++ ipa-inline.c        (working copy)
> @@ -298,6 +298,27 @@ sanitize_attrs_match_for_inline_p (const
>        DECL_ATTRIBUTES (callee));
>  }
>
> +/* Used for flags where it is safe to inline when caller's value is
> +   grater than callee's.  */
> +#define check_maybe_up(flag) \
> +      (opts_for_fn (caller->decl)->x_##flag            \
> +       != opts_for_fn (callee->decl)->x_##flag         \
> +       && (!always_inline                              \
> +          || opts_for_fn (caller->decl)->x_##flag      \
> +             < opts_for_fn (callee->decl)->x_##flag))
> +/* Used for flags where it is safe to inline when caller's value is
> +   smaller than callee's.  */
> +#define check_maybe_down(flag) \
> +      (opts_for_fn (caller->decl)->x_##flag            \
> +       != opts_for_fn (callee->decl)->x_##flag         \
> +       && (!always_inline                              \
> +          || opts_for_fn (caller->decl)->x_##flag      \
> +             > opts_for_fn (callee->decl)->x_##flag))
> +/* Used for flags where exact match is needed for correctness.  */
> +#define check_match(flag) \
> +      (opts_for_fn (caller->decl)->x_##flag            \
> +       != opts_for_fn (callee->decl)->x_##flag)
> +
>   /* Decide if we can inline the edge and possibly update
>     inline_failed reason.
>     We check whether inlining is possible at all and whether
> @@ -401,74 +422,60 @@ can_inline_edge_p (struct cgraph_edge *e
>       optimization attribute.  */
>    else if (caller_tree != callee_tree)
>      {
> +      bool always_inline =
> +            (DECL_DISREGARD_INLINE_LIMITS (callee->decl)
> +             && lookup_attribute ("always_inline",
> +                                  DECL_ATTRIBUTES (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 (caller->decl, flag_strict_overflow)
> -          != opt_for_fn (callee->decl, flag_strict_overflow))
> -         || (opt_for_fn (caller->decl, flag_wrapv)
> -             != opt_for_fn (callee->decl, flag_wrapv))
> -         || (opt_for_fn (caller->decl, flag_trapv)
> -             != opt_for_fn (callee->decl, flag_trapv))
> +      if ((check_maybe_up (flag_strict_overflow)
> +          /* this flag is set by optimize.  Allow inlining across
> +             optimize boundary.  */
> +          && (!opt_for_fn (caller->decl, optimize)
> +              == !opt_for_fn (callee->decl, optimize) || !always_inline))
> +         || check_match (flag_wrapv)
> +         || check_match (flag_trapv)
>           /* Strictly speaking only when the callee contains memory
>              accesses that are not using alias-set zero anyway.  */
> -         || (opt_for_fn (caller->decl, flag_strict_aliasing)
> -             != opt_for_fn (callee->decl, flag_strict_aliasing))
> +         || check_maybe_down (flag_strict_aliasing)
>           /* Strictly speaking only when the callee uses FP math.  */
> -         || (opt_for_fn (caller->decl, flag_rounding_math)
> -             != opt_for_fn (callee->decl, flag_rounding_math))
> -         || (opt_for_fn (caller->decl, flag_trapping_math)
> -             != opt_for_fn (callee->decl, flag_trapping_math))
> -         || (opt_for_fn (caller->decl, flag_unsafe_math_optimizations)
> -             != opt_for_fn (callee->decl, flag_unsafe_math_optimizations))
> -         || (opt_for_fn (caller->decl, flag_finite_math_only)
> -             != opt_for_fn (callee->decl, flag_finite_math_only))
> -         || (opt_for_fn (caller->decl, flag_signaling_nans)
> -             != opt_for_fn (callee->decl, flag_signaling_nans))
> -         || (opt_for_fn (caller->decl, flag_cx_limited_range)
> -             != opt_for_fn (callee->decl, flag_cx_limited_range))
> -         || (opt_for_fn (caller->decl, flag_signed_zeros)
> -             != opt_for_fn (callee->decl, flag_signed_zeros))
> -         || (opt_for_fn (caller->decl, flag_associative_math)
> -             != opt_for_fn (callee->decl, flag_associative_math))
> -         || (opt_for_fn (caller->decl, flag_reciprocal_math)
> -             != opt_for_fn (callee->decl, flag_reciprocal_math))
> +         || check_maybe_up (flag_rounding_math)
> +         || check_maybe_up (flag_trapping_math)
> +         || check_maybe_down (flag_unsafe_math_optimizations)
> +         || check_maybe_down (flag_finite_math_only)
> +         || check_maybe_up (flag_signaling_nans)
> +         || check_maybe_down (flag_cx_limited_range)
> +         || check_maybe_up (flag_signed_zeros)
> +         || check_maybe_down (flag_associative_math)
> +         || check_maybe_down (flag_reciprocal_math)
>           /* We do not want to make code compiled with exceptions to be brought
>              into a non-EH function unless we know that the callee does not
>              throw.  This is tracked by DECL_FUNCTION_PERSONALITY.  */
> -         || (opt_for_fn (caller->decl, flag_non_call_exceptions)
> -             != opt_for_fn (callee->decl, flag_non_call_exceptions)
> +         || (check_match (flag_non_call_exceptions)
>               /* TODO: We also may allow bringing !flag_non_call_exceptions
>                  to flag_non_call_exceptions function, but that may need
>                  extra work in tree-inline to add the extra EH edges.  */
>               && (!opt_for_fn (callee->decl, flag_non_call_exceptions)
>                   || DECL_FUNCTION_PERSONALITY (callee->decl)))
> -         || (!opt_for_fn (caller->decl, flag_exceptions)
> -             && opt_for_fn (callee->decl, flag_exceptions)
> +         || (check_maybe_up (flag_exceptions)
>               && DECL_FUNCTION_PERSONALITY (callee->decl))
>           /* Strictly speaking only when the callee contains function
>              calls that may end up setting errno.  */
> -         || (opt_for_fn (caller->decl, flag_errno_math)
> -             != opt_for_fn (callee->decl, flag_errno_math))
> +         || check_maybe_up (flag_errno_math)
>           /* When devirtualization is diabled for callee, it is not safe
>              to inline it as we possibly mangled the type info.
>              Allow early inlining of always inlines.  */
> -         || (opt_for_fn (caller->decl, flag_devirtualize)
> -             && !opt_for_fn (callee->decl, flag_devirtualize)
> -             && (!early
> -                 || (!DECL_DISREGARD_INLINE_LIMITS (callee->decl)
> -                     || !lookup_attribute ("always_inline",
> -                                           DECL_ATTRIBUTES (callee->decl))))))
> +         || (!early && check_maybe_down (flag_devirtualize)))
>         {
>           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)))
> +      else if (always_inline)
>         ;
>        /* When user added an attribute to the callee honor it.  */
>        else if (lookup_attribute ("optimize", DECL_ATTRIBUTES (callee->decl))
Jan Hubicka March 26, 2015, 11:46 p.m. UTC | #2
> On 26 March 2015 at 22:12, Jan Hubicka <hubicka@ucw.cz> wrote:
> > Hi,
> > this patch missed hunk adding CIF code that I commited now
> >         * cif-code.def (CILK_SPAWN): New code
> 
> Hi,
> After this fix, I can see build failures in glibc:
> key_call.c:574:1: internal compiler error: in inline_call, at
> ipa-inline-transform.c:386
>  }
>  ^
> 0x10eb475 inline_call(cgraph_edge*, bool, vec<cgraph_edge*, va_heap,
> vl_ptr>*, int*, bool, bool*)
>         /tmp/9065933_3.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/ipa-inline-transform.c:381
> 0x10e43ed inline_small_functions
>         /tmp/9065933_3.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/ipa-inline.c:1942
> 0x10e4ead ipa_inline
>         /tmp/9065933_3.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/ipa-inline.c:2345
> 
> See http://abe.tcwglab.linaro.org/logs/validations/cross-validation/gcc-build/trunk/221710/build.html
> for more detailed logs.

Can you please send me preprocessed testcase? It is probably another misaccounting
bug in ipa-inline-analysis.  I may just silence this assert for this stage 4.

Honza
Jan Hubicka March 27, 2015, 2:05 a.m. UTC | #3
Hi,
I reproduced the problem of Firefox LTO (though small testcase would be welcome)
The problem is in a way we walk edges that now can be turned from indirect to direct.
I am working on a fix.

Honza
Markus Trippelsdorf March 27, 2015, 2:09 a.m. UTC | #4
On 2015.03.27 at 00:46 +0100, Jan Hubicka wrote:
> > On 26 March 2015 at 22:12, Jan Hubicka <hubicka@ucw.cz> wrote:
> > After this fix, I can see build failures in glibc:
> > key_call.c:574:1: internal compiler error: in inline_call, at
> > ipa-inline-transform.c:386
> 
> Can you please send me preprocessed testcase? It is probably another misaccounting
> bug in ipa-inline-analysis.  I may just silence this assert for this stage 4.

Also happens when building the Linux kernel:

trippels@gcc2-power8 linux-3.18.8 % cat nf_sockopt.i
int a;
int (*b)(), (*c)();
int fn1(int p1) {
  if (a)
    return 0;
  if (p1) {
    c();
    b();
  }
}
void fn2() { fn1(0); }

trippels@gcc2-power8 linux-3.18.8 % /home/trippels/gcc_test/usr/local/bin/gcc -O2 -c nf_sockopt.i
nf_sockopt.i:11:1: internal compiler error: in inline_call, at ipa-inline-transform.c:386
 void fn2() { fn1(0); }
 ^
0x10e435cf inline_call(cgraph_edge*, bool, vec<cgraph_edge*, va_heap, vl_ptr>*, int*, bool, bool*)
        ../../gcc/gcc/ipa-inline-transform.c:381
0x10e39a7b inline_small_functions
        ../../gcc/gcc/ipa-inline.c:1949
0x10e39a7b ipa_inline
        ../../gcc/gcc/ipa-inline.c:2352
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <http://gcc.gnu.org/bugs.html> for instructions.
diff mbox

Patch

--- trunk/gcc/cif-code.def	2015/03/26 20:37:53	221709
+++ trunk/gcc/cif-code.def	2015/03/26 21:10:28	221710
@@ -124,6 +124,10 @@ 
 DEFCIFCODE(ATTRIBUTE_MISMATCH, CIF_FINAL_ERROR,
 	   N_("function attribute mismatch"))
 
+/* We can't inline because of mismatched caller/callee attributes.  */
+DEFCIFCODE(CILK_SPAWN, CIF_FINAL_ERROR,
+	   N_("caller function contains cilk spawn"))
+
 /* We proved that the call is unreachable.  */
 DEFCIFCODE(UNREACHABLE, CIF_FINAL_ERROR,
 	   N_("unreachable"))

I also noticed that this breaks one testcase which is cured by the following patch
I am testing and will commit once it concludes. I apologize for the breakage.

Honza

	* ipa-inline.c (check_maybe_up, check_maybe_down, check_match):
	New macros.
	(can_inline_edge_p): Relax option matching for always inline functions.
Index: ipa-inline.c
===================================================================
--- ipa-inline.c	(revision 221706)
+++ ipa-inline.c	(working copy)
@@ -298,6 +298,27 @@  sanitize_attrs_match_for_inline_p (const
       DECL_ATTRIBUTES (callee));
 }
 
+/* Used for flags where it is safe to inline when caller's value is
+   grater than callee's.  */
+#define check_maybe_up(flag) \
+      (opts_for_fn (caller->decl)->x_##flag		\
+       != opts_for_fn (callee->decl)->x_##flag		\
+       && (!always_inline 				\
+	   || opts_for_fn (caller->decl)->x_##flag	\
+	      < opts_for_fn (callee->decl)->x_##flag))
+/* Used for flags where it is safe to inline when caller's value is
+   smaller than callee's.  */
+#define check_maybe_down(flag) \
+      (opts_for_fn (caller->decl)->x_##flag		\
+       != opts_for_fn (callee->decl)->x_##flag		\
+       && (!always_inline 				\
+	   || opts_for_fn (caller->decl)->x_##flag	\
+	      > opts_for_fn (callee->decl)->x_##flag))
+/* Used for flags where exact match is needed for correctness.  */
+#define check_match(flag) \
+      (opts_for_fn (caller->decl)->x_##flag		\
+       != opts_for_fn (callee->decl)->x_##flag)
+
  /* Decide if we can inline the edge and possibly update
    inline_failed reason.  
    We check whether inlining is possible at all and whether
@@ -401,74 +422,60 @@  can_inline_edge_p (struct cgraph_edge *e
      optimization attribute.  */
   else if (caller_tree != callee_tree)
     {
+      bool always_inline =
+	     (DECL_DISREGARD_INLINE_LIMITS (callee->decl)
+	      && lookup_attribute ("always_inline",
+				   DECL_ATTRIBUTES (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 (caller->decl, flag_strict_overflow)
-	   != opt_for_fn (callee->decl, flag_strict_overflow))
-	  || (opt_for_fn (caller->decl, flag_wrapv)
-	      != opt_for_fn (callee->decl, flag_wrapv))
-	  || (opt_for_fn (caller->decl, flag_trapv)
-	      != opt_for_fn (callee->decl, flag_trapv))
+      if ((check_maybe_up (flag_strict_overflow)
+	   /* this flag is set by optimize.  Allow inlining across
+	      optimize boundary.  */
+	   && (!opt_for_fn (caller->decl, optimize)
+	       == !opt_for_fn (callee->decl, optimize) || !always_inline))
+	  || check_match (flag_wrapv)
+	  || check_match (flag_trapv)
 	  /* Strictly speaking only when the callee contains memory
 	     accesses that are not using alias-set zero anyway.  */
-	  || (opt_for_fn (caller->decl, flag_strict_aliasing)
-	      != opt_for_fn (callee->decl, flag_strict_aliasing))
+	  || check_maybe_down (flag_strict_aliasing)
 	  /* Strictly speaking only when the callee uses FP math.  */
-	  || (opt_for_fn (caller->decl, flag_rounding_math)
-	      != opt_for_fn (callee->decl, flag_rounding_math))
-	  || (opt_for_fn (caller->decl, flag_trapping_math)
-	      != opt_for_fn (callee->decl, flag_trapping_math))
-	  || (opt_for_fn (caller->decl, flag_unsafe_math_optimizations)
-	      != opt_for_fn (callee->decl, flag_unsafe_math_optimizations))
-	  || (opt_for_fn (caller->decl, flag_finite_math_only)
-	      != opt_for_fn (callee->decl, flag_finite_math_only))
-	  || (opt_for_fn (caller->decl, flag_signaling_nans)
-	      != opt_for_fn (callee->decl, flag_signaling_nans))
-	  || (opt_for_fn (caller->decl, flag_cx_limited_range)
-	      != opt_for_fn (callee->decl, flag_cx_limited_range))
-	  || (opt_for_fn (caller->decl, flag_signed_zeros)
-	      != opt_for_fn (callee->decl, flag_signed_zeros))
-	  || (opt_for_fn (caller->decl, flag_associative_math)
-	      != opt_for_fn (callee->decl, flag_associative_math))
-	  || (opt_for_fn (caller->decl, flag_reciprocal_math)
-	      != opt_for_fn (callee->decl, flag_reciprocal_math))
+	  || check_maybe_up (flag_rounding_math)
+	  || check_maybe_up (flag_trapping_math)
+	  || check_maybe_down (flag_unsafe_math_optimizations)
+	  || check_maybe_down (flag_finite_math_only)
+	  || check_maybe_up (flag_signaling_nans)
+	  || check_maybe_down (flag_cx_limited_range)
+	  || check_maybe_up (flag_signed_zeros)
+	  || check_maybe_down (flag_associative_math)
+	  || check_maybe_down (flag_reciprocal_math)
 	  /* We do not want to make code compiled with exceptions to be brought
 	     into a non-EH function unless we know that the callee does not
 	     throw.  This is tracked by DECL_FUNCTION_PERSONALITY.  */
-	  || (opt_for_fn (caller->decl, flag_non_call_exceptions)
-	      != opt_for_fn (callee->decl, flag_non_call_exceptions)
+	  || (check_match (flag_non_call_exceptions)
 	      /* TODO: We also may allow bringing !flag_non_call_exceptions
 		 to flag_non_call_exceptions function, but that may need
 		 extra work in tree-inline to add the extra EH edges.  */
 	      && (!opt_for_fn (callee->decl, flag_non_call_exceptions)
 		  || DECL_FUNCTION_PERSONALITY (callee->decl)))
-	  || (!opt_for_fn (caller->decl, flag_exceptions)
-	      && opt_for_fn (callee->decl, flag_exceptions)
+	  || (check_maybe_up (flag_exceptions)
 	      && DECL_FUNCTION_PERSONALITY (callee->decl))
 	  /* Strictly speaking only when the callee contains function
 	     calls that may end up setting errno.  */
-	  || (opt_for_fn (caller->decl, flag_errno_math)
-	      != opt_for_fn (callee->decl, flag_errno_math))
+	  || check_maybe_up (flag_errno_math)
 	  /* When devirtualization is diabled for callee, it is not safe
 	     to inline it as we possibly mangled the type info.
 	     Allow early inlining of always inlines.  */
-	  || (opt_for_fn (caller->decl, flag_devirtualize)
-	      && !opt_for_fn (callee->decl, flag_devirtualize)
-	      && (!early
-		  || (!DECL_DISREGARD_INLINE_LIMITS (callee->decl)
-		      || !lookup_attribute ("always_inline",
-				            DECL_ATTRIBUTES (callee->decl))))))
+	  || (!early && check_maybe_down (flag_devirtualize)))
 	{
 	  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)))
+      else if (always_inline)
 	;
       /* When user added an attribute to the callee honor it.  */
       else if (lookup_attribute ("optimize", DECL_ATTRIBUTES (callee->decl))