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 15A69439E3; Sat, 27 Jan 2024 20:15:26 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 88DF4402E0; Sat, 27 Jan 2024 20:15:25 +0100 (CET) Received: from mail.lysator.liu.se (mail.lysator.liu.se [130.236.254.3]) by mails.dpdk.org (Postfix) with ESMTP id 1AAD2402D5 for ; Sat, 27 Jan 2024 20:15:24 +0100 (CET) Received: from mail.lysator.liu.se (localhost [127.0.0.1]) by mail.lysator.liu.se (Postfix) with ESMTP id B5EB959C5 for ; Sat, 27 Jan 2024 20:15:23 +0100 (CET) Received: by mail.lysator.liu.se (Postfix, from userid 1004) id AA27F59C4; Sat, 27 Jan 2024 20:15:23 +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 2FA8F5A2A; Sat, 27 Jan 2024 20:15:20 +0100 (CET) Message-ID: <635f0d9f-6665-426b-b778-d61e5e732fbe@lysator.liu.se> Date: Sat, 27 Jan 2024 20:15:19 +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: =?UTF-8?Q?Morten_Br=C3=B8rup?= , Tyler Retzlaff , dev@dpdk.org Cc: =?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> From: =?UTF-8?Q?Mattias_R=C3=B6nnblom?= In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35E9F1A7@smartserver.smartshare.dk> 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-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? >> >> __rte_cache_aligned is shorter, provides a tiny bit of abstraction, and >> is already an established DPDK standard. So just keep the macro. If it >> would change, I would argue for it to be changed to rte_cache_aligned >> (i.e., just moving it out of __ namespace, and maybe making it >> all-uppercase). >> >> Non-trivial C programs wrap things all the time, standard or not. It's >> not something to be overly concerned about, imo. > > Using the cache alignment macro was obviously a bad example for discussing the __rte_aligned() macro. > > FYI, Tyler later agreed to introducing the RTE_CACHE_ALIGNAS you had proposed in an earlier correspondence. > >> >>> >>> Note: I don't mind convenience macros for common use cases, so we >> could also introduce the macro suggested by Mattias [1]: >>> >>> #define RTE_CACHE_ALIGNAS alignas(RTE_CACHE_LINE_SIZE) >>> >>> [1]: https://inbox.dpdk.org/dev/dc3f3131-38e6-4219-861e- >> b31ec10c08bb@lysator.liu.se/ >>> >>>> >>>> 2. where should we place alignas(n) or __rte_macro (if we use a >> macro) >>>> >>>> Should it be on the same line as the variable or field or on the >>>> preceeding line? >>>> >>>> /* same line example struct */ >>>> struct T { >>>> /* alignas(64) applies to field0 *not* struct T type >> declaration >>>> */ >>>> alignas(64) void *field0; >>>> void *field1; >>>> >>>> ... other fields ... >>>> >>>> alignas(64) uint64_t field5; >>>> uint32_t field6; >>>> >>>> ... more fields ... >>>> >>>> }; >>>> >>>> /* same line example array */ >>>> alignas(64) static const uint32_t array[4] = { ... }; >>>> >>>> -- or -- >>>> >>>> /* preceeding line example struct */ >>>> struct T { >>>> /* alignas(64) applies to field0 *not* struct T type >> declaration >>>> */ >>>> alignas(64) >>>> void *field0; >>>> void *field1; >>>> >>>> ... other fields ... >>>> >>>> alignas(64) >>>> uint64_t field5; >>>> uint32_t field6; >>>> >>>> ... more fields ... >>>> >>>> }; >>>> >>>> /* preceeding line example array */ >>>> alignas(64) >>>> static const uint32_t array[4] = { ... }; >>>> >>> >>> Searching the net for what other projects do, I came across this >> required placement [2]: >>> >>> uint64_t alignas(64) field5; >>> >>> [2]: >> https://lore.kernel.org/buildroot/20230730000851.6faa3391@windsurf/T/ >>> >>> So let's follow the standard's intention and put them on the same >> line. >>> On an case-by-case basis, we can wrap lines if it improves >> readability, like we do with function headers that have a lot of >> attributes. >>> >>>> >>>> I'll submit patches for lib/* once the discussion is concluded. >>>> >>>> thanks folks >>>