Fix ipcp_transform_function (PR tree-optimization/58984)

Hi!

This patch fixes PR58984, where determine_known_aggregate_parts
notices that o.f0 = 1 store (because of ESRA) and p.f0 = 1
store (because the field isn't a bitfield) are known constants
at offset 0 of the first parameter, but doesn't record the access
of that size (8 bits) and happily replaces BIT_FIELD_REF <p, 32, 0>
in the called function with 1 (which is wrong, because the f1 bitfield
also contains 1).  As the size isn't recorded explicitly, this patch
looks at TYPE_SIZE of the constants we've recorded, if the IL was correct
then the store to it should be a useless type conversion and thus have the
same number of bits.

Bootstrapped/regtested on x86_64-linux and i686-linux (together with a short
hack to record how many times this has changed compiler's behavior and the
only case during bootstrap/regtest was on this testcase on x86_64-linux
64-bit).

Ok for trunk/4.8?

2013-11-04  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/58984
	* ipa-prop.c (ipa_load_from_parm_agg_1): Add SIZE_P argument,
	set *SIZE_P if non-NULL on success.
	(ipa_load_from_parm_agg, ipa_analyze_indirect_call_uses): Adjust
	callers.
	(ipcp_transform_function): Likewise.  Punt if size of access
	is different from TYPE_SIZE on v->value's type.

	* gcc.c-torture/execute/pr58984.c: New test.


	Jakub

On Mon, 4 Nov 2013, Jakub Jelinek wrote:

> Hi!
> 
> This patch fixes PR58984, where determine_known_aggregate_parts
> notices that o.f0 = 1 store (because of ESRA) and p.f0 = 1
> store (because the field isn't a bitfield) are known constants
> at offset 0 of the first parameter, but doesn't record the access
> of that size (8 bits) and happily replaces BIT_FIELD_REF <p, 32, 0>
> in the called function with 1 (which is wrong, because the f1 bitfield
> also contains 1).  As the size isn't recorded explicitly, this patch
> looks at TYPE_SIZE of the constants we've recorded, if the IL was correct
> then the store to it should be a useless type conversion and thus have the
> same number of bits.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux (together with a short
> hack to record how many times this has changed compiler's behavior and the
> only case during bootstrap/regtest was on this testcase on x86_64-linux
> 64-bit).
> 
> Ok for trunk/4.8?

Looks ok to me.

Thanks,
Richard.

> 2013-11-04  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR tree-optimization/58984
> 	* ipa-prop.c (ipa_load_from_parm_agg_1): Add SIZE_P argument,
> 	set *SIZE_P if non-NULL on success.
> 	(ipa_load_from_parm_agg, ipa_analyze_indirect_call_uses): Adjust
> 	callers.
> 	(ipcp_transform_function): Likewise.  Punt if size of access
> 	is different from TYPE_SIZE on v->value's type.
> 
> 	* gcc.c-torture/execute/pr58984.c: New test.
> 
> --- gcc/ipa-prop.c.jj	2013-11-01 12:41:34.000000000 +0100
> +++ gcc/ipa-prop.c	2013-11-04 15:47:36.373955817 +0100
> @@ -852,7 +852,7 @@ static bool
>  ipa_load_from_parm_agg_1 (vec<ipa_param_descriptor_t> descriptors,
>  			  struct param_analysis_info *parms_ainfo, gimple stmt,
>  			  tree op, int *index_p, HOST_WIDE_INT *offset_p,
> -			  bool *by_ref_p)
> +			  HOST_WIDE_INT *size_p, bool *by_ref_p)
>  {
>    int index;
>    HOST_WIDE_INT size, max_size;
> @@ -870,6 +870,8 @@ ipa_load_from_parm_agg_1 (vec<ipa_param_
>  	{
>  	  *index_p = index;
>  	  *by_ref_p = false;
> +	  if (size_p)
> +	    *size_p = size;
>  	  return true;
>  	}
>        return false;
> @@ -912,6 +914,8 @@ ipa_load_from_parm_agg_1 (vec<ipa_param_
>      {
>        *index_p = index;
>        *by_ref_p = true;
> +      if (size_p)
> +	*size_p = size;
>        return true;
>      }
>    return false;
> @@ -926,7 +930,7 @@ ipa_load_from_parm_agg (struct ipa_node_
>  			bool *by_ref_p)
>  {
>    return ipa_load_from_parm_agg_1 (info->descriptors, NULL, stmt, op, index_p,
> -				   offset_p, by_ref_p);
> +				   offset_p, NULL, by_ref_p);
>  }
>  
>  /* Given that an actual argument is an SSA_NAME (given in NAME) and is a result
> @@ -1826,7 +1830,7 @@ ipa_analyze_indirect_call_uses (struct c
>    if (gimple_assign_single_p (def)
>        && ipa_load_from_parm_agg_1 (info->descriptors, parms_ainfo, def,
>  				   gimple_assign_rhs1 (def), &index, &offset,
> -				   &by_ref))
> +				   NULL, &by_ref))
>      {
>        struct cgraph_edge *cs = ipa_note_param_call (node, index, call);
>        cs->indirect_info->offset = offset;
> @@ -4567,7 +4571,7 @@ ipcp_transform_function (struct cgraph_n
>  	struct ipa_agg_replacement_value *v;
>  	gimple stmt = gsi_stmt (gsi);
>  	tree rhs, val, t;
> -	HOST_WIDE_INT offset;
> +	HOST_WIDE_INT offset, size;
>  	int index;
>  	bool by_ref, vce;
>  
> @@ -4594,13 +4598,15 @@ ipcp_transform_function (struct cgraph_n
>  	  continue;
>  
>  	if (!ipa_load_from_parm_agg_1 (descriptors, parms_ainfo, stmt,
> -				       rhs, &index, &offset, &by_ref))
> +				       rhs, &index, &offset, &size, &by_ref))
>  	  continue;
>  	for (v = aggval; v; v = v->next)
>  	  if (v->index == index
>  	      && v->offset == offset)
>  	    break;
> -	if (!v || v->by_ref != by_ref)
> +	if (!v
> +	    || v->by_ref != by_ref
> +	    || tree_low_cst (TYPE_SIZE (TREE_TYPE (v->value)), 0) != size)
>  	  continue;
>  
>  	gcc_checking_assert (is_gimple_ip_invariant (v->value));
> --- gcc/testsuite/gcc.c-torture/execute/pr58984.c.jj	2013-11-04 15:49:48.657239937 +0100
> +++ gcc/testsuite/gcc.c-torture/execute/pr58984.c	2013-11-04 16:00:44.876785998 +0100
> @@ -0,0 +1,57 @@
> +/* PR tree-optimization/58984 */
> +
> +struct S { int f0 : 8; int : 6; int f1 : 5; };
> +struct T { char f0; int : 6; int f1 : 5; };
> +
> +int a, *c = &a, e, n, b, m;
> +
> +static int
> +foo (struct S p)
> +{
> +  const unsigned short *f[36];
> +  for (; e < 2; e++)
> +    {
> +      const unsigned short **i = &f[0];
> +      *c ^= 1;
> +      if (p.f1)
> +	{
> +	  *i = 0;
> +	  return b;
> +	}
> +    }
> +  return 0;
> +}
> +
> +static int
> +bar (struct T p)
> +{
> +  const unsigned short *f[36];
> +  for (; e < 2; e++)
> +    {
> +      const unsigned short **i = &f[0];
> +      *c ^= 1;
> +      if (p.f1)
> +	{
> +	  *i = 0;
> +	  return b;
> +	}
> +    }
> +  return 0;
> +}
> +
> +int
> +main ()
> +{
> +  struct S o = { 1, 1 };
> +  foo (o);
> +  m = n || o.f0;
> +  if (a != 1)
> +    __builtin_abort ();
> +  e = 0;
> +  struct T p = { 1, 1 };
> +  bar (p);
> +  m |= n || p.f0;
> +  if (a != 0)
> +    __builtin_abort ();
> +  return 0;
> +}
> 
> 	Jakub
> 
>