[Bug tree-optimization/101062] New: wrong code with "-O2 -fno-toplevel-reorder -frename-registers"

[Bug tree-optimization/101062] New: wrong code with "-O2 -fno-toplevel-reorder -frename-registers"

[suochenyao at 163 dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101062

            Bug ID: 101062
           Summary: wrong code with "-O2 -fno-toplevel-reorder
                    -frename-registers"
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: suochenyao at 163 dot com
  Target Milestone: ---

*******************************************************************************
OS and Platform:
CentOS Linux release 7.8.2003 (Core), x86_64 GNU/Linux
*******************************************************************************
Program:
int printf(const char *, ...);
union a {
  signed b : 5;
};
int c;
volatile union a d[7] = {{8}};
short e = 1;
void f() {
  c = 5;
  for (; c; c--) {
    short *g = &e;
    *g = d[6].b = 0;
  }
}
int main() {
  f();
  printf("%d\n", e);
}
*******************************************************************************
gcc version:
$ gcc -v
Using built-in specs.
COLLECT_GCC=/data/bin/gcc-dev/bin/gcc
COLLECT_LTO_WRAPPER=/data/bin/gcc-dev/libexec/gcc/x86_64-pc-linux-gnu/12.0.0/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: ../configure --prefix=/data/bin/gcc-dev --disable-multilib
--enable-languages=c,c++
Thread model: posix
Supported LTO compression algorithms: zlib
gcc version 12.0.0 20210613 (experimental) (GCC)

git version: 681143b9b94d7f1c88a7c34e2250865c31191959
*******************************************************************************
Command Lines:
$ gcc a.c -o a1.out
$ gcc -O2 -fno-toplevel-reorder -frename-registers -Wall -Wextra
-fno-strict-aliasing -fwrapv a.c -o a2.out
$ ./a1.out
0
$ ./a2.out
1

[Bug tree-optimization/101062] [10/11/12 Regression] wrong code with "-O2 -fno-toplevel-reorder -frename-registers"

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101062

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
   Target Milestone|---                         |10.4
                 CC|                            |jakub at gcc dot gnu.org
   Last reconfirmed|                            |2021-06-14
            Summary|wrong code with "-O2        |[10/11/12 Regression] wrong
                   |-fno-toplevel-reorder       |code with "-O2
                   |-frename-registers"         |-fno-toplevel-reorder
                   |                            |-frename-registers"

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Started with r10-3542-g0b92cf305dcf34387a8e2564e55ca8948df3b47a
so likely latent before.

Slightly adjusted testcase:
/* { 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 ()
{
  for (c = 5; c; c--)
    e = d[6].b = 0;
}

int
main ()
{
  foo ();
  if (e != 0)
    __builtin_abort ();
  return 0;
}

[Bug tree-optimization/101062] [10/11/12 Regression] wrong code with "-O2 -fno-toplevel-reorder -frename-registers"

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101062

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Actually, with noinline,noclone,noipa it started already with
r0-102336-g8f0fe813790d58066714c8f38f4916925c83517d

[Bug tree-optimization/101062] [10/11/12 Regression] wrong code with "-O2 -fno-toplevel-reorder -frename-registers"

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101062

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
And
union U { signed b : 5; };
int c;
volatile union U d[7] = {{8}};
short e = 1;

__attribute__((noipa, noinline, noclone)) 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;
}

it started even in between r0-74314-gc245c134da2fdced8608d3e992c969ae22932752
and r0-74353-g5cd88d6857dffe4f10c834c773c300881ec20e32
Seems the problem is the d[6].b store, which incorrectly uses a 64-bit load
from d+24 and then later on stores adjusted value to that.  But d is only 28
bytes long and with -fno-toplevel-reorder the e variable is right after it and
scheduling moves the load from *(long long *)(((char *)&d) + 24) before store
to e and its update after store to e.

[Bug tree-optimization/101062] [10/11/12 Regression] wrong code with "-O2 -fno-toplevel-reorder -frename-registers"

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101062

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ebotcazou at gcc dot gnu.org

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Ah, and there is also a problem that for bitfields in unions we don't create
DECL_BIT_FIELD_REPRESENTATIVE in finish_bitfield_layout and so then
get_bit_range returns { 0, 0 } as bitstart, bitend and so the access isn't
really restricted.

So, wonder, shall we for bitfields in UNION_TYPE in get_bit_range return the
range of the union?  Or is it ok for d[4].b RMW to update also d[5].b next to
it, just not anything beyond end of the variable (so essentially set
bitregion_start/bitregion_end to the start and end of d variable), something
else?
At least for multithreaded apps, even the modification of another element of
the same array seems bad to me.

Does Ada allow bitfields in unions and if yes, what does it want for those?

Changing the testcase with sed -i -e s/union/struct/g fixes the bug...

[Bug tree-optimization/101062] [10/11/12 Regression] wrong code with "-O2 -fno-toplevel-reorder -frename-registers"

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101062

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
--- gcc/expr.c.jj       2021-06-14 12:27:02.000000000 +0200
+++ gcc/expr.c  2021-06-14 22:18:56.852524237 +0200
@@ -5120,15 +5120,18 @@ get_bit_range (poly_uint64_pod *bitstart
               poly_int64_pod *bitpos, tree *offset)
 {
   poly_int64 bitoffset;
-  tree field, repr;

   gcc_assert (TREE_CODE (exp) == COMPONENT_REF);

-  field = TREE_OPERAND (exp, 1);
-  repr = DECL_BIT_FIELD_REPRESENTATIVE (field);
-  /* If we do not have a DECL_BIT_FIELD_REPRESENTATIVE there is no
-     need to limit the range we can access.  */
-  if (!repr)
+  tree field = TREE_OPERAND (exp, 1);
+  tree repr = DECL_BIT_FIELD_REPRESENTATIVE (field);
+  tree type = TREE_TYPE (TREE_OPERAND (exp, 0));
+  /* If we do not have a DECL_BIT_FIELD_REPRESENTATIVE (except for bitfields
+     in unions) there is no need to limit the range we can access.  */
+  if (!repr
+      && (TREE_CODE (type) != UNION_TYPE
+          || !TYPE_SIZE (type)
+         || !tree_fits_poly_uint64_p (TYPE_SIZE (type))))
     {
       *bitstart = *bitend = 0;
       return;
@@ -5153,6 +5156,14 @@ get_bit_range (poly_uint64_pod *bitstart
        }
     }

+  /* For bitfields in unions, return bitsize of the whole union.  */
+  if (!repr)
+    {
+      *bitstart = *bitpos;
+      *bitend = *bitstart + tree_to_poly_uint64 (TYPE_SIZE (type)) - 1;
+      return;
+    }
+
   /* Compute the adjustment to bitpos from the offset of the field
      relative to the representative.  DECL_FIELD_OFFSET of field and
      repr are the same by construction if they are not constants,

fixes it for me.

[Bug tree-optimization/101062] [10/11/12 Regression] wrong code with "-O2 -fno-toplevel-reorder -frename-registers"

[ebotcazou at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101062

--- Comment #6 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> Does Ada allow bitfields in unions and if yes, what does it want for those?

Yes, it does, and I don't think there is any specific need so the default
should be OK like for structures.

[Bug tree-optimization/101062] [10/11/12 Regression] wrong code with "-O2 -fno-toplevel-reorder -frename-registers"

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101062

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Eric Botcazou from comment #6)
> > Does Ada allow bitfields in unions and if yes, what does it want for those?
> 
> Yes, it does, and I don't think there is any specific need so the default
> should be OK like for structures.

I also think since there's at most a single active union member the default
should work.

Now, it looks to me this is rather an issue that the access is larger than
the object and thus a general bug - at least I don't see how it should only
manifest with bitfields in unions?

Note we do

      if (TREE_CODE (to) == COMPONENT_REF
          && DECL_BIT_FIELD_TYPE (TREE_OPERAND (to, 1)))
        get_bit_range (&bitregion_start, &bitregion_end, to, &bitpos, &offset);
      /* The C++ memory model naturally applies to byte-aligned fields.
         However, if we do not have a DECL_BIT_FIELD_TYPE but BITPOS or
         BITSIZE are not byte-aligned, there is no need to limit the range
         we can access.  This can occur with packed structures in Ada.  */
      else if (maybe_gt (bitsize, 0)
               && multiple_p (bitsize, BITS_PER_UNIT)
               && multiple_p (bitpos, BITS_PER_UNIT))
        {
          bitregion_start = bitpos;
          bitregion_end = bitpos + bitsize - 1;
        }

but if we assume that for DECL_BIT_FIELD_TYPE there's a representative
then we miss the else if, so - maybe get_bit_range should return whether
it handled things or the else if part should be done unconditionally
in case bitregion_start/end is not {0,0}?

[Bug tree-optimization/101062] [10/11/12 Regression] wrong code with "-O2 -fno-toplevel-reorder -frename-registers"

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101062

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #7)
> Now, it looks to me this is rather an issue that the access is larger than
> the object and thus a general bug - at least I don't see how it should only
> manifest with bitfields in unions?
> 
> Note we do
> 
>       if (TREE_CODE (to) == COMPONENT_REF
>           && DECL_BIT_FIELD_TYPE (TREE_OPERAND (to, 1)))
>         get_bit_range (&bitregion_start, &bitregion_end, to, &bitpos,
> &offset);
>       /* The C++ memory model naturally applies to byte-aligned fields.
>          However, if we do not have a DECL_BIT_FIELD_TYPE but BITPOS or
>          BITSIZE are not byte-aligned, there is no need to limit the range
>          we can access.  This can occur with packed structures in Ada.  */
>       else if (maybe_gt (bitsize, 0)
>                && multiple_p (bitsize, BITS_PER_UNIT)
>                && multiple_p (bitpos, BITS_PER_UNIT))
>         {
>           bitregion_start = bitpos;
>           bitregion_end = bitpos + bitsize - 1;
>         }
> 
> but if we assume that for DECL_BIT_FIELD_TYPE there's a representative
> then we miss the else if, so - maybe get_bit_range should return whether
> it handled things or the else if part should be done unconditionally
> in case bitregion_start/end is not {0,0}?

This wouldn't help us, bitsize is > 0, but not a multiple of BITS_PER_UNIT in
this case.  Furthermore, even if we add there bitregion_start/end for the base
variable if any as further fallthrough, I think most C/C++ programmers will
expect that with
union U { int a; int b : 5; } u[64];
u[4].b = 1; can be done safely in one thread and
u[5].a = 2; in another one.

My patch fixes that (or another possibility would be to compute the
representative even in UNION_TYPE (no idea about QUAL_UNION_TYPE) - could be as
simple as removing the early out and instead of doing prev = field; in the loop
do if (TREE_CODE (t) != RECORD_TYPE) { finish_bitfield_representative (repr,
field); repr = NULL_TREE; } else prev = field; and in
finish_bitfield_representative override nextf to NULL_TREE).

Improving expand_assignment can be done too, sure, but independently to this.

[Bug middle-end/101062] [10/11/12 Regression] wrong code with "-O2 -fno-toplevel-reorder -frename-registers"

[rguenther at suse dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101062

--- Comment #9 from rguenther at suse dot de <rguenther at suse dot de> ---
On Tue, 15 Jun 2021, jakub at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101062
> 
> --- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
> (In reply to Richard Biener from comment #7)
> > Now, it looks to me this is rather an issue that the access is larger than
> > the object and thus a general bug - at least I don't see how it should only
> > manifest with bitfields in unions?
> > 
> > Note we do
> > 
> >       if (TREE_CODE (to) == COMPONENT_REF
> >           && DECL_BIT_FIELD_TYPE (TREE_OPERAND (to, 1)))
> >         get_bit_range (&bitregion_start, &bitregion_end, to, &bitpos,
> > &offset);
> >       /* The C++ memory model naturally applies to byte-aligned fields.
> >          However, if we do not have a DECL_BIT_FIELD_TYPE but BITPOS or
> >          BITSIZE are not byte-aligned, there is no need to limit the range
> >          we can access.  This can occur with packed structures in Ada.  */
> >       else if (maybe_gt (bitsize, 0)
> >                && multiple_p (bitsize, BITS_PER_UNIT)
> >                && multiple_p (bitpos, BITS_PER_UNIT))
> >         {
> >           bitregion_start = bitpos;
> >           bitregion_end = bitpos + bitsize - 1;
> >         }
> > 
> > but if we assume that for DECL_BIT_FIELD_TYPE there's a representative
> > then we miss the else if, so - maybe get_bit_range should return whether
> > it handled things or the else if part should be done unconditionally
> > in case bitregion_start/end is not {0,0}?
> 
> This wouldn't help us, bitsize is > 0, but not a multiple of BITS_PER_UNIT in
> this case.  Furthermore, even if we add there bitregion_start/end for the base
> variable if any as further fallthrough, I think most C/C++ programmers will
> expect that with
> union U { int a; int b : 5; } u[64];
> u[4].b = 1; can be done safely in one thread and
> u[5].a = 2; in another one.
> 
> My patch fixes that (or another possibility would be to compute the
> representative even in UNION_TYPE (no idea about QUAL_UNION_TYPE) - could be as
> simple as removing the early out and instead of doing prev = field; in the loop
> do if (TREE_CODE (t) != RECORD_TYPE) { finish_bitfield_representative (repr,
> field); repr = NULL_TREE; } else prev = field; and in
> finish_bitfield_representative override nextf to NULL_TREE).

Yes, as said - the caller of get_bit_range seems to expect it to always
handle cases that do not run into the byte-align case to make sure to
not cross to next objects.  I don't remember why I didn't handle
UNION_TYPE (I guess because it was about bitfield groups, not single
bitfields), so maybe we should indeed handle them.

There are also other callers of get_bit_range it seems, even on
!DECL_BIT_FIELD_TYPE fields.

So let's try handling [QUAL_]UNION_TYPE in finish_bitfield_layout?

[Bug middle-end/101062] [10/11/12 Regression] wrong code with "-O2 -fno-toplevel-reorder -frename-registers"

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101062

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |jakub at gcc dot gnu.org

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 51023
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51023&action=edit
gcc12-pr101062.patch

Untested fix for this in stor-layout.c.

[Bug middle-end/101062] [10/11/12 Regression] wrong code with "-O2 -fno-toplevel-reorder -frename-registers"

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101062

--- Comment #11 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #10)
> Created attachment 51023 [details]
> gcc12-pr101062.patch
> 
> Untested fix for this in stor-layout.c.

LGTM

[Bug middle-end/101062] [10/11/12 Regression] wrong code with "-O2 -fno-toplevel-reorder -frename-registers"

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101062

--- Comment #12 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:b4b50bf2864e09f028a39a3f460222632c4d7348

commit r12-1527-gb4b50bf2864e09f028a39a3f460222632c4d7348
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Jun 16 12:17:55 2021 +0200

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

    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.

    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.

[Bug middle-end/101062] [10/11/12 Regression] wrong code with "-O2 -fno-toplevel-reorder -frename-registers"

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101062

--- Comment #13 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I have tried to backport this patch to the 11 branch, but unlike the trunk, it
doesn't build there, with
atree.ads:3844:34: size for "Node_Record" too small
make[3]: *** [../../gcc/ada/gcc-interface/Make-lang.in:139: ada/errout.o] Error
1
(and many further occurrences of the same error).
Seems the stor-layout.c changes trigger during ada/errout.o compilation just
once, on QUAL_UNION_TYPE atree.ads:3844:34: size for "Node_Record" too small
make[3]: *** [../../gcc/ada/gcc-interface/Make-lang.in:139: ada/errout.o] Error
1
which has byte size 28 and bitfield FIELD_DECL S0 with the same byte size (224
bits) and another bitfield FIELD_DECL O with the same size.
The code then adds two bit field representatives with the same size.

Eric, any idea what's going on?  Shall I readd if (TREE_CODE (t) ==
QUAL_UNION_TYPE) return; at the start function and thus ignore QUAL_UNION_TYPE
and handle only UNION_TYPEs, or should the bitfield representatives in
QUAL_UNION_TYPE get some DECL_QUALIFIER (either one that always evaluates to
false, or the same as corresponding bitfield), something else?
Wonder what is different that it works on the trunk...

[Bug middle-end/101062] [10/11/12 Regression] wrong code with "-O2 -fno-toplevel-reorder -frename-registers"

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101062

--- Comment #14 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Sorry for bad paste, on QUAL_UNION_TYPE
atree__atree_private_part__node_record___is_extension___XVN

[Bug middle-end/101062] [10/11/12 Regression] wrong code with "-O2 -fno-toplevel-reorder -frename-registers"

[ebotcazou at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101062

--- Comment #15 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> Eric, any idea what's going on?  Shall I readd if (TREE_CODE (t) ==
> QUAL_UNION_TYPE) return; at the start function and thus ignore
> QUAL_UNION_TYPE and handle only UNION_TYPEs, or should the bitfield
> representatives in QUAL_UNION_TYPE get some DECL_QUALIFIER (either one that
> always evaluates to false, or the same as corresponding bitfield), something
> else?

Let's try the former, i.e. disable the new code for QUAL_UNION_TYPE.

[Bug middle-end/101062] [10/11/12 Regression] wrong code with "-O2 -fno-toplevel-reorder -frename-registers"

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101062

--- Comment #16 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:76e990fd211cbb20bf74ce074eb8b2d7b096d3b7

commit r12-1640-g76e990fd211cbb20bf74ce074eb8b2d7b096d3b7
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri Jun 18 11:20:40 2021 +0200

    stor-layout: Don't create DECL_BIT_FIELD_REPRESENTATIVE for QUAL_UNION_TYPE
[PR101062]

    > 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.

    While the patch passed bootstrap/regtest on the trunk, when trying to
    backport it to 11 branch the bootstrap failed with
    atree.ads:3844:34: size for "Node_Record" too small
    errors.  Turns out the error is not about size being too small, but
actually
    about size being non-constant, and comes from:
     /* In a FIELD_DECL of a RECORD_TYPE, this is a pointer to the storage
        representative FIELD_DECL.  */
     #define DECL_BIT_FIELD_REPRESENTATIVE(NODE) \
       (FIELD_DECL_CHECK (NODE)->field_decl.qualifier)

     /* For a FIELD_DECL in a QUAL_UNION_TYPE, records the expression, which
        if nonzero, indicates that the field occupies the type.  */
      #define DECL_QUALIFIER(NODE) (FIELD_DECL_CHECK
(NODE)->field_decl.qualifier)
    so by setting up DECL_BIT_FIELD_REPRESENTATIVE in QUAL_UNION_TYPE we
    actually set or modify DECL_QUALIFIER and then construct size as COND_EXPRs
    with those bit field representatives (e.g. with array type) as conditions
    which doesn't fold into constant.

    The following patch fixes it by not creating DECL_BIT_FIELD_REPRESENTATIVEs
    for QUAL_UNION_TYPE as there is nowhere to store them,

    Shall we change tree.h to document that DECL_BIT_FIELD_REPRESENTATIVE
    is valid also on UNION_TYPE?
    I see:
    tree-ssa-alias.c-  if (TREE_CODE (type1) == RECORD_TYPE
    tree-ssa-alias.c:      && DECL_BIT_FIELD_REPRESENTATIVE (field1))
    tree-ssa-alias.c:    field1 = DECL_BIT_FIELD_REPRESENTATIVE (field1);
    tree-ssa-alias.c-  if (TREE_CODE (type2) == RECORD_TYPE
    tree-ssa-alias.c:      && DECL_BIT_FIELD_REPRESENTATIVE (field2))
    tree-ssa-alias.c:    field2 = DECL_BIT_FIELD_REPRESENTATIVE (field2);
    Shall we change that to || == UNION_TYPE or do we assume all fields
    are overlapping in a UNION_TYPE already?
    At other spots (asan, ubsan, expr.c) it is unclear what will happen
    if they see a QUAL_UNION_TYPE with a DECL_QUALIFIER (or does the Ada FE
    lower that somehow)?

    2021-06-18  Jakub Jelinek  <jakub@redhat.com>

            PR middle-end/101062
            * stor-layout.c (finish_bitfield_layout): Don't add bitfield
            representatives in QUAL_UNION_TYPE.

[Bug middle-end/101062] [10/11/12 Regression] wrong code with "-O2 -fno-toplevel-reorder -frename-registers"

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101062

--- Comment #17 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:c63b440cda7449fb6079831db3911ab3dde7c9f0

commit r11-8614-gc63b440cda7449fb6079831db3911ab3dde7c9f0
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Jun 16 12:17:55 2021 +0200

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

    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.

    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.

    (cherry picked from commit b4b50bf2864e09f028a39a3f460222632c4d7348)

[Bug middle-end/101062] [10/11/12 Regression] wrong code with "-O2 -fno-toplevel-reorder -frename-registers"

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101062

--- Comment #18 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:3587c2c241eda0f3ab54ea60d46e9caf12d69b5a

commit r11-8615-g3587c2c241eda0f3ab54ea60d46e9caf12d69b5a
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri Jun 18 11:20:40 2021 +0200

    stor-layout: Don't create DECL_BIT_FIELD_REPRESENTATIVE for QUAL_UNION_TYPE
[PR101062]

    > 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.

    While the patch passed bootstrap/regtest on the trunk, when trying to
    backport it to 11 branch the bootstrap failed with
    atree.ads:3844:34: size for "Node_Record" too small
    errors.  Turns out the error is not about size being too small, but
actually
    about size being non-constant, and comes from:
     /* In a FIELD_DECL of a RECORD_TYPE, this is a pointer to the storage
        representative FIELD_DECL.  */
     #define DECL_BIT_FIELD_REPRESENTATIVE(NODE) \
       (FIELD_DECL_CHECK (NODE)->field_decl.qualifier)

     /* For a FIELD_DECL in a QUAL_UNION_TYPE, records the expression, which
        if nonzero, indicates that the field occupies the type.  */
      #define DECL_QUALIFIER(NODE) (FIELD_DECL_CHECK
(NODE)->field_decl.qualifier)
    so by setting up DECL_BIT_FIELD_REPRESENTATIVE in QUAL_UNION_TYPE we
    actually set or modify DECL_QUALIFIER and then construct size as COND_EXPRs
    with those bit field representatives (e.g. with array type) as conditions
    which doesn't fold into constant.

    The following patch fixes it by not creating DECL_BIT_FIELD_REPRESENTATIVEs
    for QUAL_UNION_TYPE as there is nowhere to store them,

    Shall we change tree.h to document that DECL_BIT_FIELD_REPRESENTATIVE
    is valid also on UNION_TYPE?
    I see:
    tree-ssa-alias.c-  if (TREE_CODE (type1) == RECORD_TYPE
    tree-ssa-alias.c:      && DECL_BIT_FIELD_REPRESENTATIVE (field1))
    tree-ssa-alias.c:    field1 = DECL_BIT_FIELD_REPRESENTATIVE (field1);
    tree-ssa-alias.c-  if (TREE_CODE (type2) == RECORD_TYPE
    tree-ssa-alias.c:      && DECL_BIT_FIELD_REPRESENTATIVE (field2))
    tree-ssa-alias.c:    field2 = DECL_BIT_FIELD_REPRESENTATIVE (field2);
    Shall we change that to || == UNION_TYPE or do we assume all fields
    are overlapping in a UNION_TYPE already?
    At other spots (asan, ubsan, expr.c) it is unclear what will happen
    if they see a QUAL_UNION_TYPE with a DECL_QUALIFIER (or does the Ada FE
    lower that somehow)?

    2021-06-18  Jakub Jelinek  <jakub@redhat.com>

            PR middle-end/101062
            * stor-layout.c (finish_bitfield_layout): Don't add bitfield
            representatives in QUAL_UNION_TYPE.

[Bug middle-end/101062] [10 Regression] wrong code with "-O2 -fno-toplevel-reorder -frename-registers"

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101062

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[10/11/12 Regression] wrong |[10 Regression] wrong code
                   |code with "-O2              |with "-O2
                   |-fno-toplevel-reorder       |-fno-toplevel-reorder
                   |-frename-registers"         |-frename-registers"

--- Comment #19 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed for 11.2 and trunk so far.

[Bug middle-end/101062] [10 Regression] wrong code with "-O2 -fno-toplevel-reorder -frename-registers"

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101062

--- Comment #20 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:0a278a8e2a95abe2bdad50ffd820b31363758a78

commit r10-10622-g0a278a8e2a95abe2bdad50ffd820b31363758a78
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Jun 16 12:17:55 2021 +0200

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

    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.

    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.

    (cherry picked from commit b4b50bf2864e09f028a39a3f460222632c4d7348)

[Bug middle-end/101062] [10 Regression] wrong code with "-O2 -fno-toplevel-reorder -frename-registers"

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101062

--- Comment #21 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:3df2f5828752a91343d01defb215de326b5ddb4c

commit r10-10623-g3df2f5828752a91343d01defb215de326b5ddb4c
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri Jun 18 11:20:40 2021 +0200

    stor-layout: Don't create DECL_BIT_FIELD_REPRESENTATIVE for QUAL_UNION_TYPE
[PR101062]

    > 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.

    While the patch passed bootstrap/regtest on the trunk, when trying to
    backport it to 11 branch the bootstrap failed with
    atree.ads:3844:34: size for "Node_Record" too small
    errors.  Turns out the error is not about size being too small, but
actually
    about size being non-constant, and comes from:
     /* In a FIELD_DECL of a RECORD_TYPE, this is a pointer to the storage
        representative FIELD_DECL.  */
     #define DECL_BIT_FIELD_REPRESENTATIVE(NODE) \
       (FIELD_DECL_CHECK (NODE)->field_decl.qualifier)

     /* For a FIELD_DECL in a QUAL_UNION_TYPE, records the expression, which
        if nonzero, indicates that the field occupies the type.  */
      #define DECL_QUALIFIER(NODE) (FIELD_DECL_CHECK
(NODE)->field_decl.qualifier)
    so by setting up DECL_BIT_FIELD_REPRESENTATIVE in QUAL_UNION_TYPE we
    actually set or modify DECL_QUALIFIER and then construct size as COND_EXPRs
    with those bit field representatives (e.g. with array type) as conditions
    which doesn't fold into constant.

    The following patch fixes it by not creating DECL_BIT_FIELD_REPRESENTATIVEs
    for QUAL_UNION_TYPE as there is nowhere to store them,

    Shall we change tree.h to document that DECL_BIT_FIELD_REPRESENTATIVE
    is valid also on UNION_TYPE?
    I see:
    tree-ssa-alias.c-  if (TREE_CODE (type1) == RECORD_TYPE
    tree-ssa-alias.c:      && DECL_BIT_FIELD_REPRESENTATIVE (field1))
    tree-ssa-alias.c:    field1 = DECL_BIT_FIELD_REPRESENTATIVE (field1);
    tree-ssa-alias.c-  if (TREE_CODE (type2) == RECORD_TYPE
    tree-ssa-alias.c:      && DECL_BIT_FIELD_REPRESENTATIVE (field2))
    tree-ssa-alias.c:    field2 = DECL_BIT_FIELD_REPRESENTATIVE (field2);
    Shall we change that to || == UNION_TYPE or do we assume all fields
    are overlapping in a UNION_TYPE already?
    At other spots (asan, ubsan, expr.c) it is unclear what will happen
    if they see a QUAL_UNION_TYPE with a DECL_QUALIFIER (or does the Ada FE
    lower that somehow)?

    2021-06-18  Jakub Jelinek  <jakub@redhat.com>

            PR middle-end/101062
            * stor-layout.c (finish_bitfield_layout): Don't add bitfield
            representatives in QUAL_UNION_TYPE.

    (cherry picked from commit 3587c2c241eda0f3ab54ea60d46e9caf12d69b5a)

[Bug middle-end/101062] [10 Regression] wrong code with "-O2 -fno-toplevel-reorder -frename-registers"

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101062

--- Comment #22 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-9 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:d0b1d8d3b85bb4466fae20e72a8fa35dd7277229

commit r9-10079-gd0b1d8d3b85bb4466fae20e72a8fa35dd7277229
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Jun 16 12:17:55 2021 +0200

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

    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.

    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.

    (cherry picked from commit b4b50bf2864e09f028a39a3f460222632c4d7348)

[Bug middle-end/101062] [10 Regression] wrong code with "-O2 -fno-toplevel-reorder -frename-registers"

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101062

--- Comment #23 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-9 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:80f194b25f573b409ada5761778fb6dfb3cf85e9

commit r9-10080-g80f194b25f573b409ada5761778fb6dfb3cf85e9
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri Jun 18 11:20:40 2021 +0200

    stor-layout: Don't create DECL_BIT_FIELD_REPRESENTATIVE for QUAL_UNION_TYPE
[PR101062]

    > 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.

    While the patch passed bootstrap/regtest on the trunk, when trying to
    backport it to 11 branch the bootstrap failed with
    atree.ads:3844:34: size for "Node_Record" too small
    errors.  Turns out the error is not about size being too small, but
actually
    about size being non-constant, and comes from:
     /* In a FIELD_DECL of a RECORD_TYPE, this is a pointer to the storage
        representative FIELD_DECL.  */
     #define DECL_BIT_FIELD_REPRESENTATIVE(NODE) \
       (FIELD_DECL_CHECK (NODE)->field_decl.qualifier)

     /* For a FIELD_DECL in a QUAL_UNION_TYPE, records the expression, which
        if nonzero, indicates that the field occupies the type.  */
      #define DECL_QUALIFIER(NODE) (FIELD_DECL_CHECK
(NODE)->field_decl.qualifier)
    so by setting up DECL_BIT_FIELD_REPRESENTATIVE in QUAL_UNION_TYPE we
    actually set or modify DECL_QUALIFIER and then construct size as COND_EXPRs
    with those bit field representatives (e.g. with array type) as conditions
    which doesn't fold into constant.

    The following patch fixes it by not creating DECL_BIT_FIELD_REPRESENTATIVEs
    for QUAL_UNION_TYPE as there is nowhere to store them,

    Shall we change tree.h to document that DECL_BIT_FIELD_REPRESENTATIVE
    is valid also on UNION_TYPE?
    I see:
    tree-ssa-alias.c-  if (TREE_CODE (type1) == RECORD_TYPE
    tree-ssa-alias.c:      && DECL_BIT_FIELD_REPRESENTATIVE (field1))
    tree-ssa-alias.c:    field1 = DECL_BIT_FIELD_REPRESENTATIVE (field1);
    tree-ssa-alias.c-  if (TREE_CODE (type2) == RECORD_TYPE
    tree-ssa-alias.c:      && DECL_BIT_FIELD_REPRESENTATIVE (field2))
    tree-ssa-alias.c:    field2 = DECL_BIT_FIELD_REPRESENTATIVE (field2);
    Shall we change that to || == UNION_TYPE or do we assume all fields
    are overlapping in a UNION_TYPE already?
    At other spots (asan, ubsan, expr.c) it is unclear what will happen
    if they see a QUAL_UNION_TYPE with a DECL_QUALIFIER (or does the Ada FE
    lower that somehow)?

    2021-06-18  Jakub Jelinek  <jakub@redhat.com>

            PR middle-end/101062
            * stor-layout.c (finish_bitfield_layout): Don't add bitfield
            representatives in QUAL_UNION_TYPE.

    (cherry picked from commit 3587c2c241eda0f3ab54ea60d46e9caf12d69b5a)

[Bug middle-end/101062] [10 Regression] wrong code with "-O2 -fno-toplevel-reorder -frename-registers"

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101062

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #24 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed.