| Message ID | 20210616074517.GW7746@tucnak |
|---|---|
| State | New |
| Headers | show |
| Series | stor-layout: Create DECL_BIT_FIELD_REPRESENTATIVE even for bitfields in unions [PR101062] | expand |
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 > >
--- 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; +}