diff mbox

odp_coremask.c: Use 1ULL for 64bit shifts

Message ID 1410913754021.77180@caviumnetworks.com
State New
Headers show

Commit Message

Rosenboim, Leonid Sept. 17, 2014, 12:29 a.m. UTC
?"1 << core" will fail if core >= 32, hence "1ull << core" is needed.

problem is, this will only work for core_id < 64, but that may not always be the case,

e.g. if there are more than 64 cores, or if core number are not contiguous.

Comments

Mike Holmes Sept. 17, 2014, 12:57 a.m. UTC | #1
When I am in the sweet spot between the drugs applied utopic delirium and
pain I can do things:)

Currently we populate a u64 so I agree we have hardcoded a limit of 64
processors so that should be a new bug - one I expect Tileria might bring
up soon :)
But as Leo says the code does need 1ULL as written according to the C
standard because a "1" defaults to 32 bits and we shift it up to 63


On 16 September 2014 20:29, Rosenboim, Leonid <
Leonid.Rosenboim@caviumnetworks.com> wrote:

>  ​"1 << core" will fail if core >= 32, hence "1ull << core" is needed.
>
> problem is, this will only work for core_id < 64, but that may not always
> be the case,
>
> e.g. if there are more than 64 cores, or if core number are not contiguous.
>
>
>  ------------------------------
> *From:* lng-odp-bounces@lists.linaro.org <lng-odp-bounces@lists.linaro.org>
> on behalf of Bill Fischofer <bill.fischofer@linaro.org>
> *Sent:* Tuesday, September 16, 2014 5:13 PM
> *To:* Mike Holmes
> *Cc:* lng-odp@lists.linaro.org
> *Subject:* Re: [lng-odp] [PATCH] odp_coremask.c: Use 1ULL for 64bit shifts
>
>  Is this necessary?  1 == 1ULL in this context.
>
>  Bill
>
>  PS Glad to see you're coherent/bored enough to be working :)
>
> On Tuesday, September 16, 2014, Mike Holmes <mike.holmes@linaro.org>
> wrote:
>
>> Signed-off-by: Mike Holmes <mike.holmes@linaro.org>
>> ---
>>
>> Unless specified "1" is taken as 32bits and core can be upto 63
>>
>>  platform/linux-generic/odp_coremask.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/platform/linux-generic/odp_coremask.c
>> b/platform/linux-generic/odp_coremask.c
>> index c55eb72..c7438cc 100644
>> --- a/platform/linux-generic/odp_coremask.c
>> +++ b/platform/linux-generic/odp_coremask.c
>> @@ -64,7 +64,7 @@ void odp_coremask_set(int core, odp_coremask_t *mask)
>>                 return;
>>         }
>>
>> -       mask->_u64[0] |=  (1 << core);
>> +       mask->_u64[0] |=  (1ULL << core);
>>  }
>>
>>  void odp_coremask_clr(int core, odp_coremask_t *mask)
>> @@ -77,7 +77,7 @@ void odp_coremask_clr(int core, odp_coremask_t *mask)
>>                 return;
>>         }
>>
>> -       mask->_u64[0] &= ~(1 << core);
>> +       mask->_u64[0] &= ~(1ULL << core);
>>  }
>>
>>
>> --
>> 1.9.1
>>
>>
>> _______________________________________________
>> lng-odp mailing list
>> lng-odp@lists.linaro.org
>> http://lists.linaro.org/mailman/listinfo/lng-odp
>>
>
Maxim Uvarov Sept. 22, 2014, 9:25 a.m. UTC | #2
On 09/17/2014 04:57 AM, Mike Holmes wrote:
> When I am in the sweet spot between the drugs applied utopic delirium 
> and pain I can do things:)
>
> Currently we populate a u64 so I agree we have hardcoded a limit of 64 
> processors so that should be a new bug - one I expect Tileria might 
> bring up soon :)
> But as Leo says the code does need 1ULL as written according to the C 
> standard because a "1" defaults to 32 bits and we shift it up to 63
>
yes, that is needed. gcc always use 1 as 32 bit and shift to higher then 
32 will fail.

Applying that patch.

Maxim.

>
> On 16 September 2014 20:29, Rosenboim, Leonid 
> <Leonid.Rosenboim@caviumnetworks.com 
> <mailto:Leonid.Rosenboim@caviumnetworks.com>> wrote:
>
>     ​"1 << core" will fail if core >= 32, hence "1ull << core" is needed.
>
>     problem is, this will only work for core_id < 64, but that may not
>     always be the case,
>
>     e.g. if there are more than 64 cores, or if core number are not
>     contiguous.
>
>
>     ------------------------------------------------------------------------
>     *From:* lng-odp-bounces@lists.linaro.org
>     <mailto:lng-odp-bounces@lists.linaro.org>
>     <lng-odp-bounces@lists.linaro.org
>     <mailto:lng-odp-bounces@lists.linaro.org>> on behalf of Bill
>     Fischofer <bill.fischofer@linaro.org
>     <mailto:bill.fischofer@linaro.org>>
>     *Sent:* Tuesday, September 16, 2014 5:13 PM
>     *To:* Mike Holmes
>     *Cc:* lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org>
>     *Subject:* Re: [lng-odp] [PATCH] odp_coremask.c: Use 1ULL for
>     64bit shifts
>     Is this necessary? 1 == 1ULL in this context.
>
>     Bill
>
>     PS Glad to see you're coherent/bored enough to be working :)
>
>     On Tuesday, September 16, 2014, Mike Holmes
>     <mike.holmes@linaro.org <mailto:mike.holmes@linaro.org>> wrote:
>
>         Signed-off-by: Mike Holmes <mike.holmes@linaro.org>
>         ---
>
>         Unless specified "1" is taken as 32bits and core can be upto 63
>
>         platform/linux-generic/odp_coremask.c | 4 ++--
>         1 file changed, 2 insertions(+), 2 deletions(-)
>
>         diff --git a/platform/linux-generic/odp_coremask.c
>         b/platform/linux-generic/odp_coremask.c
>         index c55eb72..c7438cc 100644
>         --- a/platform/linux-generic/odp_coremask.c
>         +++ b/platform/linux-generic/odp_coremask.c
>         @@ -64,7 +64,7 @@ void odp_coremask_set(int core,
>         odp_coremask_t *mask)
>         return;
>         }
>
>         - mask->_u64[0] |= (1 << core);
>         + mask->_u64[0] |= (1ULL << core);
>         }
>
>         void odp_coremask_clr(int core, odp_coremask_t *mask)
>         @@ -77,7 +77,7 @@ void odp_coremask_clr(int core,
>         odp_coremask_t *mask)
>         return;
>         }
>
>         - mask->_u64[0] &= ~(1 << core);
>         + mask->_u64[0] &= ~(1ULL << core);
>         }
>
>
>         --
>         1.9.1
>
>
>         _______________________________________________
>         lng-odp mailing list
>         lng-odp@lists.linaro.org
>         http://lists.linaro.org/mailman/listinfo/lng-odp
>
>
>
>
> -- 
> *Mike Holmes*
> Linaro Technical Manager / Lead
> LNG - ODP
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
diff mbox

Patch

diff --git a/platform/linux-generic/odp_coremask.c b/platform/linux-generic/odp_coremask.c
index c55eb72..c7438cc 100644
--- a/platform/linux-generic/odp_coremask.c
+++ b/platform/linux-generic/odp_coremask.c
@@ -64,7 +64,7 @@  void odp_coremask_set(int core, odp_coremask_t *mask)
                return;
        }

-       mask->_u64[0] |=  (1 << core);
+       mask->_u64[0] |=  (1ULL << core);
 }

 void odp_coremask_clr(int core, odp_coremask_t *mask)
@@ -77,7 +77,7 @@  void odp_coremask_clr(int core, odp_coremask_t *mask)
                return;
        }

-       mask->_u64[0] &= ~(1 << core);
+       mask->_u64[0] &= ~(1ULL << core);
 }