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 AD2C143903; Fri, 19 Jan 2024 22:05:32 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 5242A402D9; Fri, 19 Jan 2024 22:05:32 +0100 (CET) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mails.dpdk.org (Postfix) with ESMTP id C8F4A4029F for ; Fri, 19 Jan 2024 22:05:29 +0100 (CET) Received: by linux.microsoft.com (Postfix, from userid 1086) id E703820DFD73; Fri, 19 Jan 2024 13:05:28 -0800 (PST) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com E703820DFD73 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1705698328; bh=Og7MhZU9DPPtCT/+usBrIkmx3DUFAIca6baM4jMAGcg=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=rbHYYpybxDBmI+S1q63+CL+yZ8qywjfIZbTHx8ha9MwrgJ3IxmaOWrliGba52tgT9 MH4Kg6uTTauM87/snJez13ppgcFcdeiT8AtKbwsEzvAHUjbeu57CNe4Ye/xVP1bYaT 12g/bLKMBglDgsfjrnZqce3Kga5q5pHQXeV8/94c= Date: Fri, 19 Jan 2024 13:05:28 -0800 From: Tyler Retzlaff To: Morten =?iso-8859-1?Q?Br=F8rup?= Cc: Stephen Hemminger , Bruce Richardson , dev@dpdk.org, Jerin Jacob Subject: Re: [PATCH v4] eventdev: ensure 16-byte alignment for events Message-ID: <20240119210528.GB24862@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> References: <20231005115101.12276-1-bruce.richardson@intel.com> <20231006102932.164630-1-bruce.richardson@intel.com> <20231111160104.49360157@hermes.local> <98CBD80474FA8B44BF855DF32C47DC35E9F016@smartserver.smartshare.dk> <20231112153146.5fdd72b8@hermes.local> <98CBD80474FA8B44BF855DF32C47DC35E9F018@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: <98CBD80474FA8B44BF855DF32C47DC35E9F018@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 Mon, Nov 13, 2023 at 08:58:19AM +0100, Morten Brørup wrote: > > From: Stephen Hemminger [mailto:stephen@networkplumber.org] > > Sent: Monday, 13 November 2023 00.32 > > > > On Sun, 12 Nov 2023 09:30:24 +0100 > > Morten Brørup wrote: > > > > > > > +static_assert(sizeof(struct rte_event) == 16, "Event structure > > size > > > > is not 16-bytes in size"); > > > > > + > > > > > static struct rte_eventdev > > rte_event_devices[RTE_EVENT_MAX_DEVS]; > > > > > > > > Please don't reinvent RTE_BUILD_BUG_ON(). > > > > Instead fix that to be a static_assert() > > > > > > I would say the opposite: > > > With our upgrade to the C11 standard, let's get rid of the > > RTE_BUILD_BUG_ON() workaround for the lack of static_assert() in older > > C standards. > > > > > > Unfortunately, the static_assert(expression) variant without the > > "message" parameter, which would make our RTE_BUILD_BUG_ON() macro > > completely obsolete, requires C23. And I don't see how we can make this > > variant available with C11. So we probably have to wait until DPDK > > requires C23. > > > > > > Until then, let's gradually phase out the DPDK-specific > > RTE_BUILD_BUG_ON() in favor of standard C's static_assert(), and live > > with the inconvenience of having to provide a message parameter for it. > > > > > > Please also note that static_assert() can be used outside code > > blocks, which makes it handy for use in header files. > > > > If you look at my RFC, the message is just as good as the one in this > > code. > > It ends up being stringified version of the expression. Which is more > > exact than the wording used in some other places. > > I agree that the output of your RFC is better than that of static_assert(expr, msg). > I'm arguing that RTE_BUILD_BUG_ON() would be considered reinventing the wheel if introduced today, because the C11 standard already offers something similar. So we should prefer that over RTE_BUILD_BUG_ON(), not the other way around. > > It's a difference in opinion. Both RTE_BUILD_BUG_ON() and static_assert() are viable; you seem to prefer the first, I prefer the latter. > > Both occur in existing DPDK code, and Bruce happened to chose static_assert() for this patch. > > The overall question is a choice between three options: > 1. prefer using RTE_BUILD_BUG_ON() (using your RFC implementation), > 2. phase out RTE_BUILD_BUG_ON() in favor of the C11 standard's static_assert(), or > 3. continue using both? > > I'm not strongly opposed to either of the three, as long as the community agrees. > not a strong preference, but i see no reason to maintain macros when the standard offers nearly equivalent. * keep RTE_BUILD_BUG_ON * discourage new additions using RTE_BUILD_BUG_ON with checkpatches * convert to static_assert gradually over time glad to see the conversion picked up real problems though, thank you stephen for doing this.