Re: [PATCH, PR 43164, PR 43191] Do not scalarize structures with zero-size bit-fields as their last field

[Richard Guenther <rguenther at suse dot de>]

On Thu, 4 Mar 2010, Martin Jambor wrote:

> Hi,
> 
> the patch below fixes both PR 43164 and PR 43191, which are both
> caused by a zero-sized bit-field at the end of a record.  The
> solution is rather crude, I simply disabled total scalarization
> (attempts to turn all scalar components into separate variables so
> that the original aggregate disappears) of aggregates containing such
> things which means - this is the only way of how accesses to such
> fields get created.
> 
> I have tried not doing this and change build_ref_for_offset so that it
> could cope with them but this would have to involve trying (usually in
> vain) to look into sub-records preceeding the one into which one would
> normally look, doing the same thing even in arrays of records while
> being careful not to get out-of bounds and who knows what else.  These
> weird zero-sized bit-fields simply violate too many properties that
> normal fields have and make build_ref_for_offset overly complicated
> and not for too much gain, I assume.  Therefore I have given this up
> and will attempt it again only if I am persuaded it is a good idea.
> 
> Bootstrapped and tested on x86_64-linux. OK for trunk?

Ok.

For 4.6 I plan to re-introduce a simplified MEM_REF tree so
build_ref_for_offset would just build MEM_REF <base, constant-offset>.

Thanks,
Richard.

> Thanks,
> 
> Martin
> 
> 
> 2010-03-03  Martin Jambor  <mjambor@suse.cz>
> 
> 	PR tree-optimization/43164
> 	PR tree-optimization/43191
> 	* tree-sra.c (type_consists_of_records_p): Reject records with
> 	zero-size bit-fields at the end.
> 
> 	* testsuite/gcc.c-torture/compile/pr43164.c: New test.
> 	* testsuite/gcc.c-torture/compile/pr43191.c: Likewise.
> 
> Index: mine/gcc/testsuite/gcc.c-torture/compile/pr43164.c
> ===================================================================
> --- /dev/null
> +++ mine/gcc/testsuite/gcc.c-torture/compile/pr43164.c
> @@ -0,0 +1,16 @@
> +struct S0
> +{
> +  unsigned char f0;
> +  int:0;
> +};
> +
> +struct S1
> +{
> +  struct S0 f0;
> +};
> +
> +struct S1 func_34 (void)
> +{
> +  struct S1 l_221 = { { 1 } };
> +  return l_221;
> +}
> Index: mine/gcc/testsuite/gcc.c-torture/compile/pr43191.c
> ===================================================================
> --- /dev/null
> +++ mine/gcc/testsuite/gcc.c-torture/compile/pr43191.c
> @@ -0,0 +1,46 @@
> +struct S0
> +{
> +};
> +
> +struct S1
> +{
> +  unsigned f0:27;
> +  const unsigned:0;
> +};
> +
> +struct S2
> +{
> +  unsigned f2:1;
> +};
> +
> +unsigned char g_4[1][8][3][1][1][1];
> +unsigned char *g_17;
> +unsigned char **g_16[1][10][7];
> +
> +struct S2 g_35 = {
> +  0
> +};
> +
> +struct S2 *g_34 = &g_35;
> +
> +struct S1 func_86 (unsigned char p_87, struct S2 **p_89)
> +{
> +  struct S1 l_92[6][8][1][1] = {
> +    16143586
> +  }
> +  ;
> +  return l_92[0][0][0][0];
> +}
> +
> +void func_28 (struct S1 p_30, const struct S1 p_32)
> +{
> +}
> +
> +void func_70 (unsigned char p_72)
> +{
> +  unsigned char *const *l_93 = &g_17;
> +  struct S2 **l_94;
> +  unsigned char *const *l_97 = &g_17;
> +  func_28 (func_86 (p_72, 0),
> +           func_86 (p_72, &g_34));
> +}
> Index: mine/gcc/tree-sra.c
> ===================================================================
> --- mine.orig/gcc/tree-sra.c
> +++ mine/gcc/tree-sra.c
> @@ -802,13 +802,15 @@ create_access (tree expr, gimple stmt, b
>  
>  
>  /* Return true iff TYPE is a RECORD_TYPE with fields that are either of gimple
> -   register types or (recursively) records with only these two kinds of
> -   fields.  */
> +   register types or (recursively) records with only these two kinds of fields.
> +   It also returns false if any of these records has a zero-size field as its
> +   last field.  */
>  
>  static bool
>  type_consists_of_records_p (tree type)
>  {
>    tree fld;
> +  bool last_fld_has_zero_size = false;
>  
>    if (TREE_CODE (type) != RECORD_TYPE)
>      return false;
> @@ -821,7 +823,13 @@ type_consists_of_records_p (tree type)
>  	if (!is_gimple_reg_type (ft)
>  	    && !type_consists_of_records_p (ft))
>  	  return false;
> +
> +	last_fld_has_zero_size = tree_low_cst (DECL_SIZE (fld), 1) == 0;
>        }
> +
> +  if (last_fld_has_zero_size)
> +    return false;
> +
>    return true;
>  }
>  
> 
> 

-- 
Richard Guenther <rguenther@suse.de>
Novell / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 - GF: Markus Rex