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

["Morten Brørup" <mb@smartsharesystems.com>]

> From: Tyler Retzlaff [mailto:roretzla@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.

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.

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.