From 4b35408f1ed59dd590f683ae0f015bbaf3b84d3d Mon Sep 17 00:00:00 2001 From: John Naylor Date: Sun, 20 Feb 2022 13:22:08 +0700 Subject: [PATCH] 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 --- src/backend/executor/execGrouping.c | 4 ++-- src/backend/executor/nodeHash.c | 4 ++-- src/backend/executor/nodeMemoize.c | 8 ++++---- src/backend/utils/adt/jsonb_util.c | 3 ++- src/backend/utils/adt/multirangetypes.c | 3 ++- src/backend/utils/adt/rangetypes.c | 3 ++- src/backend/utils/cache/catcache.c | 14 ++++---------- src/common/hashfn.c | 4 ++-- src/include/port/pg_bitutils.h | 10 ++++++++-- 9 files changed, 28 insertions(+), 25 deletions(-) diff --git a/src/backend/executor/execGrouping.c b/src/backend/executor/execGrouping.c index af6e9c42d8..5da4b37530 100644 --- 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 4d68a8b97b..3510a4247c 100644 --- 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 55cdd5c4d9..23441e33ca 100644 --- 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 291fb722e2..60442758b3 100644 --- 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 7b86421465..c474b24431 100644 --- 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 c3e6c721e6..cbff4e93d5 100644 --- 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 eb83088089..ec073e1ed0 100644 --- 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 b7a322073d..8779575b99 100644 --- 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 44c74fb974..04e58cd1c4 100644 --- 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 */ -- 2.39.5