[PR,57347] Do not create aggregate jump functions for bit-fields

Hi,

I have not intended aggregate jump functions to work with bit-fields
but apparently forgot to include the test to ignore them.  PR 57347
testcase gives a good example why they need to be avoided.  If we ever
decide to optimize for them too (and not just in IPA land), they
should be lowered earlier and jump functions can then be built for the
stores to the representatives.

The following patch disables their generation.  It passes bootstrap
and testing on x8664-linux on trunk, the same for the 4.8 branch is
currently underway.  OK for trunk and for the branch if it passes?

Thanks,

Martin


2013-05-21  Martin Jambor  <mjambor@suse.cz>

	PR middle-end/57347
	* tree.h (contains_bitfld_component_ref_p): Declare.
	* tree-sra.c (contains_bitfld_comp_ref_p): Move...
	* tree.c (contains_bitfld_component_ref_p): ...here.  Adjust its caller.
	* ipa-prop.c (determine_known_aggregate_parts): Check that LHS does
	not access a bit-field.  Assert all final offsets are byte-aligned.

testsuite/
	* gcc.dg/ipa/pr57347.c: New test.

On Wed, May 22, 2013 at 6:23 PM, Martin Jambor <mjambor@suse.cz> wrote:
> Hi,
>
> I have not intended aggregate jump functions to work with bit-fields
> but apparently forgot to include the test to ignore them.  PR 57347
> testcase gives a good example why they need to be avoided.  If we ever
> decide to optimize for them too (and not just in IPA land), they
> should be lowered earlier and jump functions can then be built for the
> stores to the representatives.
>
> The following patch disables their generation.  It passes bootstrap
> and testing on x8664-linux on trunk, the same for the 4.8 branch is
> currently underway.  OK for trunk and for the branch if it passes?

Ok.

Thanks,
Richard.

> Thanks,
>
> Martin
>
>
> 2013-05-21  Martin Jambor  <mjambor@suse.cz>
>
>         PR middle-end/57347
>         * tree.h (contains_bitfld_component_ref_p): Declare.
>         * tree-sra.c (contains_bitfld_comp_ref_p): Move...
>         * tree.c (contains_bitfld_component_ref_p): ...here.  Adjust its caller.
>         * ipa-prop.c (determine_known_aggregate_parts): Check that LHS does
>         not access a bit-field.  Assert all final offsets are byte-aligned.
>
> testsuite/
>         * gcc.dg/ipa/pr57347.c: New test.
>
> Index: src/gcc/ipa-prop.c
> ===================================================================
> --- src.orig/gcc/ipa-prop.c
> +++ src/gcc/ipa-prop.c
> @@ -1327,7 +1327,9 @@ determine_known_aggregate_parts (gimple
>
>        lhs = gimple_assign_lhs (stmt);
>        rhs = gimple_assign_rhs1 (stmt);
> -      if (!is_gimple_reg_type (rhs))
> +      if (!is_gimple_reg_type (rhs)
> +         || TREE_CODE (lhs) == BIT_FIELD_REF
> +         || contains_bitfld_component_ref_p (lhs))
>         break;
>
>        lhs_base = get_ref_base_and_extent (lhs, &lhs_offset, &lhs_size,
> @@ -1418,6 +1420,7 @@ determine_known_aggregate_parts (gimple
>             {
>               struct ipa_agg_jf_item item;
>               item.offset = list->offset - arg_offset;
> +             gcc_assert ((item.offset % BITS_PER_UNIT) == 0);
>               item.value = unshare_expr_without_location (list->constant);
>               jfunc->agg.items->quick_push (item);
>             }
> Index: src/gcc/testsuite/gcc.dg/ipa/pr57347.c
> ===================================================================
> --- /dev/null
> +++ src/gcc/testsuite/gcc.dg/ipa/pr57347.c
> @@ -0,0 +1,27 @@
> +/* { dg-do run } */
> +/* { dg-options "-O3" } */
> +
> +struct S1 { int f0; int f1 : 10; int f2 : 13; };
> +int i;
> +int *j = &i;
> +
> +static void
> +foo (struct S1 s)
> +{
> +  int *p;
> +  int l[88];
> +  int **pp = &p;
> +  *pp = &l[1];
> +  l[0] = 1;
> +  *j = 1 && s.f2;
> +}
> +
> +int
> +main ()
> +{
> +  struct S1 s = { 0, 0, 1 };
> +  foo (s);
> +  if (i != 1)
> +    __builtin_abort ();
> +  return 0;
> +}
> Index: src/gcc/tree-sra.c
> ===================================================================
> --- src.orig/gcc/tree-sra.c
> +++ src/gcc/tree-sra.c
> @@ -2998,23 +2998,6 @@ get_repl_default_def_ssa_name (struct ac
>    return get_or_create_ssa_default_def (cfun, racc->replacement_decl);
>  }
>
> -/* Return true if REF has a COMPONENT_REF with a bit-field field declaration
> -   somewhere in it.  */
> -
> -static inline bool
> -contains_bitfld_comp_ref_p (const_tree ref)
> -{
> -  while (handled_component_p (ref))
> -    {
> -      if (TREE_CODE (ref) == COMPONENT_REF
> -          && DECL_BIT_FIELD (TREE_OPERAND (ref, 1)))
> -        return true;
> -      ref = TREE_OPERAND (ref, 0);
> -    }
> -
> -  return false;
> -}
> -
>  /* Return true if REF has an VIEW_CONVERT_EXPR or a COMPONENT_REF with a
>     bit-field field declaration somewhere in it.  */
>
> @@ -3110,7 +3093,7 @@ sra_modify_assign (gimple *stmt, gimple_
>              ???  This should move to fold_stmt which we simply should
>              call after building a VIEW_CONVERT_EXPR here.  */
>           if (AGGREGATE_TYPE_P (TREE_TYPE (lhs))
> -             && !contains_bitfld_comp_ref_p (lhs))
> +             && !contains_bitfld_component_ref_p (lhs))
>             {
>               lhs = build_ref_for_model (loc, lhs, 0, racc, gsi, false);
>               gimple_assign_set_lhs (*stmt, lhs);
> Index: src/gcc/tree.c
> ===================================================================
> --- src.orig/gcc/tree.c
> +++ src/gcc/tree.c
> @@ -11785,4 +11785,21 @@ warn_deprecated_use (tree node, tree att
>      }
>  }
>
> +/* Return true if REF has a COMPONENT_REF with a bit-field field declaration
> +   somewhere in it.  */
> +
> +bool
> +contains_bitfld_component_ref_p (const_tree ref)
> +{
> +  while (handled_component_p (ref))
> +    {
> +      if (TREE_CODE (ref) == COMPONENT_REF
> +          && DECL_BIT_FIELD (TREE_OPERAND (ref, 1)))
> +        return true;
> +      ref = TREE_OPERAND (ref, 0);
> +    }
> +
> +  return false;
> +}
> +
>  #include "gt-tree.h"
> Index: src/gcc/tree.h
> ===================================================================
> --- src.orig/gcc/tree.h
> +++ src/gcc/tree.h
> @@ -5974,6 +5974,7 @@ extern tree block_ultimate_origin (const
>  extern tree get_binfo_at_offset (tree, HOST_WIDE_INT, tree);
>  extern tree get_ref_base_and_extent (tree, HOST_WIDE_INT *,
>                                      HOST_WIDE_INT *, HOST_WIDE_INT *);
> +extern bool contains_bitfld_component_ref_p (const_tree);
>
>  /* In tree-nested.c */
>  extern tree build_addr (tree, tree);