Use bitwise rotate functions in more places

There were a number of places in the code that used bespoke bit-twiddling
expressions to do bitwise rotation. While we've had pg_rotate_right32()
for a while now, we hadn't gotten around to standardizing on that. Do so
now. Since many potential call sites look more natural with the "left"
equivalent, add that function too.

Reviewed by Tom Lane and Yugo Nagata

Discussion:
https://www.postgresql.org/message-id/CAFBsxsH7c1LC0CGZ0ADCBXLHU5-%3DKNXx-r7tHYPAW51b2HK4Qw%40mail.gmail.com

diff --git a/src/backend/executor/execGrouping.c b/src/backend/executor/execGrouping.c
index af6e9c42d816e6a319f325cc667c1e6c68b47907..5da4b3753006111a8d417aee23a55b7a0efb2d6a 100644 (file)
--- a/src/backend/executor/execGrouping.c
+++ b/src/backend/executor/execGrouping.c
@@ -459,8 +459,8 @@ TupleHashTableHash_internal(struct tuplehash_hash *tb,
        Datum       attr;
        bool        isNull;
 
-       /* rotate hashkey left 1 bit at each step */
-       hashkey = (hashkey << 1) | ((hashkey & 0x80000000) ? 1 : 0);
+       /* combine successive hashkeys by rotating */
+       hashkey = pg_rotate_left32(hashkey, 1);
 
        attr = slot_getattr(slot, att, &isNull);
 

diff --git a/src/backend/executor/nodeHash.c b/src/backend/executor/nodeHash.c
index 4d68a8b97b79be2888456302295241d9792867d8..3510a4247c1d1e7fe5d28a0ca205d5f143b67a43 100644 (file)
--- a/src/backend/executor/nodeHash.c
+++ b/src/backend/executor/nodeHash.c
@@ -1840,8 +1840,8 @@ ExecHashGetHashValue(HashJoinTable hashtable,
        Datum       keyval;
        bool        isNull;
 
-       /* rotate hashkey left 1 bit at each step */
-       hashkey = (hashkey << 1) | ((hashkey & 0x80000000) ? 1 : 0);
+       /* combine successive hashkeys by rotating */
+       hashkey = pg_rotate_left32(hashkey, 1);
 
        /*
         * Get the join attribute value of the tuple

diff --git a/src/backend/executor/nodeMemoize.c b/src/backend/executor/nodeMemoize.c
index 55cdd5c4d98632cc121f143857b796730a0d8b04..23441e33cad54f54f0716761c4d0861e165eafc9 100644 (file)
--- a/src/backend/executor/nodeMemoize.c
+++ b/src/backend/executor/nodeMemoize.c
@@ -166,8 +166,8 @@ MemoizeHash_hash(struct memoize_hash *tb, const MemoizeKey *key)
    {
        for (int i = 0; i < numkeys; i++)
        {
-           /* rotate hashkey left 1 bit at each step */
-           hashkey = (hashkey << 1) | ((hashkey & 0x80000000) ? 1 : 0);
+           /* combine successive hashkeys by rotating */
+           hashkey = pg_rotate_left32(hashkey, 1);
 
            if (!pslot->tts_isnull[i])  /* treat nulls as having hash key 0 */
            {
@@ -189,8 +189,8 @@ MemoizeHash_hash(struct memoize_hash *tb, const MemoizeKey *key)
 
        for (int i = 0; i < numkeys; i++)
        {
-           /* rotate hashkey left 1 bit at each step */
-           hashkey = (hashkey << 1) | ((hashkey & 0x80000000) ? 1 : 0);
+           /* combine successive hashkeys by rotating */
+           hashkey = pg_rotate_left32(hashkey, 1);
 
            if (!pslot->tts_isnull[i])  /* treat nulls as having hash key 0 */
            {

diff --git a/src/backend/utils/adt/jsonb_util.c b/src/backend/utils/adt/jsonb_util.c
index 291fb722e2ded86d3cb47eec110470962f6e8ac8..60442758b3279d025379e229f95bf0382425b81f 100644 (file)
--- a/src/backend/utils/adt/jsonb_util.c
+++ b/src/backend/utils/adt/jsonb_util.c
@@ -18,6 +18,7 @@
 #include "common/hashfn.h"
 #include "common/jsonapi.h"
 #include "miscadmin.h"
+#include "port/pg_bitutils.h"
 #include "utils/builtins.h"
 #include "utils/datetime.h"
 #include "utils/json.h"
@@ -1342,7 +1343,7 @@ JsonbHashScalarValue(const JsonbValue *scalarVal, uint32 *hash)
     * the previous value left 1 bit, then XOR'ing in the new
     * key/value/element's hash value.
     */
-   *hash = (*hash << 1) | (*hash >> 31);
+   *hash = pg_rotate_left32(*hash, 1);
    *hash ^= tmp;
 }
 

diff --git a/src/backend/utils/adt/multirangetypes.c b/src/backend/utils/adt/multirangetypes.c
index 7b86421465e6158e9c14841e2ea5d638299a65ad..c474b244316e53d1d43ce700e88ea165b4ed5a94 100644 (file)
--- a/src/backend/utils/adt/multirangetypes.c
+++ b/src/backend/utils/adt/multirangetypes.c
@@ -38,6 +38,7 @@
 #include "lib/stringinfo.h"
 #include "libpq/pqformat.h"
 #include "miscadmin.h"
+#include "port/pg_bitutils.h"
 #include "utils/builtins.h"
 #include "utils/lsyscache.h"
 #include "utils/rangetypes.h"
@@ -2772,7 +2773,7 @@ hash_multirange(PG_FUNCTION_ARGS)
        /* Merge hashes of flags and bounds */
        range_hash = hash_uint32((uint32) flags);
        range_hash ^= lower_hash;
-       range_hash = (range_hash << 1) | (range_hash >> 31);
+       range_hash = pg_rotate_left32(range_hash, 1);
        range_hash ^= upper_hash;
 
        /*

diff --git a/src/backend/utils/adt/rangetypes.c b/src/backend/utils/adt/rangetypes.c
index c3e6c721e64538d6fcaf5a4d83cdf059281bd262..cbff4e93d5c63fb9f53bf396f5bc419910a78998 100644 (file)
--- a/src/backend/utils/adt/rangetypes.c
+++ b/src/backend/utils/adt/rangetypes.c
@@ -35,6 +35,7 @@
 #include "lib/stringinfo.h"
 #include "libpq/pqformat.h"
 #include "miscadmin.h"
+#include "port/pg_bitutils.h"
 #include "utils/builtins.h"
 #include "utils/date.h"
 #include "utils/lsyscache.h"
@@ -1363,7 +1364,7 @@ hash_range(PG_FUNCTION_ARGS)
    /* Merge hashes of flags and bounds */
    result = hash_uint32((uint32) flags);
    result ^= lower_hash;
-   result = (result << 1) | (result >> 31);
+   result = pg_rotate_left32(result, 1);
    result ^= upper_hash;
 
    PG_RETURN_INT32(result);

diff --git a/src/backend/utils/cache/catcache.c b/src/backend/utils/cache/catcache.c
index eb830880893733c13beec1c1820b038cc4ed4a26..ec073e1ed061ead28a5e8c74c7f830ee501d7e98 100644 (file)
--- a/src/backend/utils/cache/catcache.c
+++ b/src/backend/utils/cache/catcache.c
@@ -26,6 +26,7 @@
 #include "catalog/pg_type.h"
 #include "common/hashfn.h"
 #include "miscadmin.h"
+#include "port/pg_bitutils.h"
 #ifdef CATCACHE_STATS
 #include "storage/ipc.h"       /* for on_proc_exit */
 #endif
@@ -281,25 +282,18 @@ CatalogCacheComputeHashValue(CatCache *cache, int nkeys,
    {
        case 4:
            oneHash = (cc_hashfunc[3]) (v4);
-
-           hashValue ^= oneHash << 24;
-           hashValue ^= oneHash >> 8;
+           hashValue ^= pg_rotate_left32(oneHash, 24);
            /* FALLTHROUGH */
        case 3:
            oneHash = (cc_hashfunc[2]) (v3);
-
-           hashValue ^= oneHash << 16;
-           hashValue ^= oneHash >> 16;
+           hashValue ^= pg_rotate_left32(oneHash, 16);
            /* FALLTHROUGH */
        case 2:
            oneHash = (cc_hashfunc[1]) (v2);
-
-           hashValue ^= oneHash << 8;
-           hashValue ^= oneHash >> 24;
+           hashValue ^= pg_rotate_left32(oneHash, 8);
            /* FALLTHROUGH */
        case 1:
            oneHash = (cc_hashfunc[0]) (v1);
-
            hashValue ^= oneHash;
            break;
        default:

diff --git a/src/common/hashfn.c b/src/common/hashfn.c
index b7a322073d0af4a9e2d3d356146887d05dcb5d31..8779575b991cf7c7e160cb8d5c7777ecb316af67 100644 (file)
--- a/src/common/hashfn.c
+++ b/src/common/hashfn.c
@@ -24,6 +24,7 @@
 #include "postgres.h"
 
 #include "common/hashfn.h"
+#include "port/pg_bitutils.h"
 
 
 /*
@@ -44,8 +45,7 @@
 /* Get a bit mask of the bits set in non-uint32 aligned addresses */
 #define UINT32_ALIGN_MASK (sizeof(uint32) - 1)
 
-/* Rotate a uint32 value left by k bits - note multiple evaluation! */
-#define rot(x,k) (((x)<<(k)) | ((x)>>(32-(k))))
+#define rot(x,k) pg_rotate_left32(x, k)
 
 /*----------
  * mix -- mix 3 32-bit values reversibly.

diff --git a/src/include/port/pg_bitutils.h b/src/include/port/pg_bitutils.h
index 44c74fb974334b88c78dbca44bfd63e41a3c1292..04e58cd1c442c81a759930860b72ece558da99a7 100644 (file)
--- a/src/include/port/pg_bitutils.h
+++ b/src/include/port/pg_bitutils.h
@@ -285,12 +285,18 @@ extern int    pg_popcount64(uint64 word);
 extern uint64 pg_popcount(const char *buf, int bytes);
 
 /*
- * Rotate the bits of "word" to the right by n bits.
+ * Rotate the bits of "word" to the right/left by n bits.
  */
 static inline uint32
 pg_rotate_right32(uint32 word, int n)
 {
-   return (word >> n) | (word << (sizeof(word) * BITS_PER_BYTE - n));
+   return (word >> n) | (word << (32 - n));
+}
+
+static inline uint32
+pg_rotate_left32(uint32 word, int n)
+{
+   return (word << n) | (word >> (32 - n));
 }
 
 #endif                         /* PG_BITUTILS_H */