[PATCH] eal: use C11 alignas instead of GCC attribute aligned

[Tyler Retzlaff]

On Wed, Nov 15, 2023 at 09:08:05PM +0100, Morten Brørup wrote:
> > From: Tyler Retzlaff [mailto:roretzla at linux.microsoft.com]
> > Sent: Wednesday, 15 November 2023 18.40
> > 
> > Now that we have enabled C11 replace the use of __rte_cache_aligned
> > and __rte_aligned(n) with alignas(RTE_CACHE_LINE_SIZE) and
> > __rte_aligned(n) respectively.
> > 
> 
> [...]
> 
> >  typedef union rte_xmm {
> > +	alignas(16)
> >  	xmm_t    x;
> >  	uint8_t  u8[XMM_SIZE / sizeof(uint8_t)];
> >  	uint16_t u16[XMM_SIZE / sizeof(uint16_t)];
> >  	uint32_t u32[XMM_SIZE / sizeof(uint32_t)];
> >  	uint64_t u64[XMM_SIZE / sizeof(uint64_t)];
> >  	double   pd[XMM_SIZE / sizeof(double)];
> > -} __rte_aligned(16) rte_xmm_t;
> > +} rte_xmm_t;
> 
> Your patch message should mention that C11 doesn't allow alignas() being applied to the declarations of struct/union types, so it is applied to the first field in the struct/union, which has the same effect.

no problem, will add a note.

> 
> Someone unfamiliar with alignas() would expect:
> 
> -typedef union rte_xmm {
> +typedef alignas(16) union rte_xmm {
> [...]
> -} __rte_aligned(16) rte_xmm_t;
> +} rte_xmm_t;
> 
> [...]
> 
> >  #ifndef RTE_VECT_RISCV_H
> >  #define RTE_VECT_RISCV_H
> > 
> > +#include <stdalign.h>
> >  #include <stdint.h>
> >  #include "generic/rte_vect.h"
> >  #include "rte_common.h"
> > @@ -23,13 +24,14 @@
> >  #define XMM_MASK	(XMM_SIZE - 1)
> > 
> >  typedef union rte_xmm {
> > +	alignas(16) /* !! NOTE !! changed to 16 it looks like this was a
> > bug? */
> >  	xmm_t		x;
> >  	uint8_t		u8[XMM_SIZE / sizeof(uint8_t)];
> >  	uint16_t	u16[XMM_SIZE / sizeof(uint16_t)];
> >  	uint32_t	u32[XMM_SIZE / sizeof(uint32_t)];
> >  	uint64_t	u64[XMM_SIZE / sizeof(uint64_t)];
> >  	double		pd[XMM_SIZE / sizeof(double)];
> > -} __rte_aligned(8) rte_xmm_t;
> > +} rte_xmm_t;
> 
> Yes, this looks very much like a bug.
> Even if a RISC-V CPU could handle alignment like that, it might interact with other software/hardware expecting type-sized alignment, i.e. 16-byte alignment, so partially using 8-byte alignment would cause bugs.
> 
> It should be a separate patch with a Fixes tag.

i'll submit a patch/fix for this so it is available and others can
discuss if it should or shouldn't be merged for 23.11.

> 
> We need to urgently decide if this bug should live on in DPDK 23.11, or if the fix should be included although we are very late in the release process.
> 
> Stanislaw, what do you think?
> 
> Furthermore, I wonder if it can be backported to stable, and to what extent backporting it would break the ABI/API.
>