Re: use rotate macro in more places

On Sat, 19 Feb 2022 20:07:58 +0700
John Naylor <john(dot)naylor(at)enterprisedb(dot)com> wrote:

> We've accumulated a few bit-twiddling hacks to get the compiler to
> emit a rotate instruction. Since we have a macro for that, let's use
> it, as in the attached. I thought the new call sites would look better
> with a "left" version, so I added a new macro for that. That's not
> necessary, however.
>
> Some comments now look a bit too obvious to keep around, but maybe
> they should be replaced with a "why", instead of a "what":
>
> /* rotate hashkey left 1 bit at each step */
> - hashkey = (hashkey << 1) | ((hashkey &
> 0x80000000) ? 1 : 0);
> + hashkey = pg_rotate_left32(hashkey, 1);

I think we can use this macro also in hash_multirange, hash_range,
and JsonbHashScalarValue as in the attached patch. How about replacing
them with the macro, too.

For avoiding undefined behaviours, maybe it is better to use unsigned
int and bit mask as a following code in Linux does[1][2], though it
would be unnecessary if they are used properly as in the current
PostgreSQL code.

static inline __u32 rol32(__u32 word, unsigned int shift)
{
return (word << (shift & 31)) | (word >> ((-shift) & 31));
}

[1] https://github.com/torvalds/linux/blob/master/include/linux/bitops.h
[2] https://lore.kernel.org/lkml/20190609164129(dot)126354143(at)linuxfoundation(dot)org/

Regards,
Yugo Nagata

>
>
> --
> John Naylor
> EDB: http://www.enterprisedb.com

--
Yugo NAGATA <nagata(at)sraoss(dot)co(dot)jp>