Re: use rotate macro in more places - Mailing list pgsql-hackers

From Yugo NAGATA
Subject Re: use rotate macro in more places
Date
Msg-id 20220220030127.32aa7b42330fe1372b9fc51f@sraoss.co.jp
Whole thread Raw
In response to use rotate macro in more places  (John Naylor <john.naylor@enterprisedb.com>)
Responses Re: use rotate macro in more places
List pgsql-hackers
On Sat, 19 Feb 2022 20:07:58 +0700
John Naylor <john.naylor@enterprisedb.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.126354143@linuxfoundation.org/

Regards,
Yugo Nagata

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


-- 
Yugo NAGATA <nagata@sraoss.co.jp>

Attachment

pgsql-hackers by date:

Previous
From: "David G. Johnston"
Date:
Subject: Re: Design of pg_stat_subscription_workers vs pgstats
Next
From: Andrew Dunstan
Date:
Subject: Re: killing perl2host