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 59B1345C76; Mon, 4 Nov 2024 13:53:38 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id E32DC40BA4; Mon, 4 Nov 2024 13:53:37 +0100 (CET) Received: from dkmailrelay1.smartsharesystems.com (smartserver.smartsharesystems.com [77.243.40.215]) by mails.dpdk.org (Postfix) with ESMTP id CBBA4400D5 for ; Mon, 4 Nov 2024 13:53:36 +0100 (CET) Received: from smartserver.smartsharesystems.com (smartserver.smartsharesys.local [192.168.4.10]) by dkmailrelay1.smartsharesystems.com (Postfix) with ESMTP id 92ABB2027D; Mon, 4 Nov 2024 13:53:36 +0100 (CET) Content-class: urn:content-classes:message MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Subject: RE: [PATCH 1/2] eal: add unreachable and precondition hints Date: Mon, 4 Nov 2024 13:53:35 +0100 Message-ID: <98CBD80474FA8B44BF855DF32C47DC35E9F871@smartserver.smartshare.dk> X-MimeOLE: Produced By Microsoft Exchange V6.5 In-Reply-To: X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: [PATCH 1/2] eal: add unreachable and precondition hints Thread-Index: Adsus8hSSxRidA4FSn+TXWQYjVW0xAAAWj6w References: <20241025115223.1230680-1-mb@smartsharesystems.com> <20241025115223.1230680-2-mb@smartsharesystems.com> <98CBD80474FA8B44BF855DF32C47DC35E9F86E@smartserver.smartshare.dk> From: =?iso-8859-1?Q?Morten_Br=F8rup?= To: "Bruce Richardson" Cc: "Tyler Retzlaff" , "Thomas Monjalon" , , , "Ferruh Yigit" , "Andrew Rybchenko" , "Ajit Khaparde" , "Somnath Kotur" , "Gaetan Rivet" , "Jie Hai" , "Long Li" , "Wei Hu" 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 > From: Bruce Richardson [mailto:bruce.richardson@intel.com] > Sent: Monday, 4 November 2024 13.19 >=20 > On Mon, Nov 04, 2024 at 12:40:49PM +0100, Morten Br=F8rup wrote: > > Ping for review. > > > > > From: Morten Br=F8rup [mailto:mb@smartsharesystems.com] > > > Sent: Friday, 25 October 2024 13.52 > > > > > > Added two new compiler/optimizer hints: > > > * The __rte_unreachable hint for use in points in code known never > to > > > be > > > reached. > > > * The __rte_assume hint for providing information about > preconditions > > > the > > > compiler/optimizer might be unable to figure out by itself. > > > > > > Signed-off-by: Morten Br=F8rup > > > --- > > > lib/eal/include/rte_common.h | 27 +++++++++++++++++++++++++++ > > > 1 file changed, 27 insertions(+) > > > > > > diff --git a/lib/eal/include/rte_common.h > > > b/lib/eal/include/rte_common.h > > > index c79f9ed319..2f143c87e4 100644 > > > --- a/lib/eal/include/rte_common.h > > > +++ b/lib/eal/include/rte_common.h > > > @@ -366,6 +366,16 @@ static void > > > __attribute__((destructor(RTE_PRIO(prio)), used)) func(void) > > > #define __rte_noreturn __attribute__((noreturn)) > > > #endif > > > > > > +/** > > > + * Hint point in program never reached > > > + */ > > > +#if defined(RTE_TOOLCHAIN_GCC) || defined(RTE_TOOLCHAIN_CLANG) > > > +#define __rte_unreachable() = __extension__(__builtin_unreachable()) > > > +#else > > > +/* MSVC or ICC */ > > > +#define __rte_unreachable() __assume(0) > > > +#endif > > > + >=20 > Is there already somewhere in the code where we might want to use = this? Yes. It's used in the __rte_assume() macro below for GCC before version = 13. > I'm > not sure about just adding macros just in case they might be needed in > future. IMHO, such hints might be useful for applications, and DPDK could use = them in the future. But if it came to a vote about adding unused/dead code, it would = probably not be accepted. Anyway, this macro is not unused, so no problem here. > Having unreachable code seems a bit problematic to me generally > anyway. Agree. The correct use of __rte_unreachable() is to provide information to the = compiler/optimizer/sanitizer it cannot figure out by itself, and/or as = information to the code reviewer. Using it to prevent compiler warnings from unreachable code would be = utterly wrong. I don't know if that is even possible. >=20 > > > /** > > > * Issue a warning in case the function's return value is = ignored. > > > * > > > @@ -423,6 +433,23 @@ static void > > > __attribute__((destructor(RTE_PRIO(prio)), used)) func(void) > > > #define __rte_cold __attribute__((cold)) > > > #endif > > > > > > +/** > > > + * Hint precondition > > > + * > > > + * @warning Depending on the compiler, any code in ``condition`` > might > > > be executed. > > > + * This currently only occurs with GCC prior to version 13. > > > + */ > > > +#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION >=3D 130000) > > > +#define __rte_assume(condition) = __attribute__((assume(condition))) > > > +#elif defined(RTE_TOOLCHAIN_GCC) > > > +#define __rte_assume(condition) do { if (!(condition)) > > > __rte_unreachable(); } while (0) Note: I did not come up with this myself; this seems to be the = "official" way of doing it with older GCC versions. > > > +#elif defined(RTE_TOOLCHAIN_CLANG) > > > +#define __rte_assume(condition) > > > __extension__(__builtin_assume(condition)) > > > +#else > > > +/* MSVC or ICC */ > > > +#define __rte_assume(condition) __assume(condition) > > > +#endif > > > + >=20 > This part seems ok, I see it used in the next patch, and also looks > rather > useful to have. >=20 > /Bruce