Re: [PATCH] stor-layout: Create DECL_BIT_FIELD_REPRESENTATIVE even for bitfields in unions [PR101062]

[Richard Biener <rguenther@suse.de>]

On Wed, 16 Jun 2021, Jakub Jelinek wrote:

> Hi!
> 
> The following testcase is miscompiled on x86_64-linux, the bitfield store
> is implemented as a RMW 64-bit operation at d+24 when the d variable has
> size of only 28 bytes and scheduling moves in between the R and W part
> a store to a different variable that happens to be right after the d
> variable.
> 
> The reason for this is that we weren't creating
> DECL_BIT_FIELD_REPRESENTATIVEs for bitfields in unions.
> 
> The following patch does create them, but treats all such bitfields as if
> they were in a structure where the particular bitfield is the only field.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Thanks,
Richard.

> 2021-06-16  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR middle-end/101062
> 	* stor-layout.c (finish_bitfield_representative): For fields in unions
> 	assume nextf is always NULL.
> 	(finish_bitfield_layout): Compute bit field representatives also in
> 	unions, but handle it as if each bitfield was the only field in the
> 	aggregate.
> 
> 	* gcc.dg/pr101062.c: New test.
> 
> --- gcc/stor-layout.c.jj	2021-03-30 18:11:52.537092233 +0200
> +++ gcc/stor-layout.c	2021-06-15 10:58:59.244353965 +0200
> @@ -2072,9 +2072,14 @@ finish_bitfield_representative (tree rep
>    bitsize = (bitsize + BITS_PER_UNIT - 1) & ~(BITS_PER_UNIT - 1);
>  
>    /* Now nothing tells us how to pad out bitsize ...  */
> -  nextf = DECL_CHAIN (field);
> -  while (nextf && TREE_CODE (nextf) != FIELD_DECL)
> -    nextf = DECL_CHAIN (nextf);
> +  if (TREE_CODE (DECL_CONTEXT (field)) == RECORD_TYPE)
> +    {
> +      nextf = DECL_CHAIN (field);
> +      while (nextf && TREE_CODE (nextf) != FIELD_DECL)
> +	nextf = DECL_CHAIN (nextf);
> +    }
> +  else
> +    nextf = NULL_TREE;
>    if (nextf)
>      {
>        tree maxsize;
> @@ -2167,13 +2172,6 @@ finish_bitfield_layout (tree t)
>    tree field, prev;
>    tree repr = NULL_TREE;
>  
> -  /* Unions would be special, for the ease of type-punning optimizations
> -     we could use the underlying type as hint for the representative
> -     if the bitfield would fit and the representative would not exceed
> -     the union in size.  */
> -  if (TREE_CODE (t) != RECORD_TYPE)
> -    return;
> -
>    for (prev = NULL_TREE, field = TYPE_FIELDS (t);
>         field; field = DECL_CHAIN (field))
>      {
> @@ -2233,7 +2231,13 @@ finish_bitfield_layout (tree t)
>        if (repr)
>  	DECL_BIT_FIELD_REPRESENTATIVE (field) = repr;
>  
> -      prev = field;
> +      if (TREE_CODE (t) == RECORD_TYPE)
> +	prev = field;
> +      else if (repr)
> +	{
> +	  finish_bitfield_representative (repr, field);
> +	  repr = NULL_TREE;
> +	}
>      }
>  
>    if (repr)
> --- gcc/testsuite/gcc.dg/pr101062.c.jj	2021-06-15 10:42:58.642919880 +0200
> +++ gcc/testsuite/gcc.dg/pr101062.c	2021-06-15 10:42:40.897171191 +0200
> @@ -0,0 +1,29 @@
> +/* PR middle-end/101062 */
> +/* { dg-do run } */
> +/* { dg-options "-O2 -fno-toplevel-reorder -frename-registers" } */
> +
> +union U { signed b : 5; };
> +int c;
> +volatile union U d[7] = { { 8 } };
> +short e = 1;
> +
> +__attribute__((noipa)) void
> +foo ()
> +{
> +  d[6].b = 0;
> +  d[6].b = 0;
> +  d[6].b = 0;
> +  d[6].b = 0;
> +  d[6].b = 0;
> +  e = 0;
> +  c = 0;
> +}
> +
> +int
> +main ()
> +{
> +  foo ();
> +  if (e != 0)
> +    __builtin_abort ();
> +  return 0;
> +}
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)