Fix PR ipa/60315 (inliner explosion)

Hi,
this patch fixes compile time issue in the testcase that is caused by fact that the inliner
is repeatedly inlining into an call it earlier proved to be unreachable.  The analysis part
knows that such calls are not accounted into overall function summaries, but the transform
part sees them merely as cold calls and thus it attempts to do inlining for size.
This patch makes analysis part to practively redirect all known to be unreachable
calls to BUILTIN_UNREAHABLE.

Doing so uncovered the bug in ipa-pure-const I fixed earlier and also another
but in set_cond_stmt_execution_predicate where invert_tree_comparison is
called and the condition is used further. The function returns ERROR_MARK for FP
comparsions in some cases (though I think it should not since there seems to
be no toher way to invert comparsion without this and we have the unordered codes).
This concide with ipa-inline-analysis.c internal use of ERROR_MARK and leads to
inconsistent predicates.

Bootstrapped/regtested x86_64-linux, comitted.

	PR ipa/60315
	* cif-code.def (UNREACHABLE) New code.
	* ipa-inline.c (inline_small_functions): Skip edges to __builtlin_unreachable.
	(estimate_edge_growth): Allow edges to __builtlin_unreachable.
	* ipa-inline-analysis.c (edge_set_predicate): Redirect edges with false
	predicate to __bulitin_unreachable.
	(set_cond_stmt_execution_predicate): Fix issue when invert_tree_comparison
	returns ERROR_MARK.
	* ipa-pure-const.c (propagate_pure_const, propagate_nothrow): Do not
	propagate to inline clones.
	* cgraph.c (verify_edge_corresponds_to_fndecl): Allow redirection
	to unreachable.
	* ipa-cp.c (create_specialized_node): Be ready for new node to appear.
	* cgraphclones.c (cgraph_clone_node): If call destination is already
	ureachable, do not redirect it back.
	* tree-inline.c (fold_marked_statements): Hanlde calls becoming
	unreachable.

	* testsuite/g++.dg/torture/pr60315.C: New testcase.

On 26 March 2014 03:17:11 Jan Hubicka <hubicka@ucw.cz> wrote:

> Hi,

Just 2 nits, cannot comment on the patch itself.
s/clonning/cloning/g as usual :)
And the content of the testcase is duplicated.

Thanks,

> this patch fixes compile time issue in the testcase that is caused by fact 
> that the inliner
> is repeatedly inlining into an call it earlier proved to be unreachable.  
> The analysis part
> knows that such calls are not accounted into overall function summaries, 
> but the transform
> part sees them merely as cold calls and thus it attempts to do inlining for 
> size.
> This patch makes analysis part to practively redirect all known to be 
> unreachable
> calls to BUILTIN_UNREAHABLE.
>
> Doing so uncovered the bug in ipa-pure-const I fixed earlier and also another
> but in set_cond_stmt_execution_predicate where invert_tree_comparison is
> called and the condition is used further. The function returns ERROR_MARK 
> for FP
> comparsions in some cases (though I think it should not since there seems to
> be no toher way to invert comparsion without this and we have the unordered 
> codes).
> This concide with ipa-inline-analysis.c internal use of ERROR_MARK and leads to
> inconsistent predicates.
>
> Bootstrapped/regtested x86_64-linux, comitted.
>
> 	PR ipa/60315
> 	* cif-code.def (UNREACHABLE) New code.
> 	* ipa-inline.c (inline_small_functions): Skip edges to __builtlin_unreachable.
> 	(estimate_edge_growth): Allow edges to __builtlin_unreachable.
> 	* ipa-inline-analysis.c (edge_set_predicate): Redirect edges with false
> 	predicate to __bulitin_unreachable.
> 	(set_cond_stmt_execution_predicate): Fix issue when invert_tree_comparison
> 	returns ERROR_MARK.
> 	* ipa-pure-const.c (propagate_pure_const, propagate_nothrow): Do not
> 	propagate to inline clones.
> 	* cgraph.c (verify_edge_corresponds_to_fndecl): Allow redirection
> 	to unreachable.
> 	* ipa-cp.c (create_specialized_node): Be ready for new node to appear.
> 	* cgraphclones.c (cgraph_clone_node): If call destination is already
> 	ureachable, do not redirect it back.
> 	* tree-inline.c (fold_marked_statements): Hanlde calls becoming
> 	unreachable.
>
> 	* testsuite/g++.dg/torture/pr60315.C: New testcase.
> Index: cif-code.def
> ===================================================================
> --- cif-code.def	(revision 208829)
> +++ cif-code.def	(working copy)
> @@ -127,3 +127,7 @@ DEFCIFCODE(USES_COMDAT_LOCAL, CIF_FINAL_
>  /* We can't inline because of mismatched caller/callee attributes.  */
>  DEFCIFCODE(ATTRIBUTE_MISMATCH, CIF_FINAL_NORMAL,
>  	   N_("function attribute mismatch"))
> +
> +/* We proved that the call is unreachable.  */
> +DEFCIFCODE(UNREACHABLE, CIF_FINAL_NORMAL,
> +	   N_("unreachable"))
> Index: cgraphclones.c
> ===================================================================
> --- cgraphclones.c	(revision 208829)
> +++ cgraphclones.c	(working copy)
> @@ -238,8 +238,12 @@ cgraph_clone_node (struct cgraph_node *n
>    FOR_EACH_VEC_ELT (redirect_callers, i, e)
>      {
>        /* Redirect calls to the old version node to point to its new
> -	 version.  */
> -      cgraph_redirect_edge_callee (e, new_node);
> +	 version.  The only exception is when the edge was proved to
> +	 be unreachable during the clonning procedure.  */
> +      if (!e->callee
> +	  || DECL_BUILT_IN_CLASS (e->callee->decl) != BUILT_IN_NORMAL
> +	  || DECL_FUNCTION_CODE (e->callee->decl) != BUILT_IN_UNREACHABLE)
> +        cgraph_redirect_edge_callee (e, new_node);
>      }
>
>
> Index: ipa-inline.c
> ===================================================================
> --- ipa-inline.c	(revision 208829)
> +++ ipa-inline.c	(working copy)
> @@ -1685,7 +1685,7 @@ inline_small_functions (void)
>        edge = (struct cgraph_edge *) fibheap_extract_min (edge_heap);
>        gcc_assert (edge->aux);
>        edge->aux = NULL;
> -      if (!edge->inline_failed)
> +      if (!edge->inline_failed || !edge->callee->analyzed)
>  	continue;
>
>        /* Be sure that caches are maintained consistent.
> Index: ipa-inline.h
> ===================================================================
> --- ipa-inline.h	(revision 208829)
> +++ ipa-inline.h	(working copy)
> @@ -285,7 +285,8 @@ static inline int
>  estimate_edge_growth (struct cgraph_edge *edge)
>  {
>  #ifdef ENABLE_CHECKING
> -  gcc_checking_assert (inline_edge_summary (edge)->call_stmt_size);
> +  gcc_checking_assert (inline_edge_summary (edge)->call_stmt_size
> +		       || !edge->callee->analyzed);
>  #endif
>    return (estimate_edge_size (edge)
>  	  - inline_edge_summary (edge)->call_stmt_size);
> Index: testsuite/g++.dg/torture/pr60315.C
> ===================================================================
> --- testsuite/g++.dg/torture/pr60315.C	(revision 0)
> +++ testsuite/g++.dg/torture/pr60315.C	(revision 0)
> @@ -0,0 +1,32 @@
> +// { dg-do compile }
> +struct Base {
> +    virtual int f() = 0;
> +};
> +
> +struct Derived : public Base {
> +    virtual int f() final override {
> +        return 42;
> +    }
> +};
> +
> +extern Base* b;
> +
> +int main() {
> +    return (static_cast<Derived*>(b)->*(&Derived::f))();
> +}
> +// { dg-do compile }
> +struct Base {
> +    virtual int f() = 0;
> +};
> +
> +struct Derived : public Base {
> +    virtual int f() final override {
> +        return 42;
> +    }
> +};
> +
> +extern Base* b;
> +
> +int main() {
> +    return (static_cast<Derived*>(b)->*(&Derived::f))();
> +}
> Index: cgraph.c
> ===================================================================
> --- cgraph.c	(revision 208829)
> +++ cgraph.c	(working copy)
> @@ -2612,6 +2612,12 @@ verify_edge_corresponds_to_fndecl (struc
>        || node->in_other_partition
>        || e->callee->in_other_partition)
>      return false;
> +
> +  /* Optimizers can redirect unreachable calls or calls triggering undefined
> +     behaviour to builtin_unreachable.  */
> +  if (DECL_BUILT_IN_CLASS (e->callee->decl) == BUILT_IN_NORMAL
> +      && DECL_FUNCTION_CODE (e->callee->decl) == BUILT_IN_UNREACHABLE)
> +    return false;
>    node = cgraph_function_or_thunk_node (node, NULL);
>
>    if (e->callee->former_clone_of != node->decl
> Index: ipa-inline-analysis.c
> ===================================================================
> --- ipa-inline-analysis.c	(revision 208829)
> +++ ipa-inline-analysis.c	(working copy)
> @@ -746,6 +746,20 @@ static void
>  edge_set_predicate (struct cgraph_edge *e, struct predicate *predicate)
>  {
>    struct inline_edge_summary *es = inline_edge_summary (e);
> +
> +  /* If the edge is determined to be never executed, redirect it
> +     to BUILTIN_UNREACHABLE to save inliner from inlining into it.  */
> +  if (predicate && false_predicate_p (predicate) && e->callee)
> +    {
> +      struct cgraph_node *callee = !e->inline_failed ? e->callee : NULL;
> +
> +      cgraph_redirect_edge_callee (e,
> +				   cgraph_get_create_node
> +				     (builtin_decl_implicit (BUILT_IN_UNREACHABLE)));
> +      e->inline_failed = CIF_UNREACHABLE;
> +      if (callee)
> +	cgraph_remove_node_and_inline_clones (callee, NULL);
> +    }
>    if (predicate && !true_predicate_p (predicate))
>      {
>        if (!es->predicate)
> @@ -1724,12 +1738,20 @@ set_cond_stmt_execution_predicate (struc
>
>        FOR_EACH_EDGE (e, ei, bb->succs)
>  	{
> -	  struct predicate p = add_condition (summary, index, &aggpos,
> -					      e->flags & EDGE_TRUE_VALUE
> -					      ? code : inverted_code,
> -					      gimple_cond_rhs (last));
> -	  e->aux = pool_alloc (edge_predicate_pool);
> -	  *(struct predicate *) e->aux = p;
> +	  enum tree_code this_code = (e->flags & EDGE_TRUE_VALUE
> +				      ? code : inverted_code);
> +	  /* invert_tree_comparison will return ERROR_MARK on FP
> +	     comparsions that are not EQ/NE instead of returning proper
> +	     unordered one.  Be sure it is not confused with NON_CONSTANT.  */
> +	  if (this_code != ERROR_MARK)
> +	    {
> +	      struct predicate p = add_condition (summary, index, &aggpos,
> +						  e->flags & EDGE_TRUE_VALUE
> +						  ? code : inverted_code,
> +						  gimple_cond_rhs (last));
> +	      e->aux = pool_alloc (edge_predicate_pool);
> +	      *(struct predicate *) e->aux = p;
> +	    }
>  	}
>      }
>
> Index: ipa-cp.c
> ===================================================================
> --- ipa-cp.c	(revision 208829)
> +++ ipa-cp.c	(working copy)
> @@ -2811,9 +2811,7 @@ create_specialized_node (struct cgraph_n
>        if (aggvals)
>  	ipa_dump_agg_replacement_values (dump_file, aggvals);
>      }
> -  gcc_checking_assert (ipa_node_params_vector.exists ()
> -		       && (ipa_node_params_vector.length ()
> -			   > (unsigned) cgraph_max_uid));
> +  ipa_check_create_node_params ();
>    update_profiling_info (node, new_node);
>    new_info = IPA_NODE_REF (new_node);
>    new_info->ipcp_orig_node = node;



Sent with AquaMail for Android
http://www.aqua-mail.com

> Bootstrapped/regtested x86_64-linux, comitted.

Not with Ada apparently, resulting in 

                === acats tests ===
FAIL:   c34007d
FAIL:   c34007g
FAIL:   c34007s
FAIL:   c37213j
FAIL:   c37213k
FAIL:   c37213l
FAIL:   ce2201g
FAIL:   cxa5a03
FAIL:   cxa5a04
FAIL:   cxa5a06
FAIL:   cxg2013
FAIL:   cxg2015

                === acats Summary ===
# of expected passes            2308
# of unexpected failures        12

Reduced testcase attached, compile p.adb at -O2.

> > Bootstrapped/regtested x86_64-linux, comitted.
> 
> Not with Ada apparently, resulting in 
> 
>                 === acats tests ===
> FAIL:   c34007d
> FAIL:   c34007g
> FAIL:   c34007s
> FAIL:   c37213j
> FAIL:   c37213k
> FAIL:   c37213l
> FAIL:   ce2201g
> FAIL:   cxa5a03
> FAIL:   cxa5a04
> FAIL:   cxa5a06
> FAIL:   cxg2013
> FAIL:   cxg2015
> 
>                 === acats Summary ===
> # of expected passes            2308
> # of unexpected failures        12
> 
> Reduced testcase attached, compile p.adb at -O2.

I will check, thanks for the reduced testcase. It seems like another case where
we get predicate wrong that ought to be fixed, of course.

Honza

Jan Hubicka <hubicka@ucw.cz> writes:

> Index: testsuite/g++.dg/torture/pr60315.C
> ===================================================================
> --- testsuite/g++.dg/torture/pr60315.C	(revision 0)
> +++ testsuite/g++.dg/torture/pr60315.C	(revision 0)
> @@ -0,0 +1,32 @@
> +// { dg-do compile }
> +struct Base {
> +    virtual int f() = 0;
> +};
> +
> +struct Derived : public Base {
> +    virtual int f() final override {
> +        return 42;
> +    }
> +};
> +
> +extern Base* b;
> +
> +int main() {
> +    return (static_cast<Derived*>(b)->*(&Derived::f))();
> +}

FAIL: g++.dg/torture/pr60315.C  -O0  (test for excess errors)
Excess errors:
/usr/local/gcc/gcc-20140327/gcc/testsuite/g++.dg/torture/pr60315.C:7:19: warning: override controls (override/final) only available with -std=c++11 or -std=gnu++11
/usr/local/gcc/gcc-20140327/gcc/testsuite/g++.dg/torture/pr60315.C:7:21: warning: override controls (override/final) only available with -std=c++11 or -std=gnu++11

Andreas.

> I will check, thanks for the reduced testcase. It seems like another case
> where we get predicate wrong that ought to be fixed, of course.

You're welcome.  Another bug introduced by the patch:

eric@polaris:~/build/gcc/native> gcc/xgcc -Bgcc -S opt33.adb -O
/home/eric/install/gcc/lib64/gcc/x86_64-suse-linux/4.9.0/adainclude/a-
coorse.adb: In function 'Opt33.My_Ordered_Sets.To_Set':
/home/eric/install/gcc/lib64/gcc/x86_64-suse-linux/4.9.0/adainclude/a-
coorse.adb:1983:4: error: static chain with function that doesn't use one
.builtin_unreachable (&tree, node_54, 1, 0B); [static-chain: &FRAME.437]
/home/eric/install/gcc/lib64/gcc/x86_64-suse-linux/4.9.0/adainclude/a-
coorse.adb:1983:4: error: static chain with function that doesn't use one
.builtin_unreachable (&tree, node_54, _60, node_57); [static-chain: 
&FRAME.437]
+===========================GNAT BUG DETECTED==============================+
| 4.9.0 20140327 (experimental) [trunk revision 208879] (x86_64-suse-linux) 
GCC error:|
| verify_gimple failed                                                     |
| Error detected around /home/eric/install/gcc/lib64/gcc/x86_64-suse-
linux/4.9.0/adainclude/a-coorse.adb:1983:4|

You can install the testcase as gnat.dg/opt33.adb in the testsuite.

> > I will check, thanks for the reduced testcase. It seems like another case
> > where we get predicate wrong that ought to be fixed, of course.
> 
> You're welcome.  Another bug introduced by the patch:
> 
> eric@polaris:~/build/gcc/native> gcc/xgcc -Bgcc -S opt33.adb -O
> /home/eric/install/gcc/lib64/gcc/x86_64-suse-linux/4.9.0/adainclude/a-
> coorse.adb: In function 'Opt33.My_Ordered_Sets.To_Set':
> /home/eric/install/gcc/lib64/gcc/x86_64-suse-linux/4.9.0/adainclude/a-
> coorse.adb:1983:4: error: static chain with function that doesn't use one
> .builtin_unreachable (&tree, node_54, 1, 0B); [static-chain: &FRAME.437]
> /home/eric/install/gcc/lib64/gcc/x86_64-suse-linux/4.9.0/adainclude/a-
> coorse.adb:1983:4: error: static chain with function that doesn't use one
> .builtin_unreachable (&tree, node_54, _60, node_57); [static-chain: 
> &FRAME.437]
> +===========================GNAT BUG DETECTED==============================+
> | 4.9.0 20140327 (experimental) [trunk revision 208879] (x86_64-suse-linux) 
> GCC error:|
> | verify_gimple failed                                                     |
> | Error detected around /home/eric/install/gcc/lib64/gcc/x86_64-suse-
> linux/4.9.0/adainclude/a-coorse.adb:1983:4|

Thanks, looks like another (semi)-latent bug.  I will add code to cgraph redirection
to get rid of static chain if it is uneeded.

Honza