From mboxrd@z Thu Jan  1 00:00:00 1970
From: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
Subject: Re: [PATCH v2 1/2] eal: add API to align integer to
 previous power of 2
Date: Tue, 27 Feb 2018 00:40:18 +0530
Message-ID: <20180226191017.GA9950@ltp-pvn>
References: <20180217104934.17291-1-pbhagavatula@caviumnetworks.com>
 <20180219113643.10337-1-pbhagavatula@caviumnetworks.com>
 <AM4PR0501MB26573D7AD80F83C590A8297FD2C80@AM4PR0501MB2657.eurprd05.prod.outlook.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: dev@dpdk.org
To: Matan Azrad <matan@mellanox.com>,
 "jerin.jacob@caviumnetworks.com" <jerin.jacob@caviumnetworks.com>,
 "keith.wiles@intel.com" <keith.wiles@intel.com>,
 Thomas Monjalon <thomas@monjalon.net>
Return-path: <dev-bounces@dpdk.org>
Received: from NAM02-BL2-obe.outbound.protection.outlook.com
 (mail-bl2nam02on0070.outbound.protection.outlook.com [104.47.38.70])
 by dpdk.org (Postfix) with ESMTP id 907051D7
 for <dev@dpdk.org>; Mon, 26 Feb 2018 20:10:42 +0100 (CET)
Content-Disposition: inline
In-Reply-To: <AM4PR0501MB26573D7AD80F83C590A8297FD2C80@AM4PR0501MB2657.eurprd05.prod.outlook.com>
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

Hi Matan,

On Mon, Feb 19, 2018 at 12:09:46PM +0000, Matan Azrad wrote:
>
> Hi Pavan
> > From: Pavan Nikhilesh, Monday, February 19, 2018 1:37 PM
> > Add 32b and 64b API's to align the given integer to the previous power of 2.
> >
> > Signed-off-by: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
> > ---
> >  v2 Changes:
> >  - Modified api name to `rte_align(32/64)prevpow2` from
> > `rte_align(32/64)lowpow2`.
> >  - corrected fuction to return if the integer is already aligned to  power of 2.
> >
> >  lib/librte_eal/common/include/rte_common.h | 43
> > ++++++++++++++++++++++++++++++
> >  1 file changed, 43 insertions(+)
> >
> > diff --git a/lib/librte_eal/common/include/rte_common.h
> > b/lib/librte_eal/common/include/rte_common.h
> > index c7803e41c..b2017ee5c 100644
> > --- a/lib/librte_eal/common/include/rte_common.h
> > +++ b/lib/librte_eal/common/include/rte_common.h
> > @@ -259,6 +259,27 @@ rte_align32pow2(uint32_t x)
> >  	return x + 1;
> >  }
> >
> > +/**
> > + * Aligns input parameter to the previous power of 2
> > + *
> > + * @param x
> > + *   The integer value to algin
> > + *
> > + * @return
> > + *   Input parameter aligned to the previous power of 2
> > + */
> > +static inline uint32_t
> > +rte_align32prevpow2(uint32_t x)
> > +{
> > +	x |= x >> 1;
> > +	x |= x >> 2;
> > +	x |= x >> 4;
> > +	x |= x >> 8;
> > +	x |= x >> 16;
> > +
> > +	return x - (x >> 1);
> > +}
>
> Nice.
>
> Since you are using the same 5 lines from the rte_align32pow2() function, I think this part can be in a separate function to do reuse.
> Also the "fill ones 32" function can be used for other purpose.
> What do you think?

I do agree that it would be cleaner to have a common function for both, but not
able to decide on a appropriate function name "fill ones 32" doesn't convey
what the function truly does. If you have a cleaner name do suggest, i will
roll up a v3 adding the function and experimental tag.

Thanks,
Pavan

>
>