question on alignment of anonymous unions inside structs

question on alignment of anonymous unions inside structs

[Henrik Holst]

[-- Attachment #1: Type: text/plain, Size: 2078 bytes --]

Hi,

 I understand that unions inside structs have to be aligned in case they
are referenced by a pointer or someone creates an array of them but when
the union is completely anonymous and thus none of this is possible,
shouldn't the alignment rules be relaxed?

Case in point, here I'm using pahole to show the layout and padding of the
struct:

struct cb3_item {
        union {
                struct {
                        void *     ptr1;                 /*     0     8 */
                        void *     ptr2;                 /*     8     8 */
                };                                       /*     0    16 */
                uint8_t            raw[60];              /*     0    60 */
        };                                               /*     0    64 */
        /* --- cacheline 1 boundary (64 bytes) --- */
        unsigned int               _avail;               /*    64     4 */

        /* size: 72, cachelines: 2, members: 2 */
        /* padding: 4 */
        /* last cacheline: 8 bytes */
};

So my issue here is that IMHO since the union inside the struct is
anonymous there should be no need for the 4 byte padding between the union
and the _avail member. The _avail being a 32-bit integer is perfectly fine
on a 4 byte alignment so fits perfectly at position 60 which would make the
full struct 64 bytes in size and thus perfectly aligned for 64-bit
alignment.

But instead GCC (v13.2.0 in this case) aligns the union in case I would
make a pointer to it or an array of them but this cannot be done when
defined the way it is (I mean if I had done it as "union { } *p;" or
similar then I would fully understand that gcc would align it this way).

And yes I know that I can "fix" this with __attribute__((packed,aligned(1))
but that is a very broad hammer here since now one have to perform manual
alignment of each union member since alignment inside the union is ok here
but the issue is the alignment of the union as a whole.

Unless ofc if there are some other nifty solution to this that is not as
broad as using forced alignment?

/HH

Re: question on alignment of anonymous unions inside structs

[David Brown]

On 07/05/2024 02:06, Henrik Holst via Gcc-help wrote:
> Hi,
> 
>   I understand that unions inside structs have to be aligned in case they
> are referenced by a pointer or someone creates an array of them but when
> the union is completely anonymous and thus none of this is possible,
> shouldn't the alignment rules be relaxed?
> 

Alignment rules are decided by the ABI for a given platform, along with 
the C standards.  They have to be consistent, so that you can have the 
same declaration in different bits of code, perhaps compiled with 
different tools, and the layout is the same.

So the alignment values, such as on your target, "void *" pointers have 
an alignment of 8, are determined by the target ABI.  On other 
platforms, such pointers might have alignment 8, 4, 2 or 1.  But the 
rules that say unions and structs take the alignment of their most 
aligned field, and that padding is added to the end of structs or 
between fields to maintain alignment, are determined by the C standards. 
  And these rules apply equally to anonymous unions and structs.


A compiler can't change these rules without being non-conforming. 
That's fine to do with explicit extensions, such as the "packed" 
attribute.  But it takes a very strong motivation, and usually an 
explicit compiler flag, to break conformity for code without using 
extensions.

I think your best bet is a slightly smaller hammer - put 
__attribute__((packed, aligned(4)) on the anonymous struct, and 
__attribute__((aligned(64))) on the outer struct :

struct __attribute__((aligned(64))) cb3_item  {
         union {
                 struct __attribute__((packed, aligned(4))) {
                         void *     ptr1;
                         void *     ptr2;
                 };
                 uint8_t            raw[60];
         };
         unsigned int               _avail;
};
_Static_assert(sizeof(struct cb3_item) == 64, "Check cb3_item struct");


This way you'll get your "packed" attribute only on the bit you need it, 
and an alignment of 4 is better than an alignment of 1.  And an outer 
alignment of 64 ensures that the structure is cache-friendly - you 
wouldn't want a big array of these things to just have an 8-byte alignment.

mvh.,

David






> Case in point, here I'm using pahole to show the layout and padding of the
> struct:
> 
> struct cb3_item {
>          union {
>                  struct {
>                          void *     ptr1;                 /*     0     8 */
>                          void *     ptr2;                 /*     8     8 */
>                  };                                       /*     0    16 */
>                  uint8_t            raw[60];              /*     0    60 */
>          };                                               /*     0    64 */
>          /* --- cacheline 1 boundary (64 bytes) --- */
>          unsigned int               _avail;               /*    64     4 */
> 
>          /* size: 72, cachelines: 2, members: 2 */
>          /* padding: 4 */
>          /* last cacheline: 8 bytes */
> };
> 
> So my issue here is that IMHO since the union inside the struct is
> anonymous there should be no need for the 4 byte padding between the union
> and the _avail member. The _avail being a 32-bit integer is perfectly fine
> on a 4 byte alignment so fits perfectly at position 60 which would make the
> full struct 64 bytes in size and thus perfectly aligned for 64-bit
> alignment.
> 
> But instead GCC (v13.2.0 in this case) aligns the union in case I would
> make a pointer to it or an array of them but this cannot be done when
> defined the way it is (I mean if I had done it as "union { } *p;" or
> similar then I would fully understand that gcc would align it this way).
> 
> And yes I know that I can "fix" this with __attribute__((packed,aligned(1))
> but that is a very broad hammer here since now one have to perform manual
> alignment of each union member since alignment inside the union is ok here
> but the issue is the alignment of the union as a whole.
> 
> Unless ofc if there are some other nifty solution to this that is not as
> broad as using forced alignment?
> 
> /HH
> 

Re: question on alignment of anonymous unions inside structs

[Henrik Holst]

[-- Attachment #1: Type: text/plain, Size: 4439 bytes --]

thanks David!

/HH

Den tis 7 maj 2024 kl 13:01 skrev David Brown <david.brown@hesbynett.no>:

> On 07/05/2024 02:06, Henrik Holst via Gcc-help wrote:
> > Hi,
> >
> >   I understand that unions inside structs have to be aligned in case they
> > are referenced by a pointer or someone creates an array of them but when
> > the union is completely anonymous and thus none of this is possible,
> > shouldn't the alignment rules be relaxed?
> >
>
> Alignment rules are decided by the ABI for a given platform, along with
> the C standards.  They have to be consistent, so that you can have the
> same declaration in different bits of code, perhaps compiled with
> different tools, and the layout is the same.
>
> So the alignment values, such as on your target, "void *" pointers have
> an alignment of 8, are determined by the target ABI.  On other
> platforms, such pointers might have alignment 8, 4, 2 or 1.  But the
> rules that say unions and structs take the alignment of their most
> aligned field, and that padding is added to the end of structs or
> between fields to maintain alignment, are determined by the C standards.
>   And these rules apply equally to anonymous unions and structs.
>
>
> A compiler can't change these rules without being non-conforming.
> That's fine to do with explicit extensions, such as the "packed"
> attribute.  But it takes a very strong motivation, and usually an
> explicit compiler flag, to break conformity for code without using
> extensions.
>
> I think your best bet is a slightly smaller hammer - put
> __attribute__((packed, aligned(4)) on the anonymous struct, and
> __attribute__((aligned(64))) on the outer struct :
>
> struct __attribute__((aligned(64))) cb3_item  {
>          union {
>                  struct __attribute__((packed, aligned(4))) {
>                          void *     ptr1;
>                          void *     ptr2;
>                  };
>                  uint8_t            raw[60];
>          };
>          unsigned int               _avail;
> };
> _Static_assert(sizeof(struct cb3_item) == 64, "Check cb3_item struct");
>
>
> This way you'll get your "packed" attribute only on the bit you need it,
> and an alignment of 4 is better than an alignment of 1.  And an outer
> alignment of 64 ensures that the structure is cache-friendly - you
> wouldn't want a big array of these things to just have an 8-byte alignment.
>
> mvh.,
>
> David
>
>
>
>
>
>
> > Case in point, here I'm using pahole to show the layout and padding of
> the
> > struct:
> >
> > struct cb3_item {
> >          union {
> >                  struct {
> >                          void *     ptr1;                 /*     0     8
> */
> >                          void *     ptr2;                 /*     8     8
> */
> >                  };                                       /*     0    16
> */
> >                  uint8_t            raw[60];              /*     0    60
> */
> >          };                                               /*     0    64
> */
> >          /* --- cacheline 1 boundary (64 bytes) --- */
> >          unsigned int               _avail;               /*    64     4
> */
> >
> >          /* size: 72, cachelines: 2, members: 2 */
> >          /* padding: 4 */
> >          /* last cacheline: 8 bytes */
> > };
> >
> > So my issue here is that IMHO since the union inside the struct is
> > anonymous there should be no need for the 4 byte padding between the
> union
> > and the _avail member. The _avail being a 32-bit integer is perfectly
> fine
> > on a 4 byte alignment so fits perfectly at position 60 which would make
> the
> > full struct 64 bytes in size and thus perfectly aligned for 64-bit
> > alignment.
> >
> > But instead GCC (v13.2.0 in this case) aligns the union in case I would
> > make a pointer to it or an array of them but this cannot be done when
> > defined the way it is (I mean if I had done it as "union { } *p;" or
> > similar then I would fully understand that gcc would align it this way).
> >
> > And yes I know that I can "fix" this with
> __attribute__((packed,aligned(1))
> > but that is a very broad hammer here since now one have to perform manual
> > alignment of each union member since alignment inside the union is ok
> here
> > but the issue is the alignment of the union as a whole.
> >
> > Unless ofc if there are some other nifty solution to this that is not as
> > broad as using forced alignment?
> >
> > /HH
> >
>
>