On Sun, Mar 02, 2025 at 07:09:54PM +0000, David Laight wrote:
> > #define parity(val) \
> > ({ \
> > __auto_type __v = (val); \
> > bool __ret; \
> > switch (BITS_PER_TYPE(val)) { \
> > case 64: \
> > __v ^= __v >> 16 >> 16; \
> > 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; \
> > })
>
> I'm seeing double-register shifts for 64bit values on 32bit systems.
> And gcc is doing 64bit double-register maths all the way down.
If you don't like GCC code generation why don't you discuss it in GCC
maillist?
> That is fixed by changing the top of the define to
> #define parity(val) \
> ({ \
> unsigned int __v = (val); \
> bool __ret; \
> switch (BITS_PER_TYPE(val)) { \
> case 64: \
> __v ^= val >> 16 >> 16; \
> fallthrough; \
>
> But it's need changing to only expand 'val' once.
> Perhaps:
> auto_type _val = (val);
> u32 __ret = val;
> and (mostly) s/__v/__ret/g
You suggest another complication to mitigate a problem that most
likely doesn't exist. I looked through the series and found that
parity64() is used for I2C, joystick and a netronome ethernet card.
For I2C and joystick performance is definitely not a problem. For
ethernet - maybe. But I feel like you didn't compile that driver
for any 32-bit arch, and didn't test with a real hardware. So your
concern is a pure speculation.
NAK.
