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 4B2A643A08; Tue, 30 Jan 2024 09:08:28 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id C70834029A; Tue, 30 Jan 2024 09:08:27 +0100 (CET) Received: from mail.lysator.liu.se (mail.lysator.liu.se [130.236.254.3]) by mails.dpdk.org (Postfix) with ESMTP id 2AAA340267 for ; Tue, 30 Jan 2024 09:08:26 +0100 (CET) Received: from mail.lysator.liu.se (localhost [127.0.0.1]) by mail.lysator.liu.se (Postfix) with ESMTP id 3C5FE10BE9 for ; Tue, 30 Jan 2024 09:08:25 +0100 (CET) Received: by mail.lysator.liu.se (Postfix, from userid 1004) id 1ED9710CAF; Tue, 30 Jan 2024 09:08:25 +0100 (CET) X-Spam-Checker-Version: SpamAssassin 4.0.0 (2022-12-13) on hermod.lysator.liu.se X-Spam-Level: X-Spam-Status: No, score=-1.4 required=5.0 tests=ALL_TRUSTED,AWL, T_SCC_BODY_TEXT_LINE autolearn=disabled version=4.0.0 X-Spam-Score: -1.4 Received: from [192.168.1.59] (h-62-63-215-114.A163.priv.bahnhof.se [62.63.215.114]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mail.lysator.liu.se (Postfix) with ESMTPSA id E548910BE5; Tue, 30 Jan 2024 09:08:21 +0100 (CET) Message-ID: Date: Tue, 30 Jan 2024 09:08:21 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] RFC: use C11 alignas instead of GCC attribute aligned Content-Language: en-US To: Tyler Retzlaff Cc: =?UTF-8?Q?Morten_Br=C3=B8rup?= , dev@dpdk.org, =?UTF-8?Q?Mattias_R=C3=B6nnblom?= , Anatoly Burakov , Bruce Richardson , David Christensen , Harry van Haaren , Konstantin Ananyev , Min Zhou , Ruifeng Wang , Stanislaw Kardach , thomas@monjalon.net References: <1700069997-4399-1-git-send-email-roretzla@linux.microsoft.com> <20240125183713.GA27715@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> <98CBD80474FA8B44BF855DF32C47DC35E9F1A3@smartserver.smartshare.dk> <91ba1ece-10dd-4698-acd9-6b51cfc63cd9@lysator.liu.se> <98CBD80474FA8B44BF855DF32C47DC35E9F1A7@smartserver.smartshare.dk> <635f0d9f-6665-426b-b778-d61e5e732fbe@lysator.liu.se> <98CBD80474FA8B44BF855DF32C47DC35E9F1AA@smartserver.smartshare.dk> <413840c1-a263-4118-adfe-d4ae0ec0b52d@lysator.liu.se> <20240129194356.GA25654@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> From: =?UTF-8?Q?Mattias_R=C3=B6nnblom?= In-Reply-To: <20240129194356.GA25654@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Virus-Scanned: ClamAV using ClamSMTP 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 2024-01-29 20:43, Tyler Retzlaff wrote: > On Sun, Jan 28, 2024 at 11:00:31AM +0100, Mattias Rönnblom wrote: >> On 2024-01-28 09:57, Morten Brørup wrote: >>>> From: Mattias Rönnblom [mailto:hofors@lysator.liu.se] >>>> Sent: Saturday, 27 January 2024 20.15 >>>> >>>> On 2024-01-26 11:18, Morten Brørup wrote: >>>>>> From: Mattias Rönnblom [mailto:hofors@lysator.liu.se] >>>>>> Sent: Friday, 26 January 2024 11.05 >>>>>> >>>>>> On 2024-01-25 23:53, Morten Brørup wrote: >>>>>>>> From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com] >>>>>>>> Sent: Thursday, 25 January 2024 19.37 >>>>>>>> >>>>>>>> ping. >>>>>>>> >>>>>>>> Please review this thread if you have time, the main point of >>>>>>>> discussion >>>>>>>> I would like to receive consensus on the following questions. >>>>>>>> >>>>>>>> 1. Should we continue to expand common alignments behind an >>>>>> __rte_macro >>>>>>>> >>>>>>>> i.e. what do we prefer to appear in code >>>>>>>> >>>>>>>> alignas(RTE_CACHE_LINE_MIN_SIZE) >>>>>>>> >>>>>>>> -- or -- >>>>>>>> >>>>>>>> __rte_cache_aligned >>>>>>>> >>>>>>>> One of the benefits of dropping the macro is it provides a clear >>>>>> visual >>>>>>>> indicator that it is not placed in the same location or get >>>> applied >>>>>>>> to types as is done with __attribute__((__aligned__(n))). >>>>>>> >>>>>>> We don't want our own proprietary variant of something that already >>>>>> exists in the C standard. Now that we have moved to C11, the __rte >>>>>> alignment macros should be considered obsolete. >>>>>> >>>>>> Making so something cache-line aligned is not in C11. >>>>> >>>>> We are talking about the __rte_aligned() macro, not the cache >>>> alignment macro. >>>>> >>>> >>>> OK, in that case, what is the relevance of question 1 above? >>> >>> With this in mind, try re-reading Tyler's clarifications in this tread. >>> >>> Briefly: alignas() can be attached to variables and structure fields, but not to types (like __rte_aligned()), so to align a structure: >>> >>> struct foo { >>> int alignas(64) bar; /* alignas(64) must be here */ >>> int baz; >>> }; /* __rte_aligned(64) was here, but alignas(64) cannot be here. */ >>> >>> So the question is: Do we want to eliminate the __rte_aligned() macro - which relies on compiler attributes - and migrate to using the C11 standard alignas()? >>> >>> I think yes; after updating to C11, the workaround for pre-C11 not offering alignment is obsolete, and its removal should be on the roadmap. >>> >> >> OK, thanks for the explanation. Interesting limitation in the standard. >> >> If the construct the standard is offering is less effective (in this >> case, less readable) and the non-standard-based option is possible >> to implement on all compilers (i.e., on MSVC too), then we should >> keep the custom option. Especially if it's already there, but also >> in cases where it isn't. >> >> In fact, one could argue *everything* related to alignment should go >> through something rte_, __rte_ or RTE_-prefixed. So, "int >> RTE_ALIGNAS(64) bar;". Maybe that would be silly, but it would be >> consistent with RTE_CACHE_ALIGNAS. >> >> I would worry more about allowing DPDK developers writing clean and >> readable code, than very slightly lowering the bar for the fraction >> of newcomers experienced with the latest and greatest from the C >> standard, and *not* familiar with age-old GCC extensions. > > I’d just like to summarize where my understanding is at after reviewing > this discussion and my downstream branch. But I also want to make it > clear that we probably need to use both standard C and non-standard > attribute/declspec for object and struct/union type alignment > respectively. > > I've assumed we prefer avoiding per-compiler conditional expansion when > possible through the use of standard C mechanisms. But there are > instances when alignas is awkward. > > So I think the following is consistent with what Mattias is advocating > sans any discussions related to actual naming of macros. > > We should have 2 macros, upon which others may be built to expand to > well-known values for e.g. cache line size. > > RTE_ALIGNAS(n) object; > > * This macro is used to align C objects i.e. variable, array, struct/union > fields etc. > * Trivially expands to alignas(n) for all toolchains. > * Placed in a location that both C and C++ translation units accept that > is on the same line preceeding the object type. > example: > // RTE_ALIGNAS(n) object; > RTE_ALIGNAS(16) char somearray[16]; > > RTE_ALIGN_TYPE(n) > > * This macro is used to align struct/union types. > * Conditionally expands to __declspec(align(n)) (msvc) and > __attribute__((__aligned__(n))) (for all other toolchains) > * Placed in a location that for all gcc,clang,msvc and both C and C++ > translation units accept. > example: > // {struct,union} RTE_ALIGN_TYPE(n) tag { ... }; > struct RTE_ALIGN_TYPE(64) sometype { ... }; > Sorry if I've missed some discussion on the list, but the current pattern of putting __rte_aligned(X) at the end doesn't work with MSVC, or why are we doing this? C11 purism doesn't seem like much of a driving force. If one defined a macro as __declspec(align(X)) on MSVC and __attribute__(__aligned__(X)) on other compilers, could it do the work of both the above RTE_ALIGNAS() and RTE_ALIGN_TYPE()? struct { int a; } ; You would have to mandate the placement of such a __rte_aligned plug-in replacement being at rather than (the more intuitive?) , since clang doesn't like __attribute__s before the struct/union keyword, correct? What about other __attribute__ wrappers like __rte_packed; would they also need to change placement to make DPDK work with MSVC? > I'm not picky about what the names actualy are if you have better > suggestions i'm happy to adopt them. > > Thoughts? Comments? > > Appreciate the discussion this has been helpful. > > ty >