diff mbox

[dpdk-dev,v2,1/2] eal: add API to align integer to previous power of 2

Message ID 20180219113643.10337-1-pbhagavatula@caviumnetworks.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Pavan Nikhilesh Feb. 19, 2018, 11:36 a.m. UTC 
  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(+)

--
2.16.1

Comments

Matan Azrad Feb. 19, 2018, 12:09 p.m. UTC | #1 
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?
 

>  /**
>   * Aligns 64b input parameter to the next power of 2
>   *
> @@ -282,6 +303,28 @@ rte_align64pow2(uint64_t v)
>  	return v + 1;
>  }
> 
> +/**
> + * Aligns 64b input parameter to the previous power of 2
> + *
> + * @param v
> + *   The 64b value to align
> + *
> + * @return
> + *   Input parameter aligned to the previous power of 2
> + */
> +static inline uint64_t
> +rte_align64prevpow2(uint64_t v)
> +{
> +	v |= v >> 1;
> +	v |= v >> 2;
> +	v |= v >> 4;
> +	v |= v >> 8;
> +	v |= v >> 16;
> +	v |= v >> 32;
> +
> +	return v - (v >> 1);
> +}
> +
>  /*********** Macros for calculating min and max **********/
> 
>  /**
> --
> 2.16.1
Pavan Nikhilesh Feb. 26, 2018, 7:10 p.m. UTC | #2 
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

>
>
Matan Azrad Feb. 27, 2018, 7:30 p.m. UTC | #3 
Hi Pavan

From: Pavan Nikhilesh, Monday, February 26, 2018 9:10 PM
> Hi Matan,
> 
> On Mon, Feb 19, 2018 at 12:09:46PM +0000, Matan Azrad wrote:
> > 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.

Sure.

I'm suggesting next names:
rte_combine32ms1b(register uint32_t x)
rte_combine64ms1b(register uint64_t x)

The description may be something like:
combine the upper bits into the LSBs to construct a value with the same most significant 1 as x but all 1's under it.

I would add the register keyword for each variables in all the align functions here just to hint for the compiler that it's better to save this variables in register and not in memory.  

Matan.
> Thanks,
> Pavan
> 
> >
> >
diff mbox

Patch

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);
+}
+
 /**
  * Aligns 64b input parameter to the next power of 2
  *
@@ -282,6 +303,28 @@  rte_align64pow2(uint64_t v)
 	return v + 1;
 }

+/**
+ * Aligns 64b input parameter to the previous power of 2
+ *
+ * @param v
+ *   The 64b value to align
+ *
+ * @return
+ *   Input parameter aligned to the previous power of 2
+ */
+static inline uint64_t
+rte_align64prevpow2(uint64_t v)
+{
+	v |= v >> 1;
+	v |= v >> 2;
+	v |= v >> 4;
+	v |= v >> 8;
+	v |= v >> 16;
+	v |= v >> 32;
+
+	return v - (v >> 1);
+}
+
 /*********** Macros for calculating min and max **********/

 /**