From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 9E6734333B; Wed, 15 Nov 2023 22:03:34 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id EB61C402B7; Wed, 15 Nov 2023 22:03:33 +0100 (CET) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mails.dpdk.org (Postfix) with ESMTP id A04D5402B2 for ; Wed, 15 Nov 2023 22:03:32 +0100 (CET) Received: by linux.microsoft.com (Postfix, from userid 1086) id D1E1220B74C1; Wed, 15 Nov 2023 13:03:31 -0800 (PST) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com D1E1220B74C1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1700082211; bh=sy0mDTHSz0MVN1FmV0ptM+v/mXGGp7zCzeKISNCPFg0=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=fz8Sbm7/2be+oUkZTE4LkAzHvNPGT6vwgNYC2SPASll2cJw8rUVw0AaIU9eOO9DgK 5thgZgqC8mvuBZrzp6MQDR5Qhe9ZIE8/fzEtuJR+bcQtgkLogL7Zujpj1xetZWHz48 qc2XJIBwSkn90x+ASUQdPRdbdsb6DaG3f30yWcu4= Date: Wed, 15 Nov 2023 13:03:31 -0800 From: Tyler Retzlaff To: Morten =?iso-8859-1?Q?Br=F8rup?= Cc: dev@dpdk.org, Stanislaw Kardach , Mattias =?iso-8859-1?Q?R=F6nnblom?= , Anatoly Burakov , Bruce Richardson , David Christensen , Harry van Haaren , Konstantin Ananyev , Min Zhou , Ruifeng Wang Subject: Re: [PATCH] eal: use C11 alignas instead of GCC attribute aligned Message-ID: <20231115210331.GA524@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> References: <1700069997-4399-1-git-send-email-roretzla@linux.microsoft.com> <1700069997-4399-2-git-send-email-roretzla@linux.microsoft.com> <98CBD80474FA8B44BF855DF32C47DC35E9F02F@smartserver.smartshare.dk> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35E9F02F@smartserver.smartshare.dk> User-Agent: Mutt/1.5.21 (2010-09-15) X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Wed, Nov 15, 2023 at 09:08:05PM +0100, Morten Brørup wrote: > > 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. 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 > > #include > > #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. >