From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 721CC43270;
	Thu,  2 Nov 2023 16:36:07 +0100 (CET)
Received: from mails.dpdk.org (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id 4C14E40DDC;
	Thu,  2 Nov 2023 16:36:07 +0100 (CET)
Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182])
 by mails.dpdk.org (Postfix) with ESMTP id 36B4E40144
 for <dev@dpdk.org>; Thu,  2 Nov 2023 16:36:06 +0100 (CET)
Received: by linux.microsoft.com (Postfix, from userid 1086)
 id 896BA20B74C0; Thu,  2 Nov 2023 08:36:05 -0700 (PDT)
DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 896BA20B74C0
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com;
 s=default; t=1698939365;
 bh=R70ZNg+qGcbIS6SlYgBL8fDDibsh9MnVXJz75zDvWGI=;
 h=Date:From:To:Cc:Subject:References:In-Reply-To:From;
 b=gKK6Mc3tCDxryEf6Kxhj51x0+qfGrsSBkp9bcSwyDrcYZPvEGWtRizsD9Ns1YFskM
 m4iJ165LuOTJI65Aoo2OnI2h3w/dXAyd0ql2XdVaUPjUgLDYTvBBbjWjVLQ6VfOVsN
 DtEd4p5Y1PRfmV/i1vo/UygLLWnSeCavdiBUBBY4=
Date: Thu, 2 Nov 2023 08:36:05 -0700
From: Tyler Retzlaff <roretzla@linux.microsoft.com>
To: Morten =?iso-8859-1?Q?Br=F8rup?= <mb@smartsharesystems.com>
Cc: dev@dpdk.org, Bruce Richardson <bruce.richardson@intel.com>,
 Cristian Dumitrescu <cristian.dumitrescu@intel.com>,
 David Hunt <david.hunt@intel.com>,
 Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>,
 Ruifeng Wang <ruifeng.wang@arm.com>,
 Sameh Gobriel <sameh.gobriel@intel.com>,
 Vladimir Medvedkin <vladimir.medvedkin@intel.com>,
 Yipeng Wang <yipeng1.wang@intel.com>
Subject: Re: [PATCH 0/5] use abstracted bit count functions
Message-ID: <20231102153605.GB27149@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
References: <1698887132-5347-1-git-send-email-roretzla@linux.microsoft.com>
 <98CBD80474FA8B44BF855DF32C47DC35E9EFC6@smartserver.smartshare.dk>
 <20231102152737.GA27149@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
 <98CBD80474FA8B44BF855DF32C47DC35E9EFCA@smartserver.smartshare.dk>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35E9EFCA@smartserver.smartshare.dk>
User-Agent: Mutt/1.5.21 (2010-09-15)
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org

On Thu, Nov 02, 2023 at 04:33:57PM +0100, Morten Brørup wrote:
> > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> > Sent: Thursday, 2 November 2023 16.28
> > 
> > On Thu, Nov 02, 2023 at 08:39:04AM +0100, Morten Brørup wrote:
> > > > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> > > > Sent: Thursday, 2 November 2023 02.05
> > > >
> > > > The first set of conversions missed the long 'l' versions of the
> > > > builtins that were being used. This series completes the conversion
> > > > of remaining libraries from __builtin_ctzl and __builtin_clzl.
> > >
> > > NAK to blind search/replace of __builtin_clzl()/clzl().
> > >
> > > Although the size of long is 64 bit on 64 bit architectures, it only
> > 32 bit on 32 bit architectures.
> > >
> > > You need to look at the types these builtins operate on:
> > > - E.g. in the hash library (patch 3/5) prim_hitmask[i]/sec_hitmask[i]
> > are uint32_t, so rte_ctz32() would be the correct replacement. (I am
> > now asking myself why they were using __builtin_ctzl() instead of
> > __builtin_ctz() here... Probably by mistake.)
> > > - And if the type is "long", you need conditional compiling (or a
> > wrapper macro) to choose between the 32 bit or 64 bit variants.
> > >
> > > NB: You can blindly replace __builtin_ctzll()/clzll(), if any, by 64
> > bit functions.
> > 
> > they haven't been blindly replaced. but i would like you to validate my
> > thinking.
> > 
> > in the case of counting trailing 0s it seems fine if the type is
> > promoted to 64-bits,
> 
> This will give the correct result, yes. However the 64-bit operation might have a higher performance cost than the 32-bit operation, especially on 32-bit architectures.

true. okay let me clean this up. thanks for the feedback.

> 
> > in the case of leading i checked the type to make
> > sure it was already operating on a 64-bit type.
> 
> If already operating on a 64-bit type, using the 64-bit function is obviously correct.
> 
> > 
> > too naive?