On 03/06/2019 11:02, Jan Beulich wrote:
>>>> On 31.05.19 at 21:40, <andrew.coop...@citrix.com> wrote:
>> On 31/05/2019 02:51, Jan Beulich wrote:
>>> --- a/xen/include/xen/bitops.h
>>> +++ b/xen/include/xen/bitops.h
>>> @@ -153,41 +153,54 @@ static __inline__ int get_count_order(un
>>>
>>> static inline unsigned int generic_hweight32(unsigned int w)
>>> {
>>> - unsigned int res = (w & 0x55555555) + ((w >> 1) & 0x55555555);
>>> - res = (res & 0x33333333) + ((res >> 2) & 0x33333333);
>>> - res = (res & 0x0F0F0F0F) + ((res >> 4) & 0x0F0F0F0F);
>>> - res = (res & 0x00FF00FF) + ((res >> 8) & 0x00FF00FF);
>>> - return (res & 0x0000FFFF) + ((res >> 16) & 0x0000FFFF);
>>> + w -= (w >> 1) & 0x55555555;
>>> + w = (w & 0x33333333) + ((w >> 2) & 0x33333333);
>>> + w = (w + (w >> 4)) & 0x0f0f0f0f;
>>> +
>>> +#ifdef CONFIG_HAS_FAST_MULTIPLY
>>> + return (w * 0x01010101) >> 24;
>>> +#else
>>> + w += w >> 8;
>>> +
>>> + return (w + (w >> 16)) & 0xff;
>>> +#endif
>> This would be slightly shorter, less liable to bitrot, and IMO cleaner,
>> to do
>>
>> if ( IS_ENABLED(CONFIG_HAS_FAST_MULTIPLY) )
>> w = w * 0x01010101) >> 24;
>> else
>> w += w >> 8;
>>
>> return w;
> Would you be okay with
>
> static inline unsigned int generic_hweight32(unsigned int w)
> {
> w -= (w >> 1) & 0x55555555;
> w = (w & 0x33333333) + ((w >> 2) & 0x33333333);
> w = (w + (w >> 4)) & 0x0f0f0f0f;
>
> if ( IS_ENABLED(CONFIG_HAS_FAST_MULTIPLY) )
> return (w * 0x01010101) >> 24;
>
> w += w >> 8;
>
> return (w + (w >> 16)) & 0xff;
> }
>
> despite there then still being two return statements?
Yeah - this form does read like a regular function.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
