From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id B867FA052B; Tue, 28 Jul 2020 17:59:43 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id F10C11BFF2; Tue, 28 Jul 2020 17:59:42 +0200 (CEST) Received: from new4-smtp.messagingengine.com (new4-smtp.messagingengine.com [66.111.4.230]) by dpdk.org (Postfix) with ESMTP id 6D06E2B9C for ; Tue, 28 Jul 2020 17:59:41 +0200 (CEST) Received: from compute7.internal (compute7.nyi.internal [10.202.2.47]) by mailnew.nyi.internal (Postfix) with ESMTP id C5740580505; Tue, 28 Jul 2020 11:59:40 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute7.internal (MEProxy); Tue, 28 Jul 2020 11:59:40 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=fm1; bh= 2A/EflRqkMF4deGvqJ6dgtr6bHX2T4975SR5LyNO0HY=; b=vaizR2nFYBmKRsOG CwC6qbLsK4p5Wx555/p5gGJmdwvh4YYA+AHs1gnQ+7urnHLMrlUV481Er3iFrfSU jRA6koPQ6bW3Be7TFN/+cYk0KZbUx+XDuJPjtkeSRkSw1ccsgpfi75aIp9oUIbhy ffqUQ/kYpyyKBWmzFmT40jdLQXjNvHX2ia4arCJB+l+Fzdxs2qXDqbLSxWp3C1nQ ydloqpuGtVdTWbsEVatc4ejD4svgMSbPkS1fam9w43wrt7zaYtCc5nTJMzeEJH/i fjgZLKg48eV8UqQhv3Ze833GUuOVN0SizVXYUxLr4CKtBbPYHDg7lhVm/MTzWNDB 2sNHqg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm3; bh=2A/EflRqkMF4deGvqJ6dgtr6bHX2T4975SR5LyNO0 HY=; b=S6cci8qdkO/mvbpyor+j+H41WEkKkpznprjCurVBiA0WuqY/lU28z7HAr 8nzitFIYCZ4JHSNEt1oap32Ozm56GYRQ4gKA8vu3ZjYJhqWsEdOqfFemNVeOthDQ nWJNVsocsF8nNDunPcQBm8EIzPhxS1HgCYQL34YvGMY4s1J19TicWeT7cVsy8rl6 K/GD93gubCZ1tZm93PhxzSBYHOC+OwM8VQuY+IS7wbyQRDtJrPTSaY5S5shSNwTY hok0tPckWmt08p93X+agJVYj2ggPJ+XnZCqdLNGc6QcFBM983ydpKqSWuHu1G+gc tPaTItOwS04V0S68CJVD89LkgEfWg== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduiedriedvgdelgecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefhvffufffkjghfggfgtgesthhqredttddtudenucfhrhhomhepvfhhohhmrghs ucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenucggtf frrghtthgvrhhnpedvteeigfevkedugeeigfdtffekvddvheevffekledviefguedugfel jeekleeugfenucffohhmrghinhepughpughkrdhorhhgpdgvuhhrphhrugdthedrphhroh gupdhouhhtlhhoohhkrdgtohhmnecukfhppeejjedrudefgedrvddtfedrudekgeenucev lhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehthhhomhgrsh esmhhonhhjrghlohhnrdhnvght X-ME-Proxy: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id 44A9E328005E; Tue, 28 Jul 2020 11:59:38 -0400 (EDT) From: Thomas Monjalon To: =?ISO-8859-1?Q?Ga=EBtan?= Rivet , Morten =?ISO-8859-1?Q?Br=F8rup?= , Honnappa Nagarahalli Cc: Parav Pandit , "dev@dpdk.org" , "ferruh.yigit@intel.com" , Ray Kinsella , Neil Horman , "rasland@mellanox.com" , "orika@mellanox.com" , "matan@mellanox.com" , Joyce Kong , nd , Honnappa Nagarahalli , nd Date: Tue, 28 Jul 2020 17:59:37 +0200 Message-ID: <3052833.ftuu6NQJMv@thomas> In-Reply-To: References: <20200610171728.89-2-parav@mellanox.com> <20200728092906.3qmnncichcettmxr@u256.net> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="iso-8859-1" Subject: Re: [dpdk-dev] [PATCH v10 01/10] eal: introduce macro for bit definition X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 28/07/2020 17:39, Honnappa Nagarahalli: > > On 28/07/20 10:24 +0200, Morten Br=F8rup wrote: > > > > From: Honnappa Nagarahalli [mailto:Honnappa.Nagarahalli@arm.com] > > > > > > > --- a/lib/librte_eal/include/rte_bitops.h > > > > > > > +++ b/lib/librte_eal/include/rte_bitops.h > > > > > > > @@ -17,6 +17,14 @@ > > > > > > > #include > > > > > > > #include > > > > > > > > > > > > > > +/** > > > > > > > + * Get the uint64_t value for a specified bit set. > > > > > > > + * > > > > > > > + * @param nr > > > > > > > + * The bit number in range of 0 to 63. > > > > > > > + */ > > > > > > > +#define RTE_BIT64(nr) (UINT64_C(1) << (nr)) > > > > > > In general, the macros have been avoided in this file. Suggest > > > > > > changing this to an inline function. > > > > > > > > > > That has been discussed already, and rejected for good reasons: > > > > > > > http://inbox.dpdk.org/dev/AM0PR05MB4866823B0170B90F679A2765D1640@ > > > > > AM0PR05MB4866.eurprd05.prod.outlook.com/ > > > > Thank you for the link. > > > > In this patch series, I see the macro being used in enum > > > > initialization > > > > (7/10 in v11) as well as in functions (8/10 in v11). Does it make > > > > sense to introduce use inline functions and use the inline functions > > > > for 8/10? > > > > If we do this, we should document in rte_bitops.h that inline > > > > functions should be used wherever possible. > > > > > > I would agree, but only in theory. I disagree in reality, and argue t= hat there > > should only be macros for this. Here is why: > > > > > > rte_byteorder.h has both RTE_BEnn() macros and rte_cpu_to_be_nn() > > functions, for doing the same thing at compile time or at run time. The= re are > > no compile time warnings if the wrong one is being used, so I am certai= n that > > we can find code that uses the macro where the function should be used,= or > > vice versa. > Agree, there is not a suitable way to enforce the use of one over the oth= er (other than code review). >=20 > When the APIs in rte_bitops.h were introduced, there was a discussion aro= und using the macros. I was for using macros as it would have kept the code= as well as number of APIs smaller. However, there was a decision made not = to use macros and instead provide inline functions. It was nothing to do wi= th performance. So, I am just saying that we need to follow the same princi= ples at least for this file. I think bit definitions should be simple macros. Even macro is a bit overkill for this simple thing. I will merge this series. If we want to change the philosophy of macro definitions, it may be a larger discussion.