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 B0547460F3; Thu, 23 Jan 2025 08:55:32 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 4049B40279; Thu, 23 Jan 2025 08:55:32 +0100 (CET) Received: from dkmailrelay1.smartsharesystems.com (smartserver.smartsharesystems.com [77.243.40.215]) by mails.dpdk.org (Postfix) with ESMTP id DD72640269 for ; Thu, 23 Jan 2025 08:55:30 +0100 (CET) Received: from smartserver.smartsharesystems.com (smartserver.smartsharesys.local [192.168.4.10]) by dkmailrelay1.smartsharesystems.com (Postfix) with ESMTP id 95EDF20904; Thu, 23 Jan 2025 08:55:30 +0100 (CET) Content-class: urn:content-classes:message Subject: RE: [PATCH 2/2] lib/hash: avoid implicit conversion to 64 bit number MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Date: Thu, 23 Jan 2025 08:55:29 +0100 Message-ID: <98CBD80474FA8B44BF855DF32C47DC35E9F9DC@smartserver.smartshare.dk> X-MimeOLE: Produced By Microsoft Exchange V6.5 In-Reply-To: <20250122213643.GA26516@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: [PATCH 2/2] lib/hash: avoid implicit conversion to 64 bit number Thread-Index: AdttFb3cVgiAOHtTQmqAFhvApYPZegAVTBHg References: <1732758837-6350-1-git-send-email-andremue@linux.microsoft.com> <1732758837-6350-2-git-send-email-andremue@linux.microsoft.com> <20250122213643.GA26516@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> From: =?iso-8859-1?Q?Morten_Br=F8rup?= To: "Andre Muezerie" , "Bruce Richardson" Cc: "Yipeng Wang" , "Sameh Gobriel" , "Vladimir Medvedkin" , 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: Andre Muezerie [mailto:andremue@linux.microsoft.com] > Sent: Wednesday, 22 January 2025 22.37 >=20 > On Wed, Jan 22, 2025 at 04:12:49PM +0000, Bruce Richardson wrote: > > On Wed, Nov 27, 2024 at 05:53:57PM -0800, Andre Muezerie wrote: > > > MSVC issues the warnings below: > > > > > > 1) ../lib/hash/rte_thash_gf2_poly_math.c(128): warning C4334: = '<<': > > > result of 32-bit shift implicitly converted to 64 bits > > > (was 64-bit shift intended?) > > > > > > The code would be better off by using 64 bit numbers to begin = with. > > > That eliminates the need for a conversion to 64 bits later. > > > > > > 2) ../lib/hash/rte_thash.c(568): warning C4334: '<<': > > > result of 32-bit shift implicitly converted to 64 bits > > > (was 64-bit shift intended?) > > > > > > 1ULL should be used as the result of the bit shift gets multiplied > > > by sizeof(uint32_t). > > > > > > Signed-off-by: Andre Muezerie > > > --- > > > > Acked-by: Bruce Richardson > > > > > lib/hash/rte_thash.c | 2 +- > > > lib/hash/rte_thash_gf2_poly_math.c | 6 +++--- > > > 2 files changed, 4 insertions(+), 4 deletions(-) > > > > > > diff --git a/lib/hash/rte_thash.c b/lib/hash/rte_thash.c > > > index fa78787143..f076311b57 100644 > > > --- a/lib/hash/rte_thash.c > > > +++ b/lib/hash/rte_thash.c > > > @@ -565,7 +565,7 @@ rte_thash_add_helper(struct rte_thash_ctx = *ctx, > const char *name, uint32_t len, > > > offset; > > > > > > ent =3D rte_zmalloc(NULL, sizeof(struct = rte_thash_subtuple_helper) > + > > > - sizeof(uint32_t) * (1 << ctx->reta_sz_log), > > > + sizeof(uint32_t) * (1ULL << ctx->reta_sz_log), > > > RTE_CACHE_LINE_SIZE); > > > > Is there a reason not to use RTE_BIT64 here too? >=20 > Here we are calculating the size to be passed to the second argument = of > rte_zmalloc, which is of type size_t. size_t is implementation > dependent, typically 4 bytes on 32-bit systems and 8 bytes on 64-bit > systems, so using 1ULL seems more appropriate. 1ULL makes it 8 byte on 32-bit systems too. Did you mean 1UL? How about reducing the formula to directly shift the sizeof() instead, = i.e.: sizeof(uint32_t) << ctx->reta_sz_log,