v10-0001-Refactor-inlining-and-direct-calls-for-_slow-functio.patch

application/octet-stream

Filename: v10-0001-Refactor-inlining-and-direct-calls-for-_slow-functio.patch
Type: application/octet-stream
Part: 0
Message: RE: Popcount optimization using AVX512

Patch

Same data as JSON: GET /api/v1/attachments/:id/patch the parsed metadata as JSON — format, series position, per-file stats; never the diff bytes. API reference →
Format: format-patch
Series: patch v10-0001
Subject: [Refactor] inlining and direct calls for *_slow functions.
File+
src/backend/access/heap/visibilitymap.c 3 3
src/include/nodes/bitmapset.h 2 2
src/include/port/pg_bitutils.h 12 5
src/port/pg_bitutils.c 17 73
From e9483d2354bd58210805b17fa78091ecc007c612 Mon Sep 17 00:00:00 2001
From: Paul Amonson <paul.d.amonson@intel.com>
Date: Tue, 19 Mar 2024 13:35:53 -0700
Subject: [PATCH 1/3] [Refactor] inlining and direct calls for *_slow
 functions.

Signed-off-by: Paul Amonson <paul.d.amonson@intel.com>
---
 src/backend/access/heap/visibilitymap.c |  6 +-
 src/include/nodes/bitmapset.h           |  4 +-
 src/include/port/pg_bitutils.h          | 17 +++--
 src/port/pg_bitutils.c                  | 90 +++++--------------------
 4 files changed, 34 insertions(+), 83 deletions(-)

diff --git a/src/backend/access/heap/visibilitymap.c b/src/backend/access/heap/visibilitymap.c
index 1ab6c865e3..28dc497b79 100644
--- a/src/backend/access/heap/visibilitymap.c
+++ b/src/backend/access/heap/visibilitymap.c
@@ -419,14 +419,14 @@ visibilitymap_count(Relation rel, BlockNumber *all_visible, BlockNumber *all_fro
 		if (all_frozen == NULL)
 		{
 			for (i = 0; i < MAPSIZE / sizeof(uint64); i++)
-				nvisible += pg_popcount64(map[i] & VISIBLE_MASK64);
+				nvisible += PG_POPCOUNT64(map[i] & VISIBLE_MASK64);
 		}
 		else
 		{
 			for (i = 0; i < MAPSIZE / sizeof(uint64); i++)
 			{
-				nvisible += pg_popcount64(map[i] & VISIBLE_MASK64);
-				nfrozen += pg_popcount64(map[i] & FROZEN_MASK64);
+				nvisible += PG_POPCOUNT64(map[i] & VISIBLE_MASK64);
+				nfrozen += PG_POPCOUNT64(map[i] & FROZEN_MASK64);
 			}
 		}
 
diff --git a/src/include/nodes/bitmapset.h b/src/include/nodes/bitmapset.h
index 283bea5ea9..b5631d153e 100644
--- a/src/include/nodes/bitmapset.h
+++ b/src/include/nodes/bitmapset.h
@@ -77,11 +77,11 @@ typedef enum
 #if BITS_PER_BITMAPWORD == 32
 #define bmw_leftmost_one_pos(w)		pg_leftmost_one_pos32(w)
 #define bmw_rightmost_one_pos(w)	pg_rightmost_one_pos32(w)
-#define bmw_popcount(w)				pg_popcount32(w)
+#define bmw_popcount(w)				PG_POPCOUNT32(w)
 #elif BITS_PER_BITMAPWORD == 64
 #define bmw_leftmost_one_pos(w)		pg_leftmost_one_pos64(w)
 #define bmw_rightmost_one_pos(w)	pg_rightmost_one_pos64(w)
-#define bmw_popcount(w)				pg_popcount64(w)
+#define bmw_popcount(w)				PG_POPCOUNT64(w)
 #else
 #error "invalid BITS_PER_BITMAPWORD"
 #endif
diff --git a/src/include/port/pg_bitutils.h b/src/include/port/pg_bitutils.h
index 53e5239717..477e00e0da 100644
--- a/src/include/port/pg_bitutils.h
+++ b/src/include/port/pg_bitutils.h
@@ -300,16 +300,23 @@ pg_ceil_log2_64(uint64 num)
 
 #ifdef TRY_POPCNT_FAST
 /* Attempt to use the POPCNT instruction, but perform a runtime check first */
+extern int	pg_popcount32_slow(uint32 word);
+extern int	pg_popcount64_slow(uint64 word);
+extern uint64 pg_popcount_slow(const char *buf, int bytes);
 extern PGDLLIMPORT int (*pg_popcount32) (uint32 word);
 extern PGDLLIMPORT int (*pg_popcount64) (uint64 word);
 extern PGDLLIMPORT uint64 (*pg_popcount) (const char *buf, int bytes);
-
+#define PG_POPCOUNT32(x) pg_popcount32(x)
+#define PG_POPCOUNT64(x) pg_popcount64(x)
+#define PG_POPCOUNT(x,y) pg_popcount(x,y)
 #else
 /* Use a portable implementation -- no need for a function pointer. */
-extern int	pg_popcount32(uint32 word);
-extern int	pg_popcount64(uint64 word);
-extern uint64 pg_popcount(const char *buf, int bytes);
-
+extern int	pg_popcount32_slow(uint32 word);
+extern int	pg_popcount64_slow(uint64 word);
+extern uint64 pg_popcount_slow(const char *buf, int bytes);
+#define PG_POPCOUNT32(x) pg_popcount32_slow(x)
+#define PG_POPCOUNT64(x) pg_popcount64_slow(x)
+#define PG_POPCOUNT(x,y) pg_popcount_slow(x,y)
 #endif							/* TRY_POPCNT_FAST */
 
 /*
diff --git a/src/port/pg_bitutils.c b/src/port/pg_bitutils.c
index 1197696e97..e629969035 100644
--- a/src/port/pg_bitutils.c
+++ b/src/port/pg_bitutils.c
@@ -103,9 +103,9 @@ const uint8 pg_number_of_ones[256] = {
 	4, 5, 5, 6, 5, 6, 6, 7, 5, 6, 6, 7, 6, 7, 7, 8
 };
 
-static inline int pg_popcount32_slow(uint32 word);
-static inline int pg_popcount64_slow(uint64 word);
-static uint64 pg_popcount_slow(const char *buf, int bytes);
+// static inline int pg_popcount32_slow(uint32 word);
+// static inline int pg_popcount64_slow(uint64 word);
+// static uint64 pg_popcount_slow(const char *buf, int bytes);
 
 #ifdef TRY_POPCNT_FAST
 static bool pg_popcount_available(void);
@@ -119,9 +119,6 @@ static uint64 pg_popcount_fast(const char *buf, int bytes);
 int			(*pg_popcount32) (uint32 word) = pg_popcount32_choose;
 int			(*pg_popcount64) (uint64 word) = pg_popcount64_choose;
 uint64		(*pg_popcount) (const char *buf, int bytes) = pg_popcount_choose;
-#endif							/* TRY_POPCNT_FAST */
-
-#ifdef TRY_POPCNT_FAST
 
 /*
  * Return true if CPUID indicates that the POPCNT instruction is available.
@@ -148,8 +145,7 @@ pg_popcount_available(void)
  * the function pointers so that subsequent calls are routed directly to
  * the chosen implementation.
  */
-static int
-pg_popcount32_choose(uint32 word)
+static inline void set_function_pointers()
 {
 	if (pg_popcount_available())
 	{
@@ -163,45 +159,26 @@ pg_popcount32_choose(uint32 word)
 		pg_popcount64 = pg_popcount64_slow;
 		pg_popcount = pg_popcount_slow;
 	}
+}
 
+static int
+pg_popcount32_choose(uint32 word)
+{
+	set_function_pointers();
 	return pg_popcount32(word);
 }
 
 static int
 pg_popcount64_choose(uint64 word)
 {
-	if (pg_popcount_available())
-	{
-		pg_popcount32 = pg_popcount32_fast;
-		pg_popcount64 = pg_popcount64_fast;
-		pg_popcount = pg_popcount_fast;
-	}
-	else
-	{
-		pg_popcount32 = pg_popcount32_slow;
-		pg_popcount64 = pg_popcount64_slow;
-		pg_popcount = pg_popcount_slow;
-	}
-
+	set_function_pointers();
 	return pg_popcount64(word);
 }
 
 static uint64
 pg_popcount_choose(const char *buf, int bytes)
 {
-	if (pg_popcount_available())
-	{
-		pg_popcount32 = pg_popcount32_fast;
-		pg_popcount64 = pg_popcount64_fast;
-		pg_popcount = pg_popcount_fast;
-	}
-	else
-	{
-		pg_popcount32 = pg_popcount32_slow;
-		pg_popcount64 = pg_popcount64_slow;
-		pg_popcount = pg_popcount_slow;
-	}
-
+	set_function_pointers();
 	return pg_popcount(buf, bytes);
 }
 
@@ -243,7 +220,7 @@ __asm__ __volatile__(" popcntq %1,%0\n":"=q"(res):"rm"(word):"cc");
  * pg_popcount_fast
  *		Returns the number of 1-bits in buf
  */
-static uint64
+static inline uint64
 pg_popcount_fast(const char *buf, int bytes)
 {
 	uint64		popcnt = 0;
@@ -256,7 +233,7 @@ pg_popcount_fast(const char *buf, int bytes)
 
 		while (bytes >= 8)
 		{
-			popcnt += pg_popcount64_fast(*words++);
+			popcnt += PG_POPCOUNT64(*words++);
 			bytes -= 8;
 		}
 
@@ -270,7 +247,7 @@ pg_popcount_fast(const char *buf, int bytes)
 
 		while (bytes >= 4)
 		{
-			popcnt += pg_popcount32_fast(*words++);
+			popcnt += PG_POPCOUNT32(*words++);
 			bytes -= 4;
 		}
 
@@ -292,7 +269,7 @@ pg_popcount_fast(const char *buf, int bytes)
  * pg_popcount32_slow
  *		Return the number of 1 bits set in word
  */
-static inline int
+inline int
 pg_popcount32_slow(uint32 word)
 {
 #ifdef HAVE__BUILTIN_POPCOUNT
@@ -314,7 +291,7 @@ pg_popcount32_slow(uint32 word)
  * pg_popcount64_slow
  *		Return the number of 1 bits set in word
  */
-static inline int
+inline int
 pg_popcount64_slow(uint64 word)
 {
 #ifdef HAVE__BUILTIN_POPCOUNT
@@ -342,7 +319,7 @@ pg_popcount64_slow(uint64 word)
  * pg_popcount_slow
  *		Returns the number of 1-bits in buf
  */
-static uint64
+uint64
 pg_popcount_slow(const char *buf, int bytes)
 {
 	uint64		popcnt = 0;
@@ -383,36 +360,3 @@ pg_popcount_slow(const char *buf, int bytes)
 
 	return popcnt;
 }
-
-#ifndef TRY_POPCNT_FAST
-
-/*
- * When the POPCNT instruction is not available, there's no point in using
- * function pointers to vary the implementation between the fast and slow
- * method.  We instead just make these actual external functions when
- * TRY_POPCNT_FAST is not defined.  The compiler should be able to inline
- * the slow versions here.
- */
-int
-pg_popcount32(uint32 word)
-{
-	return pg_popcount32_slow(word);
-}
-
-int
-pg_popcount64(uint64 word)
-{
-	return pg_popcount64_slow(word);
-}
-
-/*
- * pg_popcount
- *		Returns the number of 1-bits in buf
- */
-uint64
-pg_popcount(const char *buf, int bytes)
-{
-	return pg_popcount_slow(buf, bytes);
-}
-
-#endif							/* !TRY_POPCNT_FAST */
-- 
2.34.1