Re: [PATCH 02/17] bitops: Add generic parity calculation for u64
From: Kuan-Wei Chiu <visitorckw@gmail.com>
Date: 2025-02-26 17:59:56
Also in:
bpf, dri-devel, linux-media, linux-serial, linux-wireless, lkml, netdev
Hi Jiri, On Wed, Feb 26, 2025 at 08:14:14AM +0100, Jiri Slaby wrote:
On 25. 02. 25, 14:29, Kuan-Wei Chiu wrote:quoted
quoted
+#define parity(val) \ +({ \ + u64 __v = (val); \ + int __ret; \ + switch (BITS_PER_TYPE(val)) { \ + case 64: \ + __v ^= __v >> 32; \ + fallthrough; \ + case 32: \ + __v ^= __v >> 16; \ + fallthrough; \ + case 16: \ + __v ^= __v >> 8; \ + fallthrough; \ + case 8: \ + __v ^= __v >> 4; \ + __ret = (0x6996 >> (__v & 0xf)) & 1; \ + break; \ + default: \ + BUILD_BUG(); \ + } \ + __ret; \ +}) + +#define parity8(val) parity((u8)(val)) +#define parity32(val) parity((u32)(val)) +#define parity64(val) parity((u64)(val))What do you think about using these inline functions instead of macros? Except for parity8(), each function is a single line and follows the same logic. I find inline functions more readable, and coding-style.rst also recommends them over macros.Not in cases where macros are inevitable. I mean, do we need parityXX() for XX in (8, 16, 32, 64) at all? Isn't the parity() above enough for everybody? And if not, you can have all those parityXX() as inlines as you suggest, but also provide a macro such as the above to call (optimized) parityXX() as per datatype len.
I agree that we can add a macro to call parity8/16/32/64 based on the
data type size. However, I think we should still keep parity8/16/32/64.
As Peter and David discussed, the x86-specific implementations of
parity8() and parity16() might use different instructions instead of
just XORing and calling another function, as in the generic version.
My current idea is to follow David's suggestion and use
__builtin_parity when there is no architecture-specific implementation.
In lib/, we can provide a generic weak function implementation of
__parity[sdt]i2.
Any comments or suggestions are welcome!
Regards,
Kuan-Wei
static inline parity32(u32 val)
{
return __builtin_const_p(val) ? _parity_const(val) : _parity32(val);
}
#ifndef _parity32
static inline _parity32(u32 val)
{
return __builtin_parity(val);
}
#endif
int __weak __paritysi2(u32 val);
int __weak __paritysi2(u32 val)
{
val ^= val >> 16;
val ^= val >> 8;
val ^= val >> 4;
return (0x6996 >> (val & 0xf)) & 1;
}
EXPORT_SYMBOL(__paritysi2);