diff mbox

[PATCHv2,1/3] linux-generic: buffers: expand ODP_BITSIZE macro for u64 use

Message ID 1420998638-23980-2-git-send-email-bill.fischofer@linaro.org
State New
Headers show

Commit Message

Bill Fischofer Jan. 11, 2015, 5:50 p.m. UTC 
Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>
---
 .../linux-generic/include/odp_buffer_internal.h    | 78 +++++++++++++++++-----
 1 file changed, 60 insertions(+), 18 deletions(-)

Comments

Taras Kondratiuk Jan. 13, 2015, 9:22 p.m. UTC | #1 
On 01/11/2015 07:50 PM, Bill Fischofer wrote:
> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>
> ---
>  .../linux-generic/include/odp_buffer_internal.h    | 78 +++++++++++++++++-----
>  1 file changed, 60 insertions(+), 18 deletions(-)
> 
> diff --git a/platform/linux-generic/include/odp_buffer_internal.h b/platform/linux-generic/include/odp_buffer_internal.h
> index 39b0b05..b8faf63 100644
> --- a/platform/linux-generic/include/odp_buffer_internal.h
> +++ b/platform/linux-generic/include/odp_buffer_internal.h
> @@ -29,25 +29,67 @@ extern "C" {
>  #include <odp_byteorder.h>
>  #include <odp_thread.h>
>  
> -
>  #define ODP_BITSIZE(x) \
> -	((x) <=     2 ?  1 : \
> -	((x) <=     4 ?  2 : \
> -	((x) <=     8 ?  3 : \
> -	((x) <=    16 ?  4 : \
> -	((x) <=    32 ?  5 : \
> -	((x) <=    64 ?  6 : \
> -	((x) <=   128 ?  7 : \
> -	((x) <=   256 ?  8 : \
> -	((x) <=   512 ?  9 : \
> -	((x) <=  1024 ? 10 : \
> -	((x) <=  2048 ? 11 : \
> -	((x) <=  4096 ? 12 : \
> -	((x) <=  8196 ? 13 : \
> -	((x) <= 16384 ? 14 : \
> -	((x) <= 32768 ? 15 : \
> -	((x) <= 65536 ? 16 : \
> -	 (0/0)))))))))))))))))
> +	((x) <= (1ULL <<  1) ?  1 : \
> +	((x) <= (1ULL <<  2) ?  2 : \
> +	((x) <= (1ULL <<  3) ?  3 : \
> +	((x) <= (1ULL <<  4) ?  4 : \
> +	((x) <= (1ULL <<  5) ?  5 : \
> +	((x) <= (1ULL <<  6) ?  6 : \
> +	((x) <= (1ULL <<  7) ?  7 : \
> +	((x) <= (1ULL <<  8) ?  8 : \
> +	((x) <= (1ULL <<  9) ?  9 : \
> +	((x) <= (1ULL << 10) ? 10 : \
> +	((x) <= (1ULL << 11) ? 11 : \
> +	((x) <= (1ULL << 12) ? 12 : \
> +	((x) <= (1ULL << 13) ? 13 : \
> +	((x) <= (1ULL << 14) ? 14 : \
> +	((x) <= (1ULL << 15) ? 15 : \
> +	((x) <= (1ULL << 16) ? 16 : \
> +	((x) <= (1ULL << 17) ? 17 : \
> +	((x) <= (1ULL << 18) ? 18 : \
> +	((x) <= (1ULL << 19) ? 19 : \
> +	((x) <= (1ULL << 20) ? 20 : \
> +	((x) <= (1ULL << 21) ? 21 : \
> +	((x) <= (1ULL << 22) ? 22 : \
> +	((x) <= (1ULL << 23) ? 23 : \
> +	((x) <= (1ULL << 24) ? 24 : \
> +	((x) <= (1ULL << 25) ? 25 : \
> +	((x) <= (1ULL << 26) ? 26 : \
> +	((x) <= (1ULL << 27) ? 27 : \
> +	((x) <= (1ULL << 28) ? 28 : \
> +	((x) <= (1ULL << 29) ? 29 : \
> +	((x) <= (1ULL << 30) ? 30 : \
> +	((x) <= (1ULL << 31) ? 31 : \
> +	((x) <= (1ULL << 32) ? 32 : \
> +	((x) <= (1ULL << 33) ? 33 : \
> +	((x) <= (1ULL << 34) ? 34 : \
> +	((x) <= (1ULL << 35) ? 35 : \
> +	((x) <= (1ULL << 36) ? 36 : \
> +	((x) <= (1ULL << 37) ? 37 : \
> +	((x) <= (1ULL << 38) ? 38 : \
> +	((x) <= (1ULL << 39) ? 39 : \
> +	((x) <= (1ULL << 40) ? 40 : \
> +	((x) <= (1ULL << 41) ? 41 : \
> +	((x) <= (1ULL << 42) ? 42 : \
> +	((x) <= (1ULL << 43) ? 43 : \
> +	((x) <= (1ULL << 44) ? 44 : \
> +	((x) <= (1ULL << 45) ? 45 : \
> +	((x) <= (1ULL << 46) ? 46 : \
> +	((x) <= (1ULL << 47) ? 47 : \
> +	((x) <= (1ULL << 48) ? 48 : \
> +	((x) <= (1ULL << 49) ? 49 : \
> +	((x) <= (1ULL << 50) ? 50 : \
> +	((x) <= (1ULL << 51) ? 51 : \
> +	((x) <= (1ULL << 52) ? 52 : \
> +	((x) <= (1ULL << 53) ? 53 : \
> +	((x) <= (1ULL << 54) ? 54 : \
> +	((x) <= (1ULL << 55) ? 55 : \
> +	((x) <= (1ULL << 56) ? 56 : \
> +	((x) <= (1ULL << 57) ? 57 : \
> +	((x) <= (1ULL << 58) ? 58 : \
> +	((x) <= (1ULL << 59) ? 59 : \
> +	 (0/0))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))

That is ugly.
Can you try built-in clz function instead? Something like:

#define ODP_BITSIZE(x) (8 * sizeof(long long) - __builtin_clzll(x - 1))
Bill Fischofer Jan. 13, 2015, 9:28 p.m. UTC | #2 
It's just extending the existing code.  It's evaluated at compile time and
is pure C and hence compiler/architecture independent.  Other solutions
would seem to have portability concerns and/or generate runtime code.

On Tue, Jan 13, 2015 at 3:22 PM, Taras Kondratiuk <
taras.kondratiuk@linaro.org> wrote:

> On 01/11/2015 07:50 PM, Bill Fischofer wrote:
> > Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>
> > ---
> >  .../linux-generic/include/odp_buffer_internal.h    | 78
> +++++++++++++++++-----
> >  1 file changed, 60 insertions(+), 18 deletions(-)
> >
> > diff --git a/platform/linux-generic/include/odp_buffer_internal.h
> b/platform/linux-generic/include/odp_buffer_internal.h
> > index 39b0b05..b8faf63 100644
> > --- a/platform/linux-generic/include/odp_buffer_internal.h
> > +++ b/platform/linux-generic/include/odp_buffer_internal.h
> > @@ -29,25 +29,67 @@ extern "C" {
> >  #include <odp_byteorder.h>
> >  #include <odp_thread.h>
> >
> > -
> >  #define ODP_BITSIZE(x) \
> > -     ((x) <=     2 ?  1 : \
> > -     ((x) <=     4 ?  2 : \
> > -     ((x) <=     8 ?  3 : \
> > -     ((x) <=    16 ?  4 : \
> > -     ((x) <=    32 ?  5 : \
> > -     ((x) <=    64 ?  6 : \
> > -     ((x) <=   128 ?  7 : \
> > -     ((x) <=   256 ?  8 : \
> > -     ((x) <=   512 ?  9 : \
> > -     ((x) <=  1024 ? 10 : \
> > -     ((x) <=  2048 ? 11 : \
> > -     ((x) <=  4096 ? 12 : \
> > -     ((x) <=  8196 ? 13 : \
> > -     ((x) <= 16384 ? 14 : \
> > -     ((x) <= 32768 ? 15 : \
> > -     ((x) <= 65536 ? 16 : \
> > -      (0/0)))))))))))))))))
> > +     ((x) <= (1ULL <<  1) ?  1 : \
> > +     ((x) <= (1ULL <<  2) ?  2 : \
> > +     ((x) <= (1ULL <<  3) ?  3 : \
> > +     ((x) <= (1ULL <<  4) ?  4 : \
> > +     ((x) <= (1ULL <<  5) ?  5 : \
> > +     ((x) <= (1ULL <<  6) ?  6 : \
> > +     ((x) <= (1ULL <<  7) ?  7 : \
> > +     ((x) <= (1ULL <<  8) ?  8 : \
> > +     ((x) <= (1ULL <<  9) ?  9 : \
> > +     ((x) <= (1ULL << 10) ? 10 : \
> > +     ((x) <= (1ULL << 11) ? 11 : \
> > +     ((x) <= (1ULL << 12) ? 12 : \
> > +     ((x) <= (1ULL << 13) ? 13 : \
> > +     ((x) <= (1ULL << 14) ? 14 : \
> > +     ((x) <= (1ULL << 15) ? 15 : \
> > +     ((x) <= (1ULL << 16) ? 16 : \
> > +     ((x) <= (1ULL << 17) ? 17 : \
> > +     ((x) <= (1ULL << 18) ? 18 : \
> > +     ((x) <= (1ULL << 19) ? 19 : \
> > +     ((x) <= (1ULL << 20) ? 20 : \
> > +     ((x) <= (1ULL << 21) ? 21 : \
> > +     ((x) <= (1ULL << 22) ? 22 : \
> > +     ((x) <= (1ULL << 23) ? 23 : \
> > +     ((x) <= (1ULL << 24) ? 24 : \
> > +     ((x) <= (1ULL << 25) ? 25 : \
> > +     ((x) <= (1ULL << 26) ? 26 : \
> > +     ((x) <= (1ULL << 27) ? 27 : \
> > +     ((x) <= (1ULL << 28) ? 28 : \
> > +     ((x) <= (1ULL << 29) ? 29 : \
> > +     ((x) <= (1ULL << 30) ? 30 : \
> > +     ((x) <= (1ULL << 31) ? 31 : \
> > +     ((x) <= (1ULL << 32) ? 32 : \
> > +     ((x) <= (1ULL << 33) ? 33 : \
> > +     ((x) <= (1ULL << 34) ? 34 : \
> > +     ((x) <= (1ULL << 35) ? 35 : \
> > +     ((x) <= (1ULL << 36) ? 36 : \
> > +     ((x) <= (1ULL << 37) ? 37 : \
> > +     ((x) <= (1ULL << 38) ? 38 : \
> > +     ((x) <= (1ULL << 39) ? 39 : \
> > +     ((x) <= (1ULL << 40) ? 40 : \
> > +     ((x) <= (1ULL << 41) ? 41 : \
> > +     ((x) <= (1ULL << 42) ? 42 : \
> > +     ((x) <= (1ULL << 43) ? 43 : \
> > +     ((x) <= (1ULL << 44) ? 44 : \
> > +     ((x) <= (1ULL << 45) ? 45 : \
> > +     ((x) <= (1ULL << 46) ? 46 : \
> > +     ((x) <= (1ULL << 47) ? 47 : \
> > +     ((x) <= (1ULL << 48) ? 48 : \
> > +     ((x) <= (1ULL << 49) ? 49 : \
> > +     ((x) <= (1ULL << 50) ? 50 : \
> > +     ((x) <= (1ULL << 51) ? 51 : \
> > +     ((x) <= (1ULL << 52) ? 52 : \
> > +     ((x) <= (1ULL << 53) ? 53 : \
> > +     ((x) <= (1ULL << 54) ? 54 : \
> > +     ((x) <= (1ULL << 55) ? 55 : \
> > +     ((x) <= (1ULL << 56) ? 56 : \
> > +     ((x) <= (1ULL << 57) ? 57 : \
> > +     ((x) <= (1ULL << 58) ? 58 : \
> > +     ((x) <= (1ULL << 59) ? 59 : \
> > +      (0/0))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))
>
> That is ugly.
> Can you try built-in clz function instead? Something like:
>
> #define ODP_BITSIZE(x) (8 * sizeof(long long) - __builtin_clzll(x - 1))
>
> --
> Taras Kondratiuk
>
Taras Kondratiuk Jan. 13, 2015, 9:55 p.m. UTC | #3 
On 01/13/2015 11:28 PM, Bill Fischofer wrote:
> It's just extending the existing code.  It's evaluated at compile time
> and is pure C and hence compiler/architecture independent.  Other
> solutions would seem to have portability concerns and/or generate
> runtime code.

Yes this is gcc specific, but most probably clang have something
similar.
diff mbox

Patch

diff --git a/platform/linux-generic/include/odp_buffer_internal.h b/platform/linux-generic/include/odp_buffer_internal.h
index 39b0b05..b8faf63 100644
--- a/platform/linux-generic/include/odp_buffer_internal.h
+++ b/platform/linux-generic/include/odp_buffer_internal.h
@@ -29,25 +29,67 @@  extern "C" {
 #include <odp_byteorder.h>
 #include <odp_thread.h>
 
-
 #define ODP_BITSIZE(x) \
-	((x) <=     2 ?  1 : \
-	((x) <=     4 ?  2 : \
-	((x) <=     8 ?  3 : \
-	((x) <=    16 ?  4 : \
-	((x) <=    32 ?  5 : \
-	((x) <=    64 ?  6 : \
-	((x) <=   128 ?  7 : \
-	((x) <=   256 ?  8 : \
-	((x) <=   512 ?  9 : \
-	((x) <=  1024 ? 10 : \
-	((x) <=  2048 ? 11 : \
-	((x) <=  4096 ? 12 : \
-	((x) <=  8196 ? 13 : \
-	((x) <= 16384 ? 14 : \
-	((x) <= 32768 ? 15 : \
-	((x) <= 65536 ? 16 : \
-	 (0/0)))))))))))))))))
+	((x) <= (1ULL <<  1) ?  1 : \
+	((x) <= (1ULL <<  2) ?  2 : \
+	((x) <= (1ULL <<  3) ?  3 : \
+	((x) <= (1ULL <<  4) ?  4 : \
+	((x) <= (1ULL <<  5) ?  5 : \
+	((x) <= (1ULL <<  6) ?  6 : \
+	((x) <= (1ULL <<  7) ?  7 : \
+	((x) <= (1ULL <<  8) ?  8 : \
+	((x) <= (1ULL <<  9) ?  9 : \
+	((x) <= (1ULL << 10) ? 10 : \
+	((x) <= (1ULL << 11) ? 11 : \
+	((x) <= (1ULL << 12) ? 12 : \
+	((x) <= (1ULL << 13) ? 13 : \
+	((x) <= (1ULL << 14) ? 14 : \
+	((x) <= (1ULL << 15) ? 15 : \
+	((x) <= (1ULL << 16) ? 16 : \
+	((x) <= (1ULL << 17) ? 17 : \
+	((x) <= (1ULL << 18) ? 18 : \
+	((x) <= (1ULL << 19) ? 19 : \
+	((x) <= (1ULL << 20) ? 20 : \
+	((x) <= (1ULL << 21) ? 21 : \
+	((x) <= (1ULL << 22) ? 22 : \
+	((x) <= (1ULL << 23) ? 23 : \
+	((x) <= (1ULL << 24) ? 24 : \
+	((x) <= (1ULL << 25) ? 25 : \
+	((x) <= (1ULL << 26) ? 26 : \
+	((x) <= (1ULL << 27) ? 27 : \
+	((x) <= (1ULL << 28) ? 28 : \
+	((x) <= (1ULL << 29) ? 29 : \
+	((x) <= (1ULL << 30) ? 30 : \
+	((x) <= (1ULL << 31) ? 31 : \
+	((x) <= (1ULL << 32) ? 32 : \
+	((x) <= (1ULL << 33) ? 33 : \
+	((x) <= (1ULL << 34) ? 34 : \
+	((x) <= (1ULL << 35) ? 35 : \
+	((x) <= (1ULL << 36) ? 36 : \
+	((x) <= (1ULL << 37) ? 37 : \
+	((x) <= (1ULL << 38) ? 38 : \
+	((x) <= (1ULL << 39) ? 39 : \
+	((x) <= (1ULL << 40) ? 40 : \
+	((x) <= (1ULL << 41) ? 41 : \
+	((x) <= (1ULL << 42) ? 42 : \
+	((x) <= (1ULL << 43) ? 43 : \
+	((x) <= (1ULL << 44) ? 44 : \
+	((x) <= (1ULL << 45) ? 45 : \
+	((x) <= (1ULL << 46) ? 46 : \
+	((x) <= (1ULL << 47) ? 47 : \
+	((x) <= (1ULL << 48) ? 48 : \
+	((x) <= (1ULL << 49) ? 49 : \
+	((x) <= (1ULL << 50) ? 50 : \
+	((x) <= (1ULL << 51) ? 51 : \
+	((x) <= (1ULL << 52) ? 52 : \
+	((x) <= (1ULL << 53) ? 53 : \
+	((x) <= (1ULL << 54) ? 54 : \
+	((x) <= (1ULL << 55) ? 55 : \
+	((x) <= (1ULL << 56) ? 56 : \
+	((x) <= (1ULL << 57) ? 57 : \
+	((x) <= (1ULL << 58) ? 58 : \
+	((x) <= (1ULL << 59) ? 59 : \
+	 (0/0))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))
 
 _ODP_STATIC_ASSERT(ODP_CONFIG_PACKET_BUF_LEN_MIN >= 256,
 		  "ODP Segment size must be a minimum of 256 bytes");