Recent IPA regression with internal functions

Hi!

I've merged today gomp-4_0-branch from the trunk, but I'm seeing various
regressions.

The ICEs look like (e.g. on libgomp.c/simd-1.c (and other simd tests):

0xadf61a crash_signal
	../../gcc/toplev.c:335
0x571a0e tree_check2(tree_node*, char const*, int, char const*, tree_code, tree_code)
	../../gcc/tree.h:2667
0x8f5424 ipa_get_callee_param_type
	../../gcc/ipa-prop.c:1513
0x8f5662 ipa_compute_jump_functions_for_edge
	../../gcc/ipa-prop.c:1561
0x8f5c20 ipa_compute_jump_functions
	../../gcc/ipa-prop.c:1649
0x8f7337 ipa_analyze_node(cgraph_node*)
	../../gcc/ipa-prop.c:2169
0x10e85d0 ipcp_generate_summary
	../../gcc/ipa-cp.c:3624

The problem is in ipa_get_callee_param_type:
1509	  int n;
1510	  tree type = (e->callee
1511		       ? TREE_TYPE (e->callee->symbol.decl)
1512		       : gimple_call_fntype (e->call_stmt));
1513	  tree t = TYPE_ARG_TYPES (type);

e->call_stmt in this case is an internal builtin function, and
builtin internal functions don't have a fntype:
static inline tree
gimple_call_fntype (const_gimple gs)
{
  GIMPLE_CHECK (gs, GIMPLE_CALL);
  if (gimple_call_internal_p (gs))
    return NULL_TREE;
  return gs->gimple_call.u.fntype;
}

The following patch fixes this.  Is this ok?  I don't see ever that
it would be worth to try to do anything with internal calls, especially
when they are all magic.

2013-09-13  Jakub Jelinek  <jakub@redhat.com>

	* ipa-prop.c (ipa_compute_jump_functions_for_edge): Return early
	for internal calls.



	Jakub

> 2013-09-13  Jakub Jelinek  <jakub@redhat.com>
> 
> 	* ipa-prop.c (ipa_compute_jump_functions_for_edge): Return early
> 	for internal calls.

That seems resonable.  I wonder if we want to consider internal calls to form
callgarph edges at all: perhaps we can just modify cgraph builder+verifier
to skip them and consider them to be normal instruction....

Honza
> 
> --- gcc/ipa-prop.c.jj	2013-09-13 16:48:54.000000000 +0200
> +++ gcc/ipa-prop.c	2013-09-13 17:28:28.086058903 +0200
> @@ -1551,6 +1551,8 @@ ipa_compute_jump_functions_for_edge (str
>      return;
>    vec_safe_grow_cleared (args->jump_functions, arg_num);
>  
> +  if (gimple_call_internal_p (call))
> +    return;
>    if (ipa_func_spec_opts_forbid_analysis_p (cs->caller))
>      return;
>  
> 
> 
> 	Jakub

On Sun, Sep 15, 2013 at 09:08:00PM +0200, Jan Hubicka wrote:
> > 2013-09-13  Jakub Jelinek  <jakub@redhat.com>
> > 
> > 	* ipa-prop.c (ipa_compute_jump_functions_for_edge): Return early
> > 	for internal calls.
> 
> That seems resonable.  I wonder if we want to consider internal calls to form
> callgarph edges at all: perhaps we can just modify cgraph builder+verifier
> to skip them and consider them to be normal instruction....

I'd wonder how many spots would need to be changed for that though, to check
for is_gimple_call && !gimple_call_internal_p instead of just
is_gimple_call.  In cgraph*, inliner, sra, IPA, whatever else assumes that a
GIMPLE_CALL should have a cgraph_edge associated with it.
The internal functions for the time being should be pretty rare,
right now they are used just for some ARM vectorization stuff (during/after
vectorizations only, so no IPA) and newly for the OpenMP/Cilk+ SIMD loops,
so if it is just about avoiding memory waste because of them, I think it
isn't a big deal.

	Jakub

> On Sun, Sep 15, 2013 at 09:08:00PM +0200, Jan Hubicka wrote:
> > > 2013-09-13  Jakub Jelinek  <jakub@redhat.com>
> > > 
> > > 	* ipa-prop.c (ipa_compute_jump_functions_for_edge): Return early
> > > 	for internal calls.
> > 
> > That seems resonable.  I wonder if we want to consider internal calls to form
> > callgarph edges at all: perhaps we can just modify cgraph builder+verifier
> > to skip them and consider them to be normal instruction....
> 
> I'd wonder how many spots would need to be changed for that though, to check
> for is_gimple_call && !gimple_call_internal_p instead of just
> is_gimple_call.  In cgraph*, inliner, sra, IPA, whatever else assumes that a

We can have something like is_real_gimpl_call_p predicate.  We already special case
is_inexpensive_builtin in ipa-inline.

> GIMPLE_CALL should have a cgraph_edge associated with it.
> The internal functions for the time being should be pretty rare,
> right now they are used just for some ARM vectorization stuff (during/after
> vectorizations only, so no IPA) and newly for the OpenMP/Cilk+ SIMD loops,
> so if it is just about avoiding memory waste because of them, I think it
> isn't a big deal.

I am more concerned about the heuristic special casing function call, such as
num_calls in ipa-inline.c.  None of these are too important at this point, but
perhaps from longer term maintanibility it would be better to not lie to the
compiler and make the callgraph to not contain fake call sites....

Honza
> 
> 	Jakub