agora inbox for pgsql-hackers@postgresql.org  
help / color / mirror / Atom feed
[PATCH 1/5] Make XLogReaderInvalReadState static.
19+ messages / 7 participants
[nested] [flat]

* [PATCH 1/5] Make XLogReaderInvalReadState static.
@ 2019-07-09 09:54 Antonin Houska <ah@cybertec.at>
  0 siblings, 0 replies; 19+ messages in thread

From: Antonin Houska @ 2019-07-09 09:54 UTC (permalink / raw)

This change is not necessary for the XLogRead() refactoring itself, but I
noticed the problem while working on it. Not sure it's worth a separate CF
entry.
---
 src/backend/access/transam/xlogreader.c | 4 ++--
 src/include/access/xlogreader.h         | 4 ----
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index 41dae916b4..08bc9695c1 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -35,10 +35,10 @@ static bool ValidXLogRecordHeader(XLogReaderState *state, XLogRecPtr RecPtr,
 								  XLogRecPtr PrevRecPtr, XLogRecord *record, bool randAccess);
 static bool ValidXLogRecord(XLogReaderState *state, XLogRecord *record,
 							XLogRecPtr recptr);
+static void XLogReaderInvalReadState(XLogReaderState *state);
 static int	ReadPageInternal(XLogReaderState *state, XLogRecPtr pageptr,
 							 int reqLen);
 static void report_invalid_record(XLogReaderState *state, const char *fmt,...) pg_attribute_printf(2, 3);
-
 static void ResetDecoder(XLogReaderState *state);
 
 /* size of the buffer allocated for error message. */
@@ -620,7 +620,7 @@ err:
 /*
  * Invalidate the xlogreader's read state to force a re-read.
  */
-void
+static void
 XLogReaderInvalReadState(XLogReaderState *state)
 {
 	state->readSegNo = 0;
diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h
index 04228e2a87..a3d3cc1e7b 100644
--- a/src/include/access/xlogreader.h
+++ b/src/include/access/xlogreader.h
@@ -212,13 +212,9 @@ extern struct XLogRecord *XLogReadRecord(XLogReaderState *state,
 extern bool XLogReaderValidatePageHeader(XLogReaderState *state,
 										 XLogRecPtr recptr, char *phdr);
 
-/* Invalidate read state */
-extern void XLogReaderInvalReadState(XLogReaderState *state);
-
 #ifdef FRONTEND
 extern XLogRecPtr XLogFindNextRecord(XLogReaderState *state, XLogRecPtr RecPtr);
 #endif							/* FRONTEND */
-
 /* Functions for decoding an XLogRecord */
 
 extern bool DecodeXLogRecord(XLogReaderState *state, XLogRecord *record,
-- 
2.16.4


--=-=-=
Content-Type: text/x-diff
Content-Disposition: attachment;
 filename=v04-0002-Remove-TLI-from-some-argument-lists.patch



^ permalink  raw  reply  [nested|flat] 19+ messages in thread

* [PATCH v3] psql: Add tab-complete for optional view parameters
@ 2023-08-07 18:37 Christoph Heiss <christoph@c8h4.io>
  0 siblings, 0 replies; 19+ messages in thread

From: Christoph Heiss @ 2023-08-07 18:37 UTC (permalink / raw)

This adds them in the same matter as it works for storage parameters of
tables.

Signed-off-by: Christoph Heiss <christoph@c8h4.io>
---
 src/bin/psql/tab-complete.c | 36 +++++++++++++++++++++++++++++++-----
 1 file changed, 31 insertions(+), 5 deletions(-)

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 779fdc90cb..83ec1508bb 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1329,6 +1329,13 @@ static const char *const table_storage_parameters[] = {
 	NULL
 };
 
+/* Optional parameters for CREATE VIEW and ALTER VIEW */
+static const char *const view_optional_parameters[] = {
+	"check_option",
+	"security_barrier",
+	"security_invoker",
+	NULL
+};
 
 /* Forward declaration of functions */
 static char **psql_completion(const char *text, int start, int end);
@@ -2216,8 +2223,7 @@ psql_completion(const char *text, int start, int end)
 		COMPLETE_WITH("TO");
 	/* ALTER VIEW <name> */
 	else if (Matches("ALTER", "VIEW", MatchAny))
-		COMPLETE_WITH("ALTER COLUMN", "OWNER TO", "RENAME",
-					  "SET SCHEMA");
+		COMPLETE_WITH("ALTER COLUMN", "OWNER TO", "RENAME", "RESET (", "SET");
 	/* ALTER VIEW xxx RENAME */
 	else if (Matches("ALTER", "VIEW", MatchAny, "RENAME"))
 		COMPLETE_WITH_ATTR_PLUS(prev2_wd, "COLUMN", "TO");
@@ -2233,6 +2239,16 @@ psql_completion(const char *text, int start, int end)
 	/* ALTER VIEW xxx RENAME COLUMN yyy */
 	else if (Matches("ALTER", "VIEW", MatchAny, "RENAME", "COLUMN", MatchAnyExcept("TO")))
 		COMPLETE_WITH("TO");
+	/* ALTER VIEW xxx SET ( yyy [= zzz] ) */
+	else if (Matches("ALTER", "VIEW", MatchAny, "SET"))
+		COMPLETE_WITH("(", "SCHEMA");
+	/* ALTER VIEW xxx SET|RESET ( yyy [= zzz] ) */
+	else if (Matches("ALTER", "VIEW", MatchAny, "SET|RESET", "("))
+		COMPLETE_WITH_LIST(view_optional_parameters);
+	else if (Matches("ALTER", "VIEW", MatchAny, "SET", "(", "check_option", "="))
+		COMPLETE_WITH("local", "cascaded");
+	else if (Matches("ALTER", "VIEW", MatchAny, "SET", "(", "security_barrier|security_invoker", "="))
+		COMPLETE_WITH("true", "false");
 
 	/* ALTER MATERIALIZED VIEW <name> */
 	else if (Matches("ALTER", "MATERIALIZED", "VIEW", MatchAny))
@@ -3525,13 +3541,23 @@ psql_completion(const char *text, int start, int end)
 	}
 
 /* CREATE VIEW --- is allowed inside CREATE SCHEMA, so use TailMatches */
-	/* Complete CREATE [ OR REPLACE ] VIEW <name> with AS */
+	/* Complete CREATE [ OR REPLACE ] VIEW <name> with AS or WITH ( */
 	else if (TailMatches("CREATE", "VIEW", MatchAny) ||
 			 TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny))
+		COMPLETE_WITH("AS", "WITH (");
+	/* Complete CREATE [ OR REPLACE ] VIEW <name> WITH ( with supported options */
+	else if (TailMatches("CREATE", "VIEW", MatchAny, "WITH", "(") ||
+			 TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "WITH", "("))
+		COMPLETE_WITH_LIST(view_optional_parameters);
+	/* Complete CREATE [ OR REPLACE ] VIEW <name> WITH ( ... ) with "AS" */
+	else if (TailMatches("CREATE", "VIEW", MatchAny, "WITH", "(*)") ||
+			 TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "WITH", "(*)"))
 		COMPLETE_WITH("AS");
-	/* Complete "CREATE [ OR REPLACE ] VIEW <sth> AS with "SELECT" */
+	/* Complete "CREATE [ OR REPLACE ] VIEW <sth> [ WITH ( ... ) ] AS with "SELECT" */
 	else if (TailMatches("CREATE", "VIEW", MatchAny, "AS") ||
-			 TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "AS"))
+			 TailMatches("CREATE", "VIEW", MatchAny, "WITH", "(*)", "AS") ||
+			 TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "AS") ||
+			 TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "WITH", "(*)", "AS"))
 		COMPLETE_WITH("SELECT");
 
 /* CREATE MATERIALIZED VIEW */
-- 
2.41.0


--h2n4c22ub7j2vbdx--





^ permalink  raw  reply  [nested|flat] 19+ messages in thread

* [PATCH v3] psql: Add tab-complete for optional view parameters
@ 2023-08-07 18:37 Christoph Heiss <christoph@c8h4.io>
  0 siblings, 0 replies; 19+ messages in thread

From: Christoph Heiss @ 2023-08-07 18:37 UTC (permalink / raw)

This adds them in the same matter as it works for storage parameters of
tables.

Signed-off-by: Christoph Heiss <christoph@c8h4.io>
---
 src/bin/psql/tab-complete.c | 36 +++++++++++++++++++++++++++++++-----
 1 file changed, 31 insertions(+), 5 deletions(-)

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 779fdc90cb..83ec1508bb 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1329,6 +1329,13 @@ static const char *const table_storage_parameters[] = {
 	NULL
 };
 
+/* Optional parameters for CREATE VIEW and ALTER VIEW */
+static const char *const view_optional_parameters[] = {
+	"check_option",
+	"security_barrier",
+	"security_invoker",
+	NULL
+};
 
 /* Forward declaration of functions */
 static char **psql_completion(const char *text, int start, int end);
@@ -2216,8 +2223,7 @@ psql_completion(const char *text, int start, int end)
 		COMPLETE_WITH("TO");
 	/* ALTER VIEW <name> */
 	else if (Matches("ALTER", "VIEW", MatchAny))
-		COMPLETE_WITH("ALTER COLUMN", "OWNER TO", "RENAME",
-					  "SET SCHEMA");
+		COMPLETE_WITH("ALTER COLUMN", "OWNER TO", "RENAME", "RESET (", "SET");
 	/* ALTER VIEW xxx RENAME */
 	else if (Matches("ALTER", "VIEW", MatchAny, "RENAME"))
 		COMPLETE_WITH_ATTR_PLUS(prev2_wd, "COLUMN", "TO");
@@ -2233,6 +2239,16 @@ psql_completion(const char *text, int start, int end)
 	/* ALTER VIEW xxx RENAME COLUMN yyy */
 	else if (Matches("ALTER", "VIEW", MatchAny, "RENAME", "COLUMN", MatchAnyExcept("TO")))
 		COMPLETE_WITH("TO");
+	/* ALTER VIEW xxx SET ( yyy [= zzz] ) */
+	else if (Matches("ALTER", "VIEW", MatchAny, "SET"))
+		COMPLETE_WITH("(", "SCHEMA");
+	/* ALTER VIEW xxx SET|RESET ( yyy [= zzz] ) */
+	else if (Matches("ALTER", "VIEW", MatchAny, "SET|RESET", "("))
+		COMPLETE_WITH_LIST(view_optional_parameters);
+	else if (Matches("ALTER", "VIEW", MatchAny, "SET", "(", "check_option", "="))
+		COMPLETE_WITH("local", "cascaded");
+	else if (Matches("ALTER", "VIEW", MatchAny, "SET", "(", "security_barrier|security_invoker", "="))
+		COMPLETE_WITH("true", "false");
 
 	/* ALTER MATERIALIZED VIEW <name> */
 	else if (Matches("ALTER", "MATERIALIZED", "VIEW", MatchAny))
@@ -3525,13 +3541,23 @@ psql_completion(const char *text, int start, int end)
 	}
 
 /* CREATE VIEW --- is allowed inside CREATE SCHEMA, so use TailMatches */
-	/* Complete CREATE [ OR REPLACE ] VIEW <name> with AS */
+	/* Complete CREATE [ OR REPLACE ] VIEW <name> with AS or WITH ( */
 	else if (TailMatches("CREATE", "VIEW", MatchAny) ||
 			 TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny))
+		COMPLETE_WITH("AS", "WITH (");
+	/* Complete CREATE [ OR REPLACE ] VIEW <name> WITH ( with supported options */
+	else if (TailMatches("CREATE", "VIEW", MatchAny, "WITH", "(") ||
+			 TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "WITH", "("))
+		COMPLETE_WITH_LIST(view_optional_parameters);
+	/* Complete CREATE [ OR REPLACE ] VIEW <name> WITH ( ... ) with "AS" */
+	else if (TailMatches("CREATE", "VIEW", MatchAny, "WITH", "(*)") ||
+			 TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "WITH", "(*)"))
 		COMPLETE_WITH("AS");
-	/* Complete "CREATE [ OR REPLACE ] VIEW <sth> AS with "SELECT" */
+	/* Complete "CREATE [ OR REPLACE ] VIEW <sth> [ WITH ( ... ) ] AS with "SELECT" */
 	else if (TailMatches("CREATE", "VIEW", MatchAny, "AS") ||
-			 TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "AS"))
+			 TailMatches("CREATE", "VIEW", MatchAny, "WITH", "(*)", "AS") ||
+			 TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "AS") ||
+			 TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "WITH", "(*)", "AS"))
 		COMPLETE_WITH("SELECT");
 
 /* CREATE MATERIALIZED VIEW */
-- 
2.41.0


--h2n4c22ub7j2vbdx--





^ permalink  raw  reply  [nested|flat] 19+ messages in thread

* [PATCH v3] psql: Add tab-complete for optional view parameters
@ 2023-08-07 18:37 Christoph Heiss <christoph@c8h4.io>
  0 siblings, 0 replies; 19+ messages in thread

From: Christoph Heiss @ 2023-08-07 18:37 UTC (permalink / raw)

This adds them in the same matter as it works for storage parameters of
tables.

Signed-off-by: Christoph Heiss <christoph@c8h4.io>
---
 src/bin/psql/tab-complete.c | 36 +++++++++++++++++++++++++++++++-----
 1 file changed, 31 insertions(+), 5 deletions(-)

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 779fdc90cb..83ec1508bb 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1329,6 +1329,13 @@ static const char *const table_storage_parameters[] = {
 	NULL
 };
 
+/* Optional parameters for CREATE VIEW and ALTER VIEW */
+static const char *const view_optional_parameters[] = {
+	"check_option",
+	"security_barrier",
+	"security_invoker",
+	NULL
+};
 
 /* Forward declaration of functions */
 static char **psql_completion(const char *text, int start, int end);
@@ -2216,8 +2223,7 @@ psql_completion(const char *text, int start, int end)
 		COMPLETE_WITH("TO");
 	/* ALTER VIEW <name> */
 	else if (Matches("ALTER", "VIEW", MatchAny))
-		COMPLETE_WITH("ALTER COLUMN", "OWNER TO", "RENAME",
-					  "SET SCHEMA");
+		COMPLETE_WITH("ALTER COLUMN", "OWNER TO", "RENAME", "RESET (", "SET");
 	/* ALTER VIEW xxx RENAME */
 	else if (Matches("ALTER", "VIEW", MatchAny, "RENAME"))
 		COMPLETE_WITH_ATTR_PLUS(prev2_wd, "COLUMN", "TO");
@@ -2233,6 +2239,16 @@ psql_completion(const char *text, int start, int end)
 	/* ALTER VIEW xxx RENAME COLUMN yyy */
 	else if (Matches("ALTER", "VIEW", MatchAny, "RENAME", "COLUMN", MatchAnyExcept("TO")))
 		COMPLETE_WITH("TO");
+	/* ALTER VIEW xxx SET ( yyy [= zzz] ) */
+	else if (Matches("ALTER", "VIEW", MatchAny, "SET"))
+		COMPLETE_WITH("(", "SCHEMA");
+	/* ALTER VIEW xxx SET|RESET ( yyy [= zzz] ) */
+	else if (Matches("ALTER", "VIEW", MatchAny, "SET|RESET", "("))
+		COMPLETE_WITH_LIST(view_optional_parameters);
+	else if (Matches("ALTER", "VIEW", MatchAny, "SET", "(", "check_option", "="))
+		COMPLETE_WITH("local", "cascaded");
+	else if (Matches("ALTER", "VIEW", MatchAny, "SET", "(", "security_barrier|security_invoker", "="))
+		COMPLETE_WITH("true", "false");
 
 	/* ALTER MATERIALIZED VIEW <name> */
 	else if (Matches("ALTER", "MATERIALIZED", "VIEW", MatchAny))
@@ -3525,13 +3541,23 @@ psql_completion(const char *text, int start, int end)
 	}
 
 /* CREATE VIEW --- is allowed inside CREATE SCHEMA, so use TailMatches */
-	/* Complete CREATE [ OR REPLACE ] VIEW <name> with AS */
+	/* Complete CREATE [ OR REPLACE ] VIEW <name> with AS or WITH ( */
 	else if (TailMatches("CREATE", "VIEW", MatchAny) ||
 			 TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny))
+		COMPLETE_WITH("AS", "WITH (");
+	/* Complete CREATE [ OR REPLACE ] VIEW <name> WITH ( with supported options */
+	else if (TailMatches("CREATE", "VIEW", MatchAny, "WITH", "(") ||
+			 TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "WITH", "("))
+		COMPLETE_WITH_LIST(view_optional_parameters);
+	/* Complete CREATE [ OR REPLACE ] VIEW <name> WITH ( ... ) with "AS" */
+	else if (TailMatches("CREATE", "VIEW", MatchAny, "WITH", "(*)") ||
+			 TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "WITH", "(*)"))
 		COMPLETE_WITH("AS");
-	/* Complete "CREATE [ OR REPLACE ] VIEW <sth> AS with "SELECT" */
+	/* Complete "CREATE [ OR REPLACE ] VIEW <sth> [ WITH ( ... ) ] AS with "SELECT" */
 	else if (TailMatches("CREATE", "VIEW", MatchAny, "AS") ||
-			 TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "AS"))
+			 TailMatches("CREATE", "VIEW", MatchAny, "WITH", "(*)", "AS") ||
+			 TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "AS") ||
+			 TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "WITH", "(*)", "AS"))
 		COMPLETE_WITH("SELECT");
 
 /* CREATE MATERIALIZED VIEW */
-- 
2.41.0


--h2n4c22ub7j2vbdx--





^ permalink  raw  reply  [nested|flat] 19+ messages in thread

* [PATCH v6 3/3] Remove specialized word-length popcount implementations.
@ 2026-01-23 23:31 Nathan Bossart <nathan@postgresql.org>
  0 siblings, 0 replies; 19+ messages in thread

From: Nathan Bossart @ 2026-01-23 23:31 UTC (permalink / raw)

---
 src/include/port/pg_bitutils.h | 75 +++++++++++++++++++++++-----------
 src/port/pg_bitutils.c         | 65 +----------------------------
 src/port/pg_popcount_aarch64.c | 25 ------------
 src/port/pg_popcount_x86.c     | 43 +------------------
 4 files changed, 54 insertions(+), 154 deletions(-)

diff --git a/src/include/port/pg_bitutils.h b/src/include/port/pg_bitutils.h
index 35761f509ec..a3e0f346ef6 100644
--- a/src/include/port/pg_bitutils.h
+++ b/src/include/port/pg_bitutils.h
@@ -276,46 +276,73 @@ pg_ceil_log2_64(uint64 num)
 		return pg_leftmost_one_pos64(num - 1) + 1;
 }
 
-extern int	pg_popcount32_portable(uint32 word);
-extern int	pg_popcount64_portable(uint64 word);
 extern uint64 pg_popcount_portable(const char *buf, int bytes);
 extern uint64 pg_popcount_masked_portable(const char *buf, int bytes, bits8 mask);
 
-#ifdef HAVE_X86_64_POPCNTQ
+#if defined(HAVE_X86_64_POPCNTQ) || defined(USE_SVE_POPCNT_WITH_RUNTIME_CHECK)
 /*
- * Attempt to use SSE4.2 or AVX-512 instructions, but perform a runtime check
+ * Attempt to use specialized CPU instructions, but perform a runtime check
  * first.
  */
-extern PGDLLIMPORT int (*pg_popcount32) (uint32 word);
-extern PGDLLIMPORT int (*pg_popcount64) (uint64 word);
 extern PGDLLIMPORT uint64 (*pg_popcount_optimized) (const char *buf, int bytes);
 extern PGDLLIMPORT uint64 (*pg_popcount_masked_optimized) (const char *buf, int bytes, bits8 mask);
 
-#elif defined(USE_NEON)
-/* Use the Neon version of pg_popcount{32,64} without function pointer. */
-extern int	pg_popcount32(uint32 word);
-extern int	pg_popcount64(uint64 word);
-
-/*
- * We can try to use an SVE-optimized pg_popcount() on some systems  For that,
- * we do use a function pointer.
- */
-#ifdef USE_SVE_POPCNT_WITH_RUNTIME_CHECK
-extern PGDLLIMPORT uint64 (*pg_popcount_optimized) (const char *buf, int bytes);
-extern PGDLLIMPORT uint64 (*pg_popcount_masked_optimized) (const char *buf, int bytes, bits8 mask);
 #else
+/* Use a portable implementation -- no need for a function pointer. */
 extern uint64 pg_popcount_optimized(const char *buf, int bytes);
 extern uint64 pg_popcount_masked_optimized(const char *buf, int bytes, bits8 mask);
+
 #endif
 
-#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_optimized(const char *buf, int bytes);
-extern uint64 pg_popcount_masked_optimized(const char *buf, int bytes, bits8 mask);
+/*
+ * pg_popcount32
+ *		Return the number of 1 bits set in word
+ */
+static inline int
+pg_popcount32(uint32 word)
+{
+#ifdef HAVE__BUILTIN_POPCOUNT
+	return __builtin_popcount(word);
+#else							/* !HAVE__BUILTIN_POPCOUNT */
+	int			result = 0;
+
+	while (word != 0)
+	{
+		result += pg_number_of_ones[word & 255];
+		word >>= 8;
+	}
 
+	return result;
+#endif							/* HAVE__BUILTIN_POPCOUNT */
+}
+
+/*
+ * pg_popcount64
+ *		Return the number of 1 bits set in word
+ */
+static inline int
+pg_popcount64(uint64 word)
+{
+#ifdef HAVE__BUILTIN_POPCOUNT
+#if SIZEOF_LONG == 8
+	return __builtin_popcountl(word);
+#elif SIZEOF_LONG_LONG == 8
+	return __builtin_popcountll(word);
+#else
+#error "cannot find integer of the same size as uint64_t"
 #endif
+#else							/* !HAVE__BUILTIN_POPCOUNT */
+	int			result = 0;
+
+	while (word != 0)
+	{
+		result += pg_number_of_ones[word & 255];
+		word >>= 8;
+	}
+
+	return result;
+#endif							/* HAVE__BUILTIN_POPCOUNT */
+}
 
 /*
  * Returns the number of 1-bits in buf.
diff --git a/src/port/pg_bitutils.c b/src/port/pg_bitutils.c
index bec06c06fc3..49b130f1306 100644
--- a/src/port/pg_bitutils.c
+++ b/src/port/pg_bitutils.c
@@ -96,56 +96,6 @@ const uint8 pg_number_of_ones[256] = {
 	4, 5, 5, 6, 5, 6, 6, 7, 5, 6, 6, 7, 6, 7, 7, 8
 };
 
-/*
- * pg_popcount32_portable
- *		Return the number of 1 bits set in word
- */
-int
-pg_popcount32_portable(uint32 word)
-{
-#ifdef HAVE__BUILTIN_POPCOUNT
-	return __builtin_popcount(word);
-#else							/* !HAVE__BUILTIN_POPCOUNT */
-	int			result = 0;
-
-	while (word != 0)
-	{
-		result += pg_number_of_ones[word & 255];
-		word >>= 8;
-	}
-
-	return result;
-#endif							/* HAVE__BUILTIN_POPCOUNT */
-}
-
-/*
- * pg_popcount64_portable
- *		Return the number of 1 bits set in word
- */
-int
-pg_popcount64_portable(uint64 word)
-{
-#ifdef HAVE__BUILTIN_POPCOUNT
-#if SIZEOF_LONG == 8
-	return __builtin_popcountl(word);
-#elif SIZEOF_LONG_LONG == 8
-	return __builtin_popcountll(word);
-#else
-#error "cannot find integer of the same size as uint64_t"
-#endif
-#else							/* !HAVE__BUILTIN_POPCOUNT */
-	int			result = 0;
-
-	while (word != 0)
-	{
-		result += pg_number_of_ones[word & 255];
-		word >>= 8;
-	}
-
-	return result;
-#endif							/* HAVE__BUILTIN_POPCOUNT */
-}
-
 /*
  * pg_popcount_portable
  *		Returns the number of 1-bits in buf
@@ -163,7 +113,7 @@ pg_popcount_portable(const char *buf, int bytes)
 
 		while (bytes >= 8)
 		{
-			popcnt += pg_popcount64_portable(*words++);
+			popcnt += pg_popcount64(*words++);
 			bytes -= 8;
 		}
 
@@ -197,7 +147,7 @@ pg_popcount_masked_portable(const char *buf, int bytes, bits8 mask)
 
 		while (bytes >= 8)
 		{
-			popcnt += pg_popcount64_portable(*words++ & maskv);
+			popcnt += pg_popcount64(*words++ & maskv);
 			bytes -= 8;
 		}
 
@@ -220,17 +170,6 @@ pg_popcount_masked_portable(const char *buf, int bytes, bits8 mask)
  * actual external functions.  The compiler should be able to inline the
  * portable versions here.
  */
-int
-pg_popcount32(uint32 word)
-{
-	return pg_popcount32_portable(word);
-}
-
-int
-pg_popcount64(uint64 word)
-{
-	return pg_popcount64_portable(word);
-}
 
 /*
  * pg_popcount_optimized
diff --git a/src/port/pg_popcount_aarch64.c b/src/port/pg_popcount_aarch64.c
index ba57f2cd4bd..74f71593721 100644
--- a/src/port/pg_popcount_aarch64.c
+++ b/src/port/pg_popcount_aarch64.c
@@ -291,31 +291,6 @@ pg_popcount_masked_optimized(const char *buf, int bytes, bits8 mask)
 
 #endif							/* ! USE_SVE_POPCNT_WITH_RUNTIME_CHECK */
 
-/*
- * pg_popcount32
- *		Return number of 1 bits in word
- */
-int
-pg_popcount32(uint32 word)
-{
-	return pg_popcount64((uint64) word);
-}
-
-/*
- * pg_popcount64
- *		Return number of 1 bits in word
- */
-int
-pg_popcount64(uint64 word)
-{
-	/*
-	 * For some compilers, __builtin_popcountl() already emits Neon
-	 * instructions.  The line below should compile to the same code on those
-	 * systems.
-	 */
-	return vaddv_u8(vcnt_u8(vld1_u8((const uint8 *) &word)));
-}
-
 /*
  * pg_popcount_neon
  *		Returns number of 1 bits in buf
diff --git a/src/port/pg_popcount_x86.c b/src/port/pg_popcount_x86.c
index 7aebf69898b..6bce089432f 100644
--- a/src/port/pg_popcount_x86.c
+++ b/src/port/pg_popcount_x86.c
@@ -36,8 +36,6 @@
  * operation, but in practice this is close enough, and "sse42" seems easier to
  * follow than "popcnt" for these names.
  */
-static inline int pg_popcount32_sse42(uint32 word);
-static inline int pg_popcount64_sse42(uint64 word);
 static uint64 pg_popcount_sse42(const char *buf, int bytes);
 static uint64 pg_popcount_masked_sse42(const char *buf, int bytes, bits8 mask);
 
@@ -55,12 +53,8 @@ static uint64 pg_popcount_masked_avx512(const char *buf, int bytes, bits8 mask);
  * what the current CPU supports) and then will call the pointer to fulfill the
  * caller's request.
  */
-static int	pg_popcount32_choose(uint32 word);
-static int	pg_popcount64_choose(uint64 word);
 static uint64 pg_popcount_choose(const char *buf, int bytes);
 static uint64 pg_popcount_masked_choose(const char *buf, int bytes, bits8 mask);
-int			(*pg_popcount32) (uint32 word) = pg_popcount32_choose;
-int			(*pg_popcount64) (uint64 word) = pg_popcount64_choose;
 uint64		(*pg_popcount_optimized) (const char *buf, int bytes) = pg_popcount_choose;
 uint64		(*pg_popcount_masked_optimized) (const char *buf, int bytes, bits8 mask) = pg_popcount_masked_choose;
 
@@ -157,7 +151,7 @@ pg_popcount_avx512_available(void)
 #endif							/* USE_AVX512_POPCNT_WITH_RUNTIME_CHECK */
 
 /*
- * These functions get called on the first call to pg_popcount32 etc.
+ * These functions get called on the first call to pg_popcount(), etc.
  * They detect whether we can use the asm implementations, and replace
  * the function pointers so that subsequent calls are routed directly to
  * the chosen implementation.
@@ -167,15 +161,11 @@ choose_popcount_functions(void)
 {
 	if (pg_popcount_sse42_available())
 	{
-		pg_popcount32 = pg_popcount32_sse42;
-		pg_popcount64 = pg_popcount64_sse42;
 		pg_popcount_optimized = pg_popcount_sse42;
 		pg_popcount_masked_optimized = pg_popcount_masked_sse42;
 	}
 	else
 	{
-		pg_popcount32 = pg_popcount32_portable;
-		pg_popcount64 = pg_popcount64_portable;
 		pg_popcount_optimized = pg_popcount_portable;
 		pg_popcount_masked_optimized = pg_popcount_masked_portable;
 	}
@@ -189,20 +179,6 @@ choose_popcount_functions(void)
 #endif
 }
 
-static int
-pg_popcount32_choose(uint32 word)
-{
-	choose_popcount_functions();
-	return pg_popcount32(word);
-}
-
-static int
-pg_popcount64_choose(uint64 word)
-{
-	choose_popcount_functions();
-	return pg_popcount64(word);
-}
-
 static uint64
 pg_popcount_choose(const char *buf, int bytes)
 {
@@ -338,23 +314,6 @@ pg_popcount_masked_avx512(const char *buf, int bytes, bits8 mask)
 
 #endif							/* USE_AVX512_POPCNT_WITH_RUNTIME_CHECK */
 
-/*
- * pg_popcount32_sse42
- *		Return the number of 1 bits set in word
- */
-static inline int
-pg_popcount32_sse42(uint32 word)
-{
-#ifdef _MSC_VER
-	return __popcnt(word);
-#else
-	uint32		res;
-
-__asm__ __volatile__(" popcntl %1,%0\n":"=q"(res):"rm"(word):"cc");
-	return (int) res;
-#endif
-}
-
 /*
  * pg_popcount64_sse42
  *		Return the number of 1 bits set in word
-- 
2.50.1 (Apple Git-155)


--vgXyZptCXHDBThGI--





^ permalink  raw  reply  [nested|flat] 19+ messages in thread

* [PATCH v12 2/4] Remove specialized word-length popcount implementations.
@ 2026-01-23 23:31 Nathan Bossart <nathan@postgresql.org>
  0 siblings, 0 replies; 19+ messages in thread

From: Nathan Bossart @ 2026-01-23 23:31 UTC (permalink / raw)

The uses of these functions do not justify the level of
micro-optimization we've done and may even hurt performance in some
cases (e.g., due to using function pointers).  This commit removes
all architecture-specific implementations of pg_popcount{32,64}()
and converts the portable ones to inlined functions in
pg_bitutils.h.  These inlined versions should produce the same code
as before (but inlined), so in theory this is a net gain for many
machines.

Suggested-by: John Naylor <johncnaylorls@gmail.com>
Reviewed-by: John Naylor <johncnaylorls@gmail.com>
Reviewed-by: Greg Burd <greg@burd.me>
Discussion: https://postgr.es/m/CANWCAZY7R%2Biy%2Br9YM_sySNydHzNqUirx1xk0tB3ej5HO62GdgQ%40mail.gmail.com
---
 src/include/port/pg_bitutils.h | 75 +++++++++++++++++++++++-----------
 src/port/pg_bitutils.c         | 65 +----------------------------
 src/port/pg_popcount_aarch64.c | 20 +++------
 src/port/pg_popcount_x86.c     | 43 +------------------
 4 files changed, 59 insertions(+), 144 deletions(-)

diff --git a/src/include/port/pg_bitutils.h b/src/include/port/pg_bitutils.h
index 20c11b79c61..789663edd93 100644
--- a/src/include/port/pg_bitutils.h
+++ b/src/include/port/pg_bitutils.h
@@ -276,46 +276,73 @@ pg_ceil_log2_64(uint64 num)
 		return pg_leftmost_one_pos64(num - 1) + 1;
 }
 
-extern int	pg_popcount32_portable(uint32 word);
-extern int	pg_popcount64_portable(uint64 word);
 extern uint64 pg_popcount_portable(const char *buf, int bytes);
 extern uint64 pg_popcount_masked_portable(const char *buf, int bytes, bits8 mask);
 
-#ifdef HAVE_X86_64_POPCNTQ
+#if defined(HAVE_X86_64_POPCNTQ) || defined(USE_SVE_POPCNT_WITH_RUNTIME_CHECK)
 /*
- * Attempt to use SSE4.2 or AVX-512 instructions, but perform a runtime check
+ * Attempt to use specialized CPU instructions, but perform a runtime check
  * first.
  */
-extern PGDLLIMPORT int (*pg_popcount32) (uint32 word);
-extern PGDLLIMPORT int (*pg_popcount64) (uint64 word);
 extern PGDLLIMPORT uint64 (*pg_popcount_optimized) (const char *buf, int bytes);
 extern PGDLLIMPORT uint64 (*pg_popcount_masked_optimized) (const char *buf, int bytes, bits8 mask);
 
-#elif defined(USE_NEON)
-/* Use the Neon version of pg_popcount{32,64} without function pointer. */
-extern int	pg_popcount32(uint32 word);
-extern int	pg_popcount64(uint64 word);
-
-/*
- * We can try to use an SVE-optimized pg_popcount() on some systems  For that,
- * we do use a function pointer.
- */
-#ifdef USE_SVE_POPCNT_WITH_RUNTIME_CHECK
-extern PGDLLIMPORT uint64 (*pg_popcount_optimized) (const char *buf, int bytes);
-extern PGDLLIMPORT uint64 (*pg_popcount_masked_optimized) (const char *buf, int bytes, bits8 mask);
 #else
+/* Use a portable implementation -- no need for a function pointer. */
 extern uint64 pg_popcount_optimized(const char *buf, int bytes);
 extern uint64 pg_popcount_masked_optimized(const char *buf, int bytes, bits8 mask);
+
 #endif
 
-#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_optimized(const char *buf, int bytes);
-extern uint64 pg_popcount_masked_optimized(const char *buf, int bytes, bits8 mask);
+/*
+ * pg_popcount32
+ *		Return the number of 1 bits set in word
+ */
+static inline int
+pg_popcount32(uint32 word)
+{
+#ifdef HAVE__BUILTIN_POPCOUNT
+	return __builtin_popcount(word);
+#else							/* !HAVE__BUILTIN_POPCOUNT */
+	int			result = 0;
+
+	while (word != 0)
+	{
+		result += pg_number_of_ones[word & 255];
+		word >>= 8;
+	}
 
+	return result;
+#endif							/* HAVE__BUILTIN_POPCOUNT */
+}
+
+/*
+ * pg_popcount64
+ *		Return the number of 1 bits set in word
+ */
+static inline int
+pg_popcount64(uint64 word)
+{
+#ifdef HAVE__BUILTIN_POPCOUNT
+#if SIZEOF_LONG == 8
+	return __builtin_popcountl(word);
+#elif SIZEOF_LONG_LONG == 8
+	return __builtin_popcountll(word);
+#else
+#error "cannot find integer of the same size as uint64_t"
 #endif
+#else							/* !HAVE__BUILTIN_POPCOUNT */
+	int			result = 0;
+
+	while (word != 0)
+	{
+		result += pg_number_of_ones[word & 255];
+		word >>= 8;
+	}
+
+	return result;
+#endif							/* HAVE__BUILTIN_POPCOUNT */
+}
 
 /*
  * Returns the number of 1-bits in buf.
diff --git a/src/port/pg_bitutils.c b/src/port/pg_bitutils.c
index bec06c06fc3..49b130f1306 100644
--- a/src/port/pg_bitutils.c
+++ b/src/port/pg_bitutils.c
@@ -96,56 +96,6 @@ const uint8 pg_number_of_ones[256] = {
 	4, 5, 5, 6, 5, 6, 6, 7, 5, 6, 6, 7, 6, 7, 7, 8
 };
 
-/*
- * pg_popcount32_portable
- *		Return the number of 1 bits set in word
- */
-int
-pg_popcount32_portable(uint32 word)
-{
-#ifdef HAVE__BUILTIN_POPCOUNT
-	return __builtin_popcount(word);
-#else							/* !HAVE__BUILTIN_POPCOUNT */
-	int			result = 0;
-
-	while (word != 0)
-	{
-		result += pg_number_of_ones[word & 255];
-		word >>= 8;
-	}
-
-	return result;
-#endif							/* HAVE__BUILTIN_POPCOUNT */
-}
-
-/*
- * pg_popcount64_portable
- *		Return the number of 1 bits set in word
- */
-int
-pg_popcount64_portable(uint64 word)
-{
-#ifdef HAVE__BUILTIN_POPCOUNT
-#if SIZEOF_LONG == 8
-	return __builtin_popcountl(word);
-#elif SIZEOF_LONG_LONG == 8
-	return __builtin_popcountll(word);
-#else
-#error "cannot find integer of the same size as uint64_t"
-#endif
-#else							/* !HAVE__BUILTIN_POPCOUNT */
-	int			result = 0;
-
-	while (word != 0)
-	{
-		result += pg_number_of_ones[word & 255];
-		word >>= 8;
-	}
-
-	return result;
-#endif							/* HAVE__BUILTIN_POPCOUNT */
-}
-
 /*
  * pg_popcount_portable
  *		Returns the number of 1-bits in buf
@@ -163,7 +113,7 @@ pg_popcount_portable(const char *buf, int bytes)
 
 		while (bytes >= 8)
 		{
-			popcnt += pg_popcount64_portable(*words++);
+			popcnt += pg_popcount64(*words++);
 			bytes -= 8;
 		}
 
@@ -197,7 +147,7 @@ pg_popcount_masked_portable(const char *buf, int bytes, bits8 mask)
 
 		while (bytes >= 8)
 		{
-			popcnt += pg_popcount64_portable(*words++ & maskv);
+			popcnt += pg_popcount64(*words++ & maskv);
 			bytes -= 8;
 		}
 
@@ -220,17 +170,6 @@ pg_popcount_masked_portable(const char *buf, int bytes, bits8 mask)
  * actual external functions.  The compiler should be able to inline the
  * portable versions here.
  */
-int
-pg_popcount32(uint32 word)
-{
-	return pg_popcount32_portable(word);
-}
-
-int
-pg_popcount64(uint64 word)
-{
-	return pg_popcount64_portable(word);
-}
 
 /*
  * pg_popcount_optimized
diff --git a/src/port/pg_popcount_aarch64.c b/src/port/pg_popcount_aarch64.c
index ba57f2cd4bd..f474ef45510 100644
--- a/src/port/pg_popcount_aarch64.c
+++ b/src/port/pg_popcount_aarch64.c
@@ -292,21 +292,11 @@ pg_popcount_masked_optimized(const char *buf, int bytes, bits8 mask)
 #endif							/* ! USE_SVE_POPCNT_WITH_RUNTIME_CHECK */
 
 /*
- * pg_popcount32
+ * pg_popcount64_neon
  *		Return number of 1 bits in word
  */
-int
-pg_popcount32(uint32 word)
-{
-	return pg_popcount64((uint64) word);
-}
-
-/*
- * pg_popcount64
- *		Return number of 1 bits in word
- */
-int
-pg_popcount64(uint64 word)
+static inline int
+pg_popcount64_neon(uint64 word)
 {
 	/*
 	 * For some compilers, __builtin_popcountl() already emits Neon
@@ -383,7 +373,7 @@ pg_popcount_neon(const char *buf, int bytes)
 	 */
 	for (; bytes >= sizeof(uint64); bytes -= sizeof(uint64))
 	{
-		popcnt += pg_popcount64(*((const uint64 *) buf));
+		popcnt += pg_popcount64_neon(*((const uint64 *) buf));
 		buf += sizeof(uint64);
 	}
 
@@ -465,7 +455,7 @@ pg_popcount_masked_neon(const char *buf, int bytes, bits8 mask)
 	 */
 	for (; bytes >= sizeof(uint64); bytes -= sizeof(uint64))
 	{
-		popcnt += pg_popcount64(*((const uint64 *) buf) & mask64);
+		popcnt += pg_popcount64_neon(*((const uint64 *) buf) & mask64);
 		buf += sizeof(uint64);
 	}
 
diff --git a/src/port/pg_popcount_x86.c b/src/port/pg_popcount_x86.c
index 7aebf69898b..6bce089432f 100644
--- a/src/port/pg_popcount_x86.c
+++ b/src/port/pg_popcount_x86.c
@@ -36,8 +36,6 @@
  * operation, but in practice this is close enough, and "sse42" seems easier to
  * follow than "popcnt" for these names.
  */
-static inline int pg_popcount32_sse42(uint32 word);
-static inline int pg_popcount64_sse42(uint64 word);
 static uint64 pg_popcount_sse42(const char *buf, int bytes);
 static uint64 pg_popcount_masked_sse42(const char *buf, int bytes, bits8 mask);
 
@@ -55,12 +53,8 @@ static uint64 pg_popcount_masked_avx512(const char *buf, int bytes, bits8 mask);
  * what the current CPU supports) and then will call the pointer to fulfill the
  * caller's request.
  */
-static int	pg_popcount32_choose(uint32 word);
-static int	pg_popcount64_choose(uint64 word);
 static uint64 pg_popcount_choose(const char *buf, int bytes);
 static uint64 pg_popcount_masked_choose(const char *buf, int bytes, bits8 mask);
-int			(*pg_popcount32) (uint32 word) = pg_popcount32_choose;
-int			(*pg_popcount64) (uint64 word) = pg_popcount64_choose;
 uint64		(*pg_popcount_optimized) (const char *buf, int bytes) = pg_popcount_choose;
 uint64		(*pg_popcount_masked_optimized) (const char *buf, int bytes, bits8 mask) = pg_popcount_masked_choose;
 
@@ -157,7 +151,7 @@ pg_popcount_avx512_available(void)
 #endif							/* USE_AVX512_POPCNT_WITH_RUNTIME_CHECK */
 
 /*
- * These functions get called on the first call to pg_popcount32 etc.
+ * These functions get called on the first call to pg_popcount(), etc.
  * They detect whether we can use the asm implementations, and replace
  * the function pointers so that subsequent calls are routed directly to
  * the chosen implementation.
@@ -167,15 +161,11 @@ choose_popcount_functions(void)
 {
 	if (pg_popcount_sse42_available())
 	{
-		pg_popcount32 = pg_popcount32_sse42;
-		pg_popcount64 = pg_popcount64_sse42;
 		pg_popcount_optimized = pg_popcount_sse42;
 		pg_popcount_masked_optimized = pg_popcount_masked_sse42;
 	}
 	else
 	{
-		pg_popcount32 = pg_popcount32_portable;
-		pg_popcount64 = pg_popcount64_portable;
 		pg_popcount_optimized = pg_popcount_portable;
 		pg_popcount_masked_optimized = pg_popcount_masked_portable;
 	}
@@ -189,20 +179,6 @@ choose_popcount_functions(void)
 #endif
 }
 
-static int
-pg_popcount32_choose(uint32 word)
-{
-	choose_popcount_functions();
-	return pg_popcount32(word);
-}
-
-static int
-pg_popcount64_choose(uint64 word)
-{
-	choose_popcount_functions();
-	return pg_popcount64(word);
-}
-
 static uint64
 pg_popcount_choose(const char *buf, int bytes)
 {
@@ -338,23 +314,6 @@ pg_popcount_masked_avx512(const char *buf, int bytes, bits8 mask)
 
 #endif							/* USE_AVX512_POPCNT_WITH_RUNTIME_CHECK */
 
-/*
- * pg_popcount32_sse42
- *		Return the number of 1 bits set in word
- */
-static inline int
-pg_popcount32_sse42(uint32 word)
-{
-#ifdef _MSC_VER
-	return __popcnt(word);
-#else
-	uint32		res;
-
-__asm__ __volatile__(" popcntl %1,%0\n":"=q"(res):"rm"(word):"cc");
-	return (int) res;
-#endif
-}
-
 /*
  * pg_popcount64_sse42
  *		Return the number of 1 bits set in word
-- 
2.50.1 (Apple Git-155)


--AFLuD1w+kMRlv6Zk
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
	filename=v12-0003-Remove-uses-of-popcount-builtins.patch



^ permalink  raw  reply  [nested|flat] 19+ messages in thread

* [PATCH v10 2/3] Remove specialized word-length popcount implementations.
@ 2026-01-23 23:31 Nathan Bossart <nathan@postgresql.org>
  0 siblings, 0 replies; 19+ messages in thread

From: Nathan Bossart @ 2026-01-23 23:31 UTC (permalink / raw)

The uses of these functions do not justify the level of
micro-optimization we've done and may even hurt performance in some
cases (e.g., due to using function pointers).  This commit removes
all architecture-specific implementations of pg_popcount{32,64}()
and converts the portable ones to inlined functions in
pg_bitutils.h.  These inlined versions should produce the same code
as before (but inlined), so in theory this is a net gain for many
machines.  As an exception, for x86-64/gcc without sse4.2/popcnt,
we use a plain C version to ensure inlining because
__builtin_popcount() and __builtin_popcountl() generate function
calls for that configuration.  Our tests indicate this is still a
net win.

Suggested-by: John Naylor <johncnaylorls@gmail.com>
Reviewed-by: John Naylor <johncnaylorls@gmail.com>
Reviewed-by: Greg Burd <greg@burd.me>
Discussion: https://postgr.es/m/CANWCAZY7R%2Biy%2Br9YM_sySNydHzNqUirx1xk0tB3ej5HO62GdgQ%40mail.gmail.com
---
 configure                      | 38 --------------------
 configure.ac                   |  1 -
 meson.build                    |  1 -
 src/include/pg_config.h.in     |  3 --
 src/include/port/pg_bitutils.h | 59 +++++++++++++++++-------------
 src/port/pg_bitutils.c         | 65 ++--------------------------------
 src/port/pg_popcount_aarch64.c | 23 +++---------
 src/port/pg_popcount_x86.c     | 43 +---------------------
 8 files changed, 41 insertions(+), 192 deletions(-)

diff --git a/configure b/configure
index ba293931878..623aa397fae 100755
--- a/configure
+++ b/configure
@@ -15920,44 +15920,6 @@ cat >>confdefs.h <<_ACEOF
 #define HAVE__BUILTIN_CTZ 1
 _ACEOF
 
-fi
-{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for __builtin_popcount" >&5
-$as_echo_n "checking for __builtin_popcount... " >&6; }
-if ${pgac_cv__builtin_popcount+:} false; then :
-  $as_echo_n "(cached) " >&6
-else
-  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
-/* end confdefs.h.  */
-
-int
-call__builtin_popcount(unsigned int x)
-{
-    return __builtin_popcount(x);
-}
-int
-main ()
-{
-
-  ;
-  return 0;
-}
-_ACEOF
-if ac_fn_c_try_link "$LINENO"; then :
-  pgac_cv__builtin_popcount=yes
-else
-  pgac_cv__builtin_popcount=no
-fi
-rm -f core conftest.err conftest.$ac_objext \
-    conftest$ac_exeext conftest.$ac_ext
-fi
-{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv__builtin_popcount" >&5
-$as_echo "$pgac_cv__builtin_popcount" >&6; }
-if test x"${pgac_cv__builtin_popcount}" = xyes ; then
-
-cat >>confdefs.h <<_ACEOF
-#define HAVE__BUILTIN_POPCOUNT 1
-_ACEOF
-
 fi
 # __builtin_frame_address may draw a diagnostic for non-constant argument,
 # so it needs a different test function.
diff --git a/configure.ac b/configure.ac
index 412fe358a2f..04c6a75bff7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1853,7 +1853,6 @@ PGAC_CHECK_BUILTIN_FUNC([__builtin_bswap64], [long int x])
 # We assume that we needn't test all widths of these explicitly:
 PGAC_CHECK_BUILTIN_FUNC([__builtin_clz], [unsigned int x])
 PGAC_CHECK_BUILTIN_FUNC([__builtin_ctz], [unsigned int x])
-PGAC_CHECK_BUILTIN_FUNC([__builtin_popcount], [unsigned int x])
 # __builtin_frame_address may draw a diagnostic for non-constant argument,
 # so it needs a different test function.
 PGAC_CHECK_BUILTIN_FUNC_PTR([__builtin_frame_address], [0])
diff --git a/meson.build b/meson.build
index 0722b16927e..c607d8ac69a 100644
--- a/meson.build
+++ b/meson.build
@@ -2004,7 +2004,6 @@ builtins = [
   'ctz',
   'constant_p',
   'frame_address',
-  'popcount',
   'unreachable',
 ]
 
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index c089f2252c3..301328b8cd3 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -530,9 +530,6 @@
 /* Define to 1 if your compiler understands __builtin_$op_overflow. */
 #undef HAVE__BUILTIN_OP_OVERFLOW
 
-/* Define to 1 if your compiler understands __builtin_popcount. */
-#undef HAVE__BUILTIN_POPCOUNT
-
 /* Define to 1 if your compiler understands __builtin_types_compatible_p. */
 #undef HAVE__BUILTIN_TYPES_COMPATIBLE_P
 
diff --git a/src/include/port/pg_bitutils.h b/src/include/port/pg_bitutils.h
index 20c11b79c61..c9b1f5f17dc 100644
--- a/src/include/port/pg_bitutils.h
+++ b/src/include/port/pg_bitutils.h
@@ -276,47 +276,56 @@ pg_ceil_log2_64(uint64 num)
 		return pg_leftmost_one_pos64(num - 1) + 1;
 }
 
-extern int	pg_popcount32_portable(uint32 word);
-extern int	pg_popcount64_portable(uint64 word);
 extern uint64 pg_popcount_portable(const char *buf, int bytes);
 extern uint64 pg_popcount_masked_portable(const char *buf, int bytes, bits8 mask);
 
-#ifdef HAVE_X86_64_POPCNTQ
+#if defined(HAVE_X86_64_POPCNTQ) || defined(USE_SVE_POPCNT_WITH_RUNTIME_CHECK)
 /*
- * Attempt to use SSE4.2 or AVX-512 instructions, but perform a runtime check
+ * Attempt to use specialized CPU instructions, but perform a runtime check
  * first.
  */
-extern PGDLLIMPORT int (*pg_popcount32) (uint32 word);
-extern PGDLLIMPORT int (*pg_popcount64) (uint64 word);
 extern PGDLLIMPORT uint64 (*pg_popcount_optimized) (const char *buf, int bytes);
 extern PGDLLIMPORT uint64 (*pg_popcount_masked_optimized) (const char *buf, int bytes, bits8 mask);
 
-#elif defined(USE_NEON)
-/* Use the Neon version of pg_popcount{32,64} without function pointer. */
-extern int	pg_popcount32(uint32 word);
-extern int	pg_popcount64(uint64 word);
-
-/*
- * We can try to use an SVE-optimized pg_popcount() on some systems  For that,
- * we do use a function pointer.
- */
-#ifdef USE_SVE_POPCNT_WITH_RUNTIME_CHECK
-extern PGDLLIMPORT uint64 (*pg_popcount_optimized) (const char *buf, int bytes);
-extern PGDLLIMPORT uint64 (*pg_popcount_masked_optimized) (const char *buf, int bytes, bits8 mask);
-#else
-extern uint64 pg_popcount_optimized(const char *buf, int bytes);
-extern uint64 pg_popcount_masked_optimized(const char *buf, int bytes, bits8 mask);
-#endif
-
 #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_optimized(const char *buf, int bytes);
 extern uint64 pg_popcount_masked_optimized(const char *buf, int bytes, bits8 mask);
 
 #endif
 
+/*
+ * pg_popcount32
+ *		Return the number of 1 bits set in word
+ *
+ * Adapted from
+ * https://graphics.stanford.edu/~seander/bithacks.html#CountBitsSetParallel.
+ */
+static inline int
+pg_popcount32(uint32 word)
+{
+	word -= (word >> 1) & 0x55555555;
+	word = (word & 0x33333333) + ((word >> 2) & 0x33333333);
+	return (((word + (word >> 4)) & 0xf0f0f0f) * 0x1010101) >> 24;
+}
+
+/*
+ * pg_popcount64
+ *		Return the number of 1 bits set in word
+ *
+ * Adapted from
+ * https://graphics.stanford.edu/~seander/bithacks.html#CountBitsSetParallel.
+ */
+static inline int
+pg_popcount64(uint64 word)
+{
+	word -= (word >> 1) & UINT64CONST(0x5555555555555555);
+	word = (word & UINT64CONST(0x3333333333333333)) +
+		((word >> 2) & UINT64CONST(0x3333333333333333));
+	word = (word + (word >> 4)) & UINT64CONST(0xf0f0f0f0f0f0f0f);
+	return (word * UINT64CONST(0x101010101010101)) >> 56;
+}
+
 /*
  * Returns the number of 1-bits in buf.
  *
diff --git a/src/port/pg_bitutils.c b/src/port/pg_bitutils.c
index bec06c06fc3..49b130f1306 100644
--- a/src/port/pg_bitutils.c
+++ b/src/port/pg_bitutils.c
@@ -96,56 +96,6 @@ const uint8 pg_number_of_ones[256] = {
 	4, 5, 5, 6, 5, 6, 6, 7, 5, 6, 6, 7, 6, 7, 7, 8
 };
 
-/*
- * pg_popcount32_portable
- *		Return the number of 1 bits set in word
- */
-int
-pg_popcount32_portable(uint32 word)
-{
-#ifdef HAVE__BUILTIN_POPCOUNT
-	return __builtin_popcount(word);
-#else							/* !HAVE__BUILTIN_POPCOUNT */
-	int			result = 0;
-
-	while (word != 0)
-	{
-		result += pg_number_of_ones[word & 255];
-		word >>= 8;
-	}
-
-	return result;
-#endif							/* HAVE__BUILTIN_POPCOUNT */
-}
-
-/*
- * pg_popcount64_portable
- *		Return the number of 1 bits set in word
- */
-int
-pg_popcount64_portable(uint64 word)
-{
-#ifdef HAVE__BUILTIN_POPCOUNT
-#if SIZEOF_LONG == 8
-	return __builtin_popcountl(word);
-#elif SIZEOF_LONG_LONG == 8
-	return __builtin_popcountll(word);
-#else
-#error "cannot find integer of the same size as uint64_t"
-#endif
-#else							/* !HAVE__BUILTIN_POPCOUNT */
-	int			result = 0;
-
-	while (word != 0)
-	{
-		result += pg_number_of_ones[word & 255];
-		word >>= 8;
-	}
-
-	return result;
-#endif							/* HAVE__BUILTIN_POPCOUNT */
-}
-
 /*
  * pg_popcount_portable
  *		Returns the number of 1-bits in buf
@@ -163,7 +113,7 @@ pg_popcount_portable(const char *buf, int bytes)
 
 		while (bytes >= 8)
 		{
-			popcnt += pg_popcount64_portable(*words++);
+			popcnt += pg_popcount64(*words++);
 			bytes -= 8;
 		}
 
@@ -197,7 +147,7 @@ pg_popcount_masked_portable(const char *buf, int bytes, bits8 mask)
 
 		while (bytes >= 8)
 		{
-			popcnt += pg_popcount64_portable(*words++ & maskv);
+			popcnt += pg_popcount64(*words++ & maskv);
 			bytes -= 8;
 		}
 
@@ -220,17 +170,6 @@ pg_popcount_masked_portable(const char *buf, int bytes, bits8 mask)
  * actual external functions.  The compiler should be able to inline the
  * portable versions here.
  */
-int
-pg_popcount32(uint32 word)
-{
-	return pg_popcount32_portable(word);
-}
-
-int
-pg_popcount64(uint64 word)
-{
-	return pg_popcount64_portable(word);
-}
 
 /*
  * pg_popcount_optimized
diff --git a/src/port/pg_popcount_aarch64.c b/src/port/pg_popcount_aarch64.c
index ba57f2cd4bd..357e938549f 100644
--- a/src/port/pg_popcount_aarch64.c
+++ b/src/port/pg_popcount_aarch64.c
@@ -291,28 +291,13 @@ pg_popcount_masked_optimized(const char *buf, int bytes, bits8 mask)
 
 #endif							/* ! USE_SVE_POPCNT_WITH_RUNTIME_CHECK */
 
-/*
- * pg_popcount32
- *		Return number of 1 bits in word
- */
-int
-pg_popcount32(uint32 word)
-{
-	return pg_popcount64((uint64) word);
-}
-
 /*
  * pg_popcount64
  *		Return number of 1 bits in word
  */
-int
-pg_popcount64(uint64 word)
+static inline int
+pg_popcount64_neon(uint64 word)
 {
-	/*
-	 * For some compilers, __builtin_popcountl() already emits Neon
-	 * instructions.  The line below should compile to the same code on those
-	 * systems.
-	 */
 	return vaddv_u8(vcnt_u8(vld1_u8((const uint8 *) &word)));
 }
 
@@ -383,7 +368,7 @@ pg_popcount_neon(const char *buf, int bytes)
 	 */
 	for (; bytes >= sizeof(uint64); bytes -= sizeof(uint64))
 	{
-		popcnt += pg_popcount64(*((const uint64 *) buf));
+		popcnt += pg_popcount64_neon(*((const uint64 *) buf));
 		buf += sizeof(uint64);
 	}
 
@@ -465,7 +450,7 @@ pg_popcount_masked_neon(const char *buf, int bytes, bits8 mask)
 	 */
 	for (; bytes >= sizeof(uint64); bytes -= sizeof(uint64))
 	{
-		popcnt += pg_popcount64(*((const uint64 *) buf) & mask64);
+		popcnt += pg_popcount64_neon(*((const uint64 *) buf) & mask64);
 		buf += sizeof(uint64);
 	}
 
diff --git a/src/port/pg_popcount_x86.c b/src/port/pg_popcount_x86.c
index 7aebf69898b..6bce089432f 100644
--- a/src/port/pg_popcount_x86.c
+++ b/src/port/pg_popcount_x86.c
@@ -36,8 +36,6 @@
  * operation, but in practice this is close enough, and "sse42" seems easier to
  * follow than "popcnt" for these names.
  */
-static inline int pg_popcount32_sse42(uint32 word);
-static inline int pg_popcount64_sse42(uint64 word);
 static uint64 pg_popcount_sse42(const char *buf, int bytes);
 static uint64 pg_popcount_masked_sse42(const char *buf, int bytes, bits8 mask);
 
@@ -55,12 +53,8 @@ static uint64 pg_popcount_masked_avx512(const char *buf, int bytes, bits8 mask);
  * what the current CPU supports) and then will call the pointer to fulfill the
  * caller's request.
  */
-static int	pg_popcount32_choose(uint32 word);
-static int	pg_popcount64_choose(uint64 word);
 static uint64 pg_popcount_choose(const char *buf, int bytes);
 static uint64 pg_popcount_masked_choose(const char *buf, int bytes, bits8 mask);
-int			(*pg_popcount32) (uint32 word) = pg_popcount32_choose;
-int			(*pg_popcount64) (uint64 word) = pg_popcount64_choose;
 uint64		(*pg_popcount_optimized) (const char *buf, int bytes) = pg_popcount_choose;
 uint64		(*pg_popcount_masked_optimized) (const char *buf, int bytes, bits8 mask) = pg_popcount_masked_choose;
 
@@ -157,7 +151,7 @@ pg_popcount_avx512_available(void)
 #endif							/* USE_AVX512_POPCNT_WITH_RUNTIME_CHECK */
 
 /*
- * These functions get called on the first call to pg_popcount32 etc.
+ * These functions get called on the first call to pg_popcount(), etc.
  * They detect whether we can use the asm implementations, and replace
  * the function pointers so that subsequent calls are routed directly to
  * the chosen implementation.
@@ -167,15 +161,11 @@ choose_popcount_functions(void)
 {
 	if (pg_popcount_sse42_available())
 	{
-		pg_popcount32 = pg_popcount32_sse42;
-		pg_popcount64 = pg_popcount64_sse42;
 		pg_popcount_optimized = pg_popcount_sse42;
 		pg_popcount_masked_optimized = pg_popcount_masked_sse42;
 	}
 	else
 	{
-		pg_popcount32 = pg_popcount32_portable;
-		pg_popcount64 = pg_popcount64_portable;
 		pg_popcount_optimized = pg_popcount_portable;
 		pg_popcount_masked_optimized = pg_popcount_masked_portable;
 	}
@@ -189,20 +179,6 @@ choose_popcount_functions(void)
 #endif
 }
 
-static int
-pg_popcount32_choose(uint32 word)
-{
-	choose_popcount_functions();
-	return pg_popcount32(word);
-}
-
-static int
-pg_popcount64_choose(uint64 word)
-{
-	choose_popcount_functions();
-	return pg_popcount64(word);
-}
-
 static uint64
 pg_popcount_choose(const char *buf, int bytes)
 {
@@ -338,23 +314,6 @@ pg_popcount_masked_avx512(const char *buf, int bytes, bits8 mask)
 
 #endif							/* USE_AVX512_POPCNT_WITH_RUNTIME_CHECK */
 
-/*
- * pg_popcount32_sse42
- *		Return the number of 1 bits set in word
- */
-static inline int
-pg_popcount32_sse42(uint32 word)
-{
-#ifdef _MSC_VER
-	return __popcnt(word);
-#else
-	uint32		res;
-
-__asm__ __volatile__(" popcntl %1,%0\n":"=q"(res):"rm"(word):"cc");
-	return (int) res;
-#endif
-}
-
 /*
  * pg_popcount64_sse42
  *		Return the number of 1 bits set in word
-- 
2.50.1 (Apple Git-155)


--rlwGoOKBy5dABfM4
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
	filename=v10-0003-Make-use-of-pg_popcount-in-more-places.patch



^ permalink  raw  reply  [nested|flat] 19+ messages in thread

* [PATCH v5 3/3] Remove specialized word-length popcount implementations.
@ 2026-01-23 23:31 Nathan Bossart <nathan@postgresql.org>
  0 siblings, 0 replies; 19+ messages in thread

From: Nathan Bossart @ 2026-01-23 23:31 UTC (permalink / raw)

---
 src/include/port/pg_bitutils.h | 75 +++++++++++++++++++++++-----------
 src/port/pg_bitutils.c         | 69 ++-----------------------------
 src/port/pg_popcount_aarch64.c | 25 ------------
 src/port/pg_popcount_x86.c     | 43 +------------------
 4 files changed, 56 insertions(+), 156 deletions(-)

diff --git a/src/include/port/pg_bitutils.h b/src/include/port/pg_bitutils.h
index 35761f509ec..a3e0f346ef6 100644
--- a/src/include/port/pg_bitutils.h
+++ b/src/include/port/pg_bitutils.h
@@ -276,46 +276,73 @@ pg_ceil_log2_64(uint64 num)
 		return pg_leftmost_one_pos64(num - 1) + 1;
 }
 
-extern int	pg_popcount32_portable(uint32 word);
-extern int	pg_popcount64_portable(uint64 word);
 extern uint64 pg_popcount_portable(const char *buf, int bytes);
 extern uint64 pg_popcount_masked_portable(const char *buf, int bytes, bits8 mask);
 
-#ifdef HAVE_X86_64_POPCNTQ
+#if defined(HAVE_X86_64_POPCNTQ) || defined(USE_SVE_POPCNT_WITH_RUNTIME_CHECK)
 /*
- * Attempt to use SSE4.2 or AVX-512 instructions, but perform a runtime check
+ * Attempt to use specialized CPU instructions, but perform a runtime check
  * first.
  */
-extern PGDLLIMPORT int (*pg_popcount32) (uint32 word);
-extern PGDLLIMPORT int (*pg_popcount64) (uint64 word);
 extern PGDLLIMPORT uint64 (*pg_popcount_optimized) (const char *buf, int bytes);
 extern PGDLLIMPORT uint64 (*pg_popcount_masked_optimized) (const char *buf, int bytes, bits8 mask);
 
-#elif defined(USE_NEON)
-/* Use the Neon version of pg_popcount{32,64} without function pointer. */
-extern int	pg_popcount32(uint32 word);
-extern int	pg_popcount64(uint64 word);
-
-/*
- * We can try to use an SVE-optimized pg_popcount() on some systems  For that,
- * we do use a function pointer.
- */
-#ifdef USE_SVE_POPCNT_WITH_RUNTIME_CHECK
-extern PGDLLIMPORT uint64 (*pg_popcount_optimized) (const char *buf, int bytes);
-extern PGDLLIMPORT uint64 (*pg_popcount_masked_optimized) (const char *buf, int bytes, bits8 mask);
 #else
+/* Use a portable implementation -- no need for a function pointer. */
 extern uint64 pg_popcount_optimized(const char *buf, int bytes);
 extern uint64 pg_popcount_masked_optimized(const char *buf, int bytes, bits8 mask);
+
 #endif
 
-#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_optimized(const char *buf, int bytes);
-extern uint64 pg_popcount_masked_optimized(const char *buf, int bytes, bits8 mask);
+/*
+ * pg_popcount32
+ *		Return the number of 1 bits set in word
+ */
+static inline int
+pg_popcount32(uint32 word)
+{
+#ifdef HAVE__BUILTIN_POPCOUNT
+	return __builtin_popcount(word);
+#else							/* !HAVE__BUILTIN_POPCOUNT */
+	int			result = 0;
+
+	while (word != 0)
+	{
+		result += pg_number_of_ones[word & 255];
+		word >>= 8;
+	}
 
+	return result;
+#endif							/* HAVE__BUILTIN_POPCOUNT */
+}
+
+/*
+ * pg_popcount64
+ *		Return the number of 1 bits set in word
+ */
+static inline int
+pg_popcount64(uint64 word)
+{
+#ifdef HAVE__BUILTIN_POPCOUNT
+#if SIZEOF_LONG == 8
+	return __builtin_popcountl(word);
+#elif SIZEOF_LONG_LONG == 8
+	return __builtin_popcountll(word);
+#else
+#error "cannot find integer of the same size as uint64_t"
 #endif
+#else							/* !HAVE__BUILTIN_POPCOUNT */
+	int			result = 0;
+
+	while (word != 0)
+	{
+		result += pg_number_of_ones[word & 255];
+		word >>= 8;
+	}
+
+	return result;
+#endif							/* HAVE__BUILTIN_POPCOUNT */
+}
 
 /*
  * Returns the number of 1-bits in buf.
diff --git a/src/port/pg_bitutils.c b/src/port/pg_bitutils.c
index ffda75825e5..f942c1be55a 100644
--- a/src/port/pg_bitutils.c
+++ b/src/port/pg_bitutils.c
@@ -96,56 +96,6 @@ const uint8 pg_number_of_ones[256] = {
 	4, 5, 5, 6, 5, 6, 6, 7, 5, 6, 6, 7, 6, 7, 7, 8
 };
 
-/*
- * pg_popcount32_portable
- *		Return the number of 1 bits set in word
- */
-int
-pg_popcount32_portable(uint32 word)
-{
-#ifdef HAVE__BUILTIN_POPCOUNT
-	return __builtin_popcount(word);
-#else							/* !HAVE__BUILTIN_POPCOUNT */
-	int			result = 0;
-
-	while (word != 0)
-	{
-		result += pg_number_of_ones[word & 255];
-		word >>= 8;
-	}
-
-	return result;
-#endif							/* HAVE__BUILTIN_POPCOUNT */
-}
-
-/*
- * pg_popcount64_portable
- *		Return the number of 1 bits set in word
- */
-int
-pg_popcount64_portable(uint64 word)
-{
-#ifdef HAVE__BUILTIN_POPCOUNT
-#if SIZEOF_LONG == 8
-	return __builtin_popcountl(word);
-#elif SIZEOF_LONG_LONG == 8
-	return __builtin_popcountll(word);
-#else
-#error "cannot find integer of the same size as uint64_t"
-#endif
-#else							/* !HAVE__BUILTIN_POPCOUNT */
-	int			result = 0;
-
-	while (word != 0)
-	{
-		result += pg_number_of_ones[word & 255];
-		word >>= 8;
-	}
-
-	return result;
-#endif							/* HAVE__BUILTIN_POPCOUNT */
-}
-
 /*
  * pg_popcount_portable
  *		Returns the number of 1-bits in buf
@@ -163,7 +113,7 @@ pg_popcount_portable(const char *buf, int bytes)
 
 		while (bytes >= 8)
 		{
-			popcnt += pg_popcount64_portable(*words++);
+			popcnt += pg_popcount64(*words++);
 			bytes -= 8;
 		}
 
@@ -177,7 +127,7 @@ pg_popcount_portable(const char *buf, int bytes)
 
 		while (bytes >= 4)
 		{
-			popcnt += pg_popcount32_portable(*words++);
+			popcnt += pg_popcount32(*words++);
 			bytes -= 4;
 		}
 
@@ -211,7 +161,7 @@ pg_popcount_masked_portable(const char *buf, int bytes, bits8 mask)
 
 		while (bytes >= 8)
 		{
-			popcnt += pg_popcount64_portable(*words++ & maskv);
+			popcnt += pg_popcount64(*words++ & maskv);
 			bytes -= 8;
 		}
 
@@ -227,7 +177,7 @@ pg_popcount_masked_portable(const char *buf, int bytes, bits8 mask)
 
 		while (bytes >= 4)
 		{
-			popcnt += pg_popcount32_portable(*words++ & maskv);
+			popcnt += pg_popcount32(*words++ & maskv);
 			bytes -= 4;
 		}
 
@@ -250,17 +200,6 @@ pg_popcount_masked_portable(const char *buf, int bytes, bits8 mask)
  * actual external functions.  The compiler should be able to inline the
  * portable versions here.
  */
-int
-pg_popcount32(uint32 word)
-{
-	return pg_popcount32_portable(word);
-}
-
-int
-pg_popcount64(uint64 word)
-{
-	return pg_popcount64_portable(word);
-}
 
 /*
  * pg_popcount_optimized
diff --git a/src/port/pg_popcount_aarch64.c b/src/port/pg_popcount_aarch64.c
index ba57f2cd4bd..74f71593721 100644
--- a/src/port/pg_popcount_aarch64.c
+++ b/src/port/pg_popcount_aarch64.c
@@ -291,31 +291,6 @@ pg_popcount_masked_optimized(const char *buf, int bytes, bits8 mask)
 
 #endif							/* ! USE_SVE_POPCNT_WITH_RUNTIME_CHECK */
 
-/*
- * pg_popcount32
- *		Return number of 1 bits in word
- */
-int
-pg_popcount32(uint32 word)
-{
-	return pg_popcount64((uint64) word);
-}
-
-/*
- * pg_popcount64
- *		Return number of 1 bits in word
- */
-int
-pg_popcount64(uint64 word)
-{
-	/*
-	 * For some compilers, __builtin_popcountl() already emits Neon
-	 * instructions.  The line below should compile to the same code on those
-	 * systems.
-	 */
-	return vaddv_u8(vcnt_u8(vld1_u8((const uint8 *) &word)));
-}
-
 /*
  * pg_popcount_neon
  *		Returns number of 1 bits in buf
diff --git a/src/port/pg_popcount_x86.c b/src/port/pg_popcount_x86.c
index 0e98f532552..9fd8e18ed16 100644
--- a/src/port/pg_popcount_x86.c
+++ b/src/port/pg_popcount_x86.c
@@ -36,8 +36,6 @@
  * operation, but in practice this is close enough, and "sse42" seems easier to
  * follow than "popcnt" for these names.
  */
-static inline int pg_popcount32_sse42(uint32 word);
-static inline int pg_popcount64_sse42(uint64 word);
 static uint64 pg_popcount_sse42(const char *buf, int bytes);
 static uint64 pg_popcount_masked_sse42(const char *buf, int bytes, bits8 mask);
 
@@ -55,12 +53,8 @@ static uint64 pg_popcount_masked_avx512(const char *buf, int bytes, bits8 mask);
  * what the current CPU supports) and then will call the pointer to fulfill the
  * caller's request.
  */
-static int	pg_popcount32_choose(uint32 word);
-static int	pg_popcount64_choose(uint64 word);
 static uint64 pg_popcount_choose(const char *buf, int bytes);
 static uint64 pg_popcount_masked_choose(const char *buf, int bytes, bits8 mask);
-int			(*pg_popcount32) (uint32 word) = pg_popcount32_choose;
-int			(*pg_popcount64) (uint64 word) = pg_popcount64_choose;
 uint64		(*pg_popcount_optimized) (const char *buf, int bytes) = pg_popcount_choose;
 uint64		(*pg_popcount_masked_optimized) (const char *buf, int bytes, bits8 mask) = pg_popcount_masked_choose;
 
@@ -157,7 +151,7 @@ pg_popcount_avx512_available(void)
 #endif							/* USE_AVX512_POPCNT_WITH_RUNTIME_CHECK */
 
 /*
- * These functions get called on the first call to pg_popcount32 etc.
+ * These functions get called on the first call to pg_popcount(), etc.
  * They detect whether we can use the asm implementations, and replace
  * the function pointers so that subsequent calls are routed directly to
  * the chosen implementation.
@@ -167,15 +161,11 @@ choose_popcount_functions(void)
 {
 	if (pg_popcount_sse42_available())
 	{
-		pg_popcount32 = pg_popcount32_sse42;
-		pg_popcount64 = pg_popcount64_sse42;
 		pg_popcount_optimized = pg_popcount_sse42;
 		pg_popcount_masked_optimized = pg_popcount_masked_sse42;
 	}
 	else
 	{
-		pg_popcount32 = pg_popcount32_portable;
-		pg_popcount64 = pg_popcount64_portable;
 		pg_popcount_optimized = pg_popcount_portable;
 		pg_popcount_masked_optimized = pg_popcount_masked_portable;
 	}
@@ -189,20 +179,6 @@ choose_popcount_functions(void)
 #endif
 }
 
-static int
-pg_popcount32_choose(uint32 word)
-{
-	choose_popcount_functions();
-	return pg_popcount32(word);
-}
-
-static int
-pg_popcount64_choose(uint64 word)
-{
-	choose_popcount_functions();
-	return pg_popcount64(word);
-}
-
 static uint64
 pg_popcount_choose(const char *buf, int bytes)
 {
@@ -338,23 +314,6 @@ pg_popcount_masked_avx512(const char *buf, int bytes, bits8 mask)
 
 #endif							/* USE_AVX512_POPCNT_WITH_RUNTIME_CHECK */
 
-/*
- * pg_popcount32_sse42
- *		Return the number of 1 bits set in word
- */
-static inline int
-pg_popcount32_sse42(uint32 word)
-{
-#ifdef _MSC_VER
-	return __popcnt(word);
-#else
-	uint32		res;
-
-__asm__ __volatile__(" popcntl %1,%0\n":"=q"(res):"rm"(word):"cc");
-	return (int) res;
-#endif
-}
-
 /*
  * pg_popcount64_sse42
  *		Return the number of 1 bits set in word
-- 
2.50.1 (Apple Git-155)


--a4C7SGgzwrPm67dM--





^ permalink  raw  reply  [nested|flat] 19+ messages in thread

* [PATCH v13 2/5] Remove specialized word-length popcount implementations.
@ 2026-01-23 23:31 Nathan Bossart <nathan@postgresql.org>
  0 siblings, 0 replies; 19+ messages in thread

From: Nathan Bossart @ 2026-01-23 23:31 UTC (permalink / raw)

The uses of these functions do not justify the level of
micro-optimization we've done and may even hurt performance in some
cases (e.g., due to using function pointers).  This commit removes
all architecture-specific implementations of pg_popcount{32,64}()
and converts the portable ones to inlined functions in
pg_bitutils.h.  These inlined versions should produce the same code
as before (but inlined), so in theory this is a net gain for many
machines.

Suggested-by: John Naylor <johncnaylorls@gmail.com>
Reviewed-by: John Naylor <johncnaylorls@gmail.com>
Reviewed-by: Greg Burd <greg@burd.me>
Discussion: https://postgr.es/m/CANWCAZY7R%2Biy%2Br9YM_sySNydHzNqUirx1xk0tB3ej5HO62GdgQ%40mail.gmail.com
---
 src/include/port/pg_bitutils.h | 75 +++++++++++++++++++++++-----------
 src/port/pg_bitutils.c         | 65 +----------------------------
 src/port/pg_popcount_aarch64.c | 20 +++------
 src/port/pg_popcount_x86.c     | 43 +------------------
 4 files changed, 59 insertions(+), 144 deletions(-)

diff --git a/src/include/port/pg_bitutils.h b/src/include/port/pg_bitutils.h
index 20c11b79c61..789663edd93 100644
--- a/src/include/port/pg_bitutils.h
+++ b/src/include/port/pg_bitutils.h
@@ -276,46 +276,73 @@ pg_ceil_log2_64(uint64 num)
 		return pg_leftmost_one_pos64(num - 1) + 1;
 }
 
-extern int	pg_popcount32_portable(uint32 word);
-extern int	pg_popcount64_portable(uint64 word);
 extern uint64 pg_popcount_portable(const char *buf, int bytes);
 extern uint64 pg_popcount_masked_portable(const char *buf, int bytes, bits8 mask);
 
-#ifdef HAVE_X86_64_POPCNTQ
+#if defined(HAVE_X86_64_POPCNTQ) || defined(USE_SVE_POPCNT_WITH_RUNTIME_CHECK)
 /*
- * Attempt to use SSE4.2 or AVX-512 instructions, but perform a runtime check
+ * Attempt to use specialized CPU instructions, but perform a runtime check
  * first.
  */
-extern PGDLLIMPORT int (*pg_popcount32) (uint32 word);
-extern PGDLLIMPORT int (*pg_popcount64) (uint64 word);
 extern PGDLLIMPORT uint64 (*pg_popcount_optimized) (const char *buf, int bytes);
 extern PGDLLIMPORT uint64 (*pg_popcount_masked_optimized) (const char *buf, int bytes, bits8 mask);
 
-#elif defined(USE_NEON)
-/* Use the Neon version of pg_popcount{32,64} without function pointer. */
-extern int	pg_popcount32(uint32 word);
-extern int	pg_popcount64(uint64 word);
-
-/*
- * We can try to use an SVE-optimized pg_popcount() on some systems  For that,
- * we do use a function pointer.
- */
-#ifdef USE_SVE_POPCNT_WITH_RUNTIME_CHECK
-extern PGDLLIMPORT uint64 (*pg_popcount_optimized) (const char *buf, int bytes);
-extern PGDLLIMPORT uint64 (*pg_popcount_masked_optimized) (const char *buf, int bytes, bits8 mask);
 #else
+/* Use a portable implementation -- no need for a function pointer. */
 extern uint64 pg_popcount_optimized(const char *buf, int bytes);
 extern uint64 pg_popcount_masked_optimized(const char *buf, int bytes, bits8 mask);
+
 #endif
 
-#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_optimized(const char *buf, int bytes);
-extern uint64 pg_popcount_masked_optimized(const char *buf, int bytes, bits8 mask);
+/*
+ * pg_popcount32
+ *		Return the number of 1 bits set in word
+ */
+static inline int
+pg_popcount32(uint32 word)
+{
+#ifdef HAVE__BUILTIN_POPCOUNT
+	return __builtin_popcount(word);
+#else							/* !HAVE__BUILTIN_POPCOUNT */
+	int			result = 0;
+
+	while (word != 0)
+	{
+		result += pg_number_of_ones[word & 255];
+		word >>= 8;
+	}
 
+	return result;
+#endif							/* HAVE__BUILTIN_POPCOUNT */
+}
+
+/*
+ * pg_popcount64
+ *		Return the number of 1 bits set in word
+ */
+static inline int
+pg_popcount64(uint64 word)
+{
+#ifdef HAVE__BUILTIN_POPCOUNT
+#if SIZEOF_LONG == 8
+	return __builtin_popcountl(word);
+#elif SIZEOF_LONG_LONG == 8
+	return __builtin_popcountll(word);
+#else
+#error "cannot find integer of the same size as uint64_t"
 #endif
+#else							/* !HAVE__BUILTIN_POPCOUNT */
+	int			result = 0;
+
+	while (word != 0)
+	{
+		result += pg_number_of_ones[word & 255];
+		word >>= 8;
+	}
+
+	return result;
+#endif							/* HAVE__BUILTIN_POPCOUNT */
+}
 
 /*
  * Returns the number of 1-bits in buf.
diff --git a/src/port/pg_bitutils.c b/src/port/pg_bitutils.c
index bec06c06fc3..49b130f1306 100644
--- a/src/port/pg_bitutils.c
+++ b/src/port/pg_bitutils.c
@@ -96,56 +96,6 @@ const uint8 pg_number_of_ones[256] = {
 	4, 5, 5, 6, 5, 6, 6, 7, 5, 6, 6, 7, 6, 7, 7, 8
 };
 
-/*
- * pg_popcount32_portable
- *		Return the number of 1 bits set in word
- */
-int
-pg_popcount32_portable(uint32 word)
-{
-#ifdef HAVE__BUILTIN_POPCOUNT
-	return __builtin_popcount(word);
-#else							/* !HAVE__BUILTIN_POPCOUNT */
-	int			result = 0;
-
-	while (word != 0)
-	{
-		result += pg_number_of_ones[word & 255];
-		word >>= 8;
-	}
-
-	return result;
-#endif							/* HAVE__BUILTIN_POPCOUNT */
-}
-
-/*
- * pg_popcount64_portable
- *		Return the number of 1 bits set in word
- */
-int
-pg_popcount64_portable(uint64 word)
-{
-#ifdef HAVE__BUILTIN_POPCOUNT
-#if SIZEOF_LONG == 8
-	return __builtin_popcountl(word);
-#elif SIZEOF_LONG_LONG == 8
-	return __builtin_popcountll(word);
-#else
-#error "cannot find integer of the same size as uint64_t"
-#endif
-#else							/* !HAVE__BUILTIN_POPCOUNT */
-	int			result = 0;
-
-	while (word != 0)
-	{
-		result += pg_number_of_ones[word & 255];
-		word >>= 8;
-	}
-
-	return result;
-#endif							/* HAVE__BUILTIN_POPCOUNT */
-}
-
 /*
  * pg_popcount_portable
  *		Returns the number of 1-bits in buf
@@ -163,7 +113,7 @@ pg_popcount_portable(const char *buf, int bytes)
 
 		while (bytes >= 8)
 		{
-			popcnt += pg_popcount64_portable(*words++);
+			popcnt += pg_popcount64(*words++);
 			bytes -= 8;
 		}
 
@@ -197,7 +147,7 @@ pg_popcount_masked_portable(const char *buf, int bytes, bits8 mask)
 
 		while (bytes >= 8)
 		{
-			popcnt += pg_popcount64_portable(*words++ & maskv);
+			popcnt += pg_popcount64(*words++ & maskv);
 			bytes -= 8;
 		}
 
@@ -220,17 +170,6 @@ pg_popcount_masked_portable(const char *buf, int bytes, bits8 mask)
  * actual external functions.  The compiler should be able to inline the
  * portable versions here.
  */
-int
-pg_popcount32(uint32 word)
-{
-	return pg_popcount32_portable(word);
-}
-
-int
-pg_popcount64(uint64 word)
-{
-	return pg_popcount64_portable(word);
-}
 
 /*
  * pg_popcount_optimized
diff --git a/src/port/pg_popcount_aarch64.c b/src/port/pg_popcount_aarch64.c
index ba57f2cd4bd..f474ef45510 100644
--- a/src/port/pg_popcount_aarch64.c
+++ b/src/port/pg_popcount_aarch64.c
@@ -292,21 +292,11 @@ pg_popcount_masked_optimized(const char *buf, int bytes, bits8 mask)
 #endif							/* ! USE_SVE_POPCNT_WITH_RUNTIME_CHECK */
 
 /*
- * pg_popcount32
+ * pg_popcount64_neon
  *		Return number of 1 bits in word
  */
-int
-pg_popcount32(uint32 word)
-{
-	return pg_popcount64((uint64) word);
-}
-
-/*
- * pg_popcount64
- *		Return number of 1 bits in word
- */
-int
-pg_popcount64(uint64 word)
+static inline int
+pg_popcount64_neon(uint64 word)
 {
 	/*
 	 * For some compilers, __builtin_popcountl() already emits Neon
@@ -383,7 +373,7 @@ pg_popcount_neon(const char *buf, int bytes)
 	 */
 	for (; bytes >= sizeof(uint64); bytes -= sizeof(uint64))
 	{
-		popcnt += pg_popcount64(*((const uint64 *) buf));
+		popcnt += pg_popcount64_neon(*((const uint64 *) buf));
 		buf += sizeof(uint64);
 	}
 
@@ -465,7 +455,7 @@ pg_popcount_masked_neon(const char *buf, int bytes, bits8 mask)
 	 */
 	for (; bytes >= sizeof(uint64); bytes -= sizeof(uint64))
 	{
-		popcnt += pg_popcount64(*((const uint64 *) buf) & mask64);
+		popcnt += pg_popcount64_neon(*((const uint64 *) buf) & mask64);
 		buf += sizeof(uint64);
 	}
 
diff --git a/src/port/pg_popcount_x86.c b/src/port/pg_popcount_x86.c
index 7aebf69898b..6bce089432f 100644
--- a/src/port/pg_popcount_x86.c
+++ b/src/port/pg_popcount_x86.c
@@ -36,8 +36,6 @@
  * operation, but in practice this is close enough, and "sse42" seems easier to
  * follow than "popcnt" for these names.
  */
-static inline int pg_popcount32_sse42(uint32 word);
-static inline int pg_popcount64_sse42(uint64 word);
 static uint64 pg_popcount_sse42(const char *buf, int bytes);
 static uint64 pg_popcount_masked_sse42(const char *buf, int bytes, bits8 mask);
 
@@ -55,12 +53,8 @@ static uint64 pg_popcount_masked_avx512(const char *buf, int bytes, bits8 mask);
  * what the current CPU supports) and then will call the pointer to fulfill the
  * caller's request.
  */
-static int	pg_popcount32_choose(uint32 word);
-static int	pg_popcount64_choose(uint64 word);
 static uint64 pg_popcount_choose(const char *buf, int bytes);
 static uint64 pg_popcount_masked_choose(const char *buf, int bytes, bits8 mask);
-int			(*pg_popcount32) (uint32 word) = pg_popcount32_choose;
-int			(*pg_popcount64) (uint64 word) = pg_popcount64_choose;
 uint64		(*pg_popcount_optimized) (const char *buf, int bytes) = pg_popcount_choose;
 uint64		(*pg_popcount_masked_optimized) (const char *buf, int bytes, bits8 mask) = pg_popcount_masked_choose;
 
@@ -157,7 +151,7 @@ pg_popcount_avx512_available(void)
 #endif							/* USE_AVX512_POPCNT_WITH_RUNTIME_CHECK */
 
 /*
- * These functions get called on the first call to pg_popcount32 etc.
+ * These functions get called on the first call to pg_popcount(), etc.
  * They detect whether we can use the asm implementations, and replace
  * the function pointers so that subsequent calls are routed directly to
  * the chosen implementation.
@@ -167,15 +161,11 @@ choose_popcount_functions(void)
 {
 	if (pg_popcount_sse42_available())
 	{
-		pg_popcount32 = pg_popcount32_sse42;
-		pg_popcount64 = pg_popcount64_sse42;
 		pg_popcount_optimized = pg_popcount_sse42;
 		pg_popcount_masked_optimized = pg_popcount_masked_sse42;
 	}
 	else
 	{
-		pg_popcount32 = pg_popcount32_portable;
-		pg_popcount64 = pg_popcount64_portable;
 		pg_popcount_optimized = pg_popcount_portable;
 		pg_popcount_masked_optimized = pg_popcount_masked_portable;
 	}
@@ -189,20 +179,6 @@ choose_popcount_functions(void)
 #endif
 }
 
-static int
-pg_popcount32_choose(uint32 word)
-{
-	choose_popcount_functions();
-	return pg_popcount32(word);
-}
-
-static int
-pg_popcount64_choose(uint64 word)
-{
-	choose_popcount_functions();
-	return pg_popcount64(word);
-}
-
 static uint64
 pg_popcount_choose(const char *buf, int bytes)
 {
@@ -338,23 +314,6 @@ pg_popcount_masked_avx512(const char *buf, int bytes, bits8 mask)
 
 #endif							/* USE_AVX512_POPCNT_WITH_RUNTIME_CHECK */
 
-/*
- * pg_popcount32_sse42
- *		Return the number of 1 bits set in word
- */
-static inline int
-pg_popcount32_sse42(uint32 word)
-{
-#ifdef _MSC_VER
-	return __popcnt(word);
-#else
-	uint32		res;
-
-__asm__ __volatile__(" popcntl %1,%0\n":"=q"(res):"rm"(word):"cc");
-	return (int) res;
-#endif
-}
-
 /*
  * pg_popcount64_sse42
  *		Return the number of 1 bits set in word
-- 
2.50.1 (Apple Git-155)


--5NNwvjHYUnRaoEDo
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
	filename=v13-0003-Remove-uses-of-popcount-builtins.patch



^ permalink  raw  reply  [nested|flat] 19+ messages in thread

* [PATCH v9 2/3] Remove specialized word-length popcount implementations.
@ 2026-01-23 23:31 Nathan Bossart <nathan@postgresql.org>
  0 siblings, 0 replies; 19+ messages in thread

From: Nathan Bossart @ 2026-01-23 23:31 UTC (permalink / raw)

The uses of these functions do not justify the level of
micro-optimization we've done and may even hurt performance in some
cases (e.g., due to using function pointers).  This commit removes
all architecture-specific implementations of pg_popcount{32,64}()
and converts the portable ones to inlined functions in
pg_bitutils.h.  These inlined versions should produce the same code
as before (but inlined), so in theory this is a net gain for many
machines.  As an exception, for x86-64/gcc without sse4.2/popcnt,
we use a plain C version to ensure inlining because
__builtin_popcount() and __builtin_popcountl() generate function
calls for that configuration.  Our tests indicate this is still a
net win.

Suggested-by: John Naylor <johncnaylorls@gmail.com>
Reviewed-by: John Naylor <johncnaylorls@gmail.com>
Reviewed-by: Greg Burd <greg@burd.me>
Discussion: https://postgr.es/m/CANWCAZY7R%2Biy%2Br9YM_sySNydHzNqUirx1xk0tB3ej5HO62GdgQ%40mail.gmail.com
---
 src/include/port/pg_bitutils.h | 83 ++++++++++++++++++++++++----------
 src/port/pg_bitutils.c         | 65 +-------------------------
 src/port/pg_popcount_aarch64.c | 25 ----------
 src/port/pg_popcount_x86.c     | 43 +-----------------
 4 files changed, 63 insertions(+), 153 deletions(-)

diff --git a/src/include/port/pg_bitutils.h b/src/include/port/pg_bitutils.h
index c3049d71894..a5c2673db51 100644
--- a/src/include/port/pg_bitutils.h
+++ b/src/include/port/pg_bitutils.h
@@ -276,46 +276,83 @@ pg_ceil_log2_64(uint64 num)
 		return pg_leftmost_one_pos64(num - 1) + 1;
 }
 
-extern int	pg_popcount32_portable(uint32 word);
-extern int	pg_popcount64_portable(uint64 word);
 extern uint64 pg_popcount_portable(const char *buf, int bytes);
 extern uint64 pg_popcount_masked_portable(const char *buf, int bytes, bits8 mask);
 
-#ifdef HAVE_X86_64_POPCNTQ
+#if defined(HAVE_X86_64_POPCNTQ) || defined(USE_SVE_POPCNT_WITH_RUNTIME_CHECK)
 /*
- * Attempt to use SSE4.2 or AVX-512 instructions, but perform a runtime check
+ * Attempt to use specialized CPU instructions, but perform a runtime check
  * first.
  */
-extern PGDLLIMPORT int (*pg_popcount32) (uint32 word);
-extern PGDLLIMPORT int (*pg_popcount64) (uint64 word);
 extern PGDLLIMPORT uint64 (*pg_popcount_optimized) (const char *buf, int bytes);
 extern PGDLLIMPORT uint64 (*pg_popcount_masked_optimized) (const char *buf, int bytes, bits8 mask);
 
-#elif defined(USE_NEON)
-/* Use the Neon version of pg_popcount{32,64} without function pointer. */
-extern int	pg_popcount32(uint32 word);
-extern int	pg_popcount64(uint64 word);
-
-/*
- * We can try to use an SVE-optimized pg_popcount() on some systems  For that,
- * we do use a function pointer.
- */
-#ifdef USE_SVE_POPCNT_WITH_RUNTIME_CHECK
-extern PGDLLIMPORT uint64 (*pg_popcount_optimized) (const char *buf, int bytes);
-extern PGDLLIMPORT uint64 (*pg_popcount_masked_optimized) (const char *buf, int bytes, bits8 mask);
 #else
+/* Use a portable implementation -- no need for a function pointer. */
 extern uint64 pg_popcount_optimized(const char *buf, int bytes);
 extern uint64 pg_popcount_masked_optimized(const char *buf, int bytes, bits8 mask);
+
 #endif
 
+/*
+ * pg_popcount32
+ *		Return the number of 1 bits set in word
+ *
+ * Plain C version adapted from
+ * https://graphics.stanford.edu/~seander/bithacks.html#CountBitsSetParallel.
+ */
+static inline int
+pg_popcount32(uint32 word)
+{
+	/*
+	 * On x86, gcc generates a function call for this built-in unless the
+	 * popcnt instruction is available, so we use the plain C version in that
+	 * case to ensure inlining.
+	 */
+#if defined(HAVE__BUILTIN_POPCOUNT) && (defined(__POPCNT__) || !defined(__x86_64__))
+	return __builtin_popcount(word);
+#elif defined(_MSC_VER)
+	return __popcnt(word);
 #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_optimized(const char *buf, int bytes);
-extern uint64 pg_popcount_masked_optimized(const char *buf, int bytes, bits8 mask);
+	word -= (word >> 1) & 0x55555555;
+	word = (word & 0x33333333) + ((word >> 2) & 0x33333333);
+	return (((word + (word >> 4)) & 0xf0f0f0f) * 0x1010101) >> 24;
+#endif
+}
 
+/*
+ * pg_popcount64
+ *		Return the number of 1 bits set in word
+ *
+ * Plain C version adapted from
+ * https://graphics.stanford.edu/~seander/bithacks.html#CountBitsSetParallel.
+ */
+static inline int
+pg_popcount64(uint64 word)
+{
+	/*
+	 * On x86, gcc generates a function call for this built-in unless the
+	 * popcnt instruction is available, so we use the plain C version in that
+	 * case to ensure inlining.
+	 */
+#if defined(HAVE__BUILTIN_POPCOUNT) && (defined(__POPCNT__) || !defined(__x86_64__))
+#if SIZEOF_LONG == 8
+	return __builtin_popcountl(word);
+#elif SIZEOF_LONG_LONG == 8
+	return __builtin_popcountll(word);
+#else
+#error "cannot find integer of the same size as uint64_t"
 #endif
+#elif defined(_MSC_VER)
+	return __popcnt64(word);
+#else
+	word -= (word >> 1) & UINT64CONST(0x5555555555555555);
+	word = (word & UINT64CONST(0x3333333333333333)) +
+		((word >> 2) & UINT64CONST(0x3333333333333333));
+	word = (word + (word >> 4)) & UINT64CONST(0xf0f0f0f0f0f0f0f);
+	return (word * UINT64CONST(0x101010101010101)) >> 56;
+#endif
+}
 
 /*
  * Returns the number of 1-bits in buf.
diff --git a/src/port/pg_bitutils.c b/src/port/pg_bitutils.c
index bec06c06fc3..49b130f1306 100644
--- a/src/port/pg_bitutils.c
+++ b/src/port/pg_bitutils.c
@@ -96,56 +96,6 @@ const uint8 pg_number_of_ones[256] = {
 	4, 5, 5, 6, 5, 6, 6, 7, 5, 6, 6, 7, 6, 7, 7, 8
 };
 
-/*
- * pg_popcount32_portable
- *		Return the number of 1 bits set in word
- */
-int
-pg_popcount32_portable(uint32 word)
-{
-#ifdef HAVE__BUILTIN_POPCOUNT
-	return __builtin_popcount(word);
-#else							/* !HAVE__BUILTIN_POPCOUNT */
-	int			result = 0;
-
-	while (word != 0)
-	{
-		result += pg_number_of_ones[word & 255];
-		word >>= 8;
-	}
-
-	return result;
-#endif							/* HAVE__BUILTIN_POPCOUNT */
-}
-
-/*
- * pg_popcount64_portable
- *		Return the number of 1 bits set in word
- */
-int
-pg_popcount64_portable(uint64 word)
-{
-#ifdef HAVE__BUILTIN_POPCOUNT
-#if SIZEOF_LONG == 8
-	return __builtin_popcountl(word);
-#elif SIZEOF_LONG_LONG == 8
-	return __builtin_popcountll(word);
-#else
-#error "cannot find integer of the same size as uint64_t"
-#endif
-#else							/* !HAVE__BUILTIN_POPCOUNT */
-	int			result = 0;
-
-	while (word != 0)
-	{
-		result += pg_number_of_ones[word & 255];
-		word >>= 8;
-	}
-
-	return result;
-#endif							/* HAVE__BUILTIN_POPCOUNT */
-}
-
 /*
  * pg_popcount_portable
  *		Returns the number of 1-bits in buf
@@ -163,7 +113,7 @@ pg_popcount_portable(const char *buf, int bytes)
 
 		while (bytes >= 8)
 		{
-			popcnt += pg_popcount64_portable(*words++);
+			popcnt += pg_popcount64(*words++);
 			bytes -= 8;
 		}
 
@@ -197,7 +147,7 @@ pg_popcount_masked_portable(const char *buf, int bytes, bits8 mask)
 
 		while (bytes >= 8)
 		{
-			popcnt += pg_popcount64_portable(*words++ & maskv);
+			popcnt += pg_popcount64(*words++ & maskv);
 			bytes -= 8;
 		}
 
@@ -220,17 +170,6 @@ pg_popcount_masked_portable(const char *buf, int bytes, bits8 mask)
  * actual external functions.  The compiler should be able to inline the
  * portable versions here.
  */
-int
-pg_popcount32(uint32 word)
-{
-	return pg_popcount32_portable(word);
-}
-
-int
-pg_popcount64(uint64 word)
-{
-	return pg_popcount64_portable(word);
-}
 
 /*
  * pg_popcount_optimized
diff --git a/src/port/pg_popcount_aarch64.c b/src/port/pg_popcount_aarch64.c
index ba57f2cd4bd..74f71593721 100644
--- a/src/port/pg_popcount_aarch64.c
+++ b/src/port/pg_popcount_aarch64.c
@@ -291,31 +291,6 @@ pg_popcount_masked_optimized(const char *buf, int bytes, bits8 mask)
 
 #endif							/* ! USE_SVE_POPCNT_WITH_RUNTIME_CHECK */
 
-/*
- * pg_popcount32
- *		Return number of 1 bits in word
- */
-int
-pg_popcount32(uint32 word)
-{
-	return pg_popcount64((uint64) word);
-}
-
-/*
- * pg_popcount64
- *		Return number of 1 bits in word
- */
-int
-pg_popcount64(uint64 word)
-{
-	/*
-	 * For some compilers, __builtin_popcountl() already emits Neon
-	 * instructions.  The line below should compile to the same code on those
-	 * systems.
-	 */
-	return vaddv_u8(vcnt_u8(vld1_u8((const uint8 *) &word)));
-}
-
 /*
  * pg_popcount_neon
  *		Returns number of 1 bits in buf
diff --git a/src/port/pg_popcount_x86.c b/src/port/pg_popcount_x86.c
index 7aebf69898b..6bce089432f 100644
--- a/src/port/pg_popcount_x86.c
+++ b/src/port/pg_popcount_x86.c
@@ -36,8 +36,6 @@
  * operation, but in practice this is close enough, and "sse42" seems easier to
  * follow than "popcnt" for these names.
  */
-static inline int pg_popcount32_sse42(uint32 word);
-static inline int pg_popcount64_sse42(uint64 word);
 static uint64 pg_popcount_sse42(const char *buf, int bytes);
 static uint64 pg_popcount_masked_sse42(const char *buf, int bytes, bits8 mask);
 
@@ -55,12 +53,8 @@ static uint64 pg_popcount_masked_avx512(const char *buf, int bytes, bits8 mask);
  * what the current CPU supports) and then will call the pointer to fulfill the
  * caller's request.
  */
-static int	pg_popcount32_choose(uint32 word);
-static int	pg_popcount64_choose(uint64 word);
 static uint64 pg_popcount_choose(const char *buf, int bytes);
 static uint64 pg_popcount_masked_choose(const char *buf, int bytes, bits8 mask);
-int			(*pg_popcount32) (uint32 word) = pg_popcount32_choose;
-int			(*pg_popcount64) (uint64 word) = pg_popcount64_choose;
 uint64		(*pg_popcount_optimized) (const char *buf, int bytes) = pg_popcount_choose;
 uint64		(*pg_popcount_masked_optimized) (const char *buf, int bytes, bits8 mask) = pg_popcount_masked_choose;
 
@@ -157,7 +151,7 @@ pg_popcount_avx512_available(void)
 #endif							/* USE_AVX512_POPCNT_WITH_RUNTIME_CHECK */
 
 /*
- * These functions get called on the first call to pg_popcount32 etc.
+ * These functions get called on the first call to pg_popcount(), etc.
  * They detect whether we can use the asm implementations, and replace
  * the function pointers so that subsequent calls are routed directly to
  * the chosen implementation.
@@ -167,15 +161,11 @@ choose_popcount_functions(void)
 {
 	if (pg_popcount_sse42_available())
 	{
-		pg_popcount32 = pg_popcount32_sse42;
-		pg_popcount64 = pg_popcount64_sse42;
 		pg_popcount_optimized = pg_popcount_sse42;
 		pg_popcount_masked_optimized = pg_popcount_masked_sse42;
 	}
 	else
 	{
-		pg_popcount32 = pg_popcount32_portable;
-		pg_popcount64 = pg_popcount64_portable;
 		pg_popcount_optimized = pg_popcount_portable;
 		pg_popcount_masked_optimized = pg_popcount_masked_portable;
 	}
@@ -189,20 +179,6 @@ choose_popcount_functions(void)
 #endif
 }
 
-static int
-pg_popcount32_choose(uint32 word)
-{
-	choose_popcount_functions();
-	return pg_popcount32(word);
-}
-
-static int
-pg_popcount64_choose(uint64 word)
-{
-	choose_popcount_functions();
-	return pg_popcount64(word);
-}
-
 static uint64
 pg_popcount_choose(const char *buf, int bytes)
 {
@@ -338,23 +314,6 @@ pg_popcount_masked_avx512(const char *buf, int bytes, bits8 mask)
 
 #endif							/* USE_AVX512_POPCNT_WITH_RUNTIME_CHECK */
 
-/*
- * pg_popcount32_sse42
- *		Return the number of 1 bits set in word
- */
-static inline int
-pg_popcount32_sse42(uint32 word)
-{
-#ifdef _MSC_VER
-	return __popcnt(word);
-#else
-	uint32		res;
-
-__asm__ __volatile__(" popcntl %1,%0\n":"=q"(res):"rm"(word):"cc");
-	return (int) res;
-#endif
-}
-
 /*
  * pg_popcount64_sse42
  *		Return the number of 1 bits set in word
-- 
2.50.1 (Apple Git-155)


--oEt0RkbzovU82iL7
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
	filename=v9-0003-Make-use-of-pg_popcount-in-more-places.patch



^ permalink  raw  reply  [nested|flat] 19+ messages in thread

* [PATCH v8 2/3] Remove specialized word-length popcount implementations.
@ 2026-01-23 23:31 Nathan Bossart <nathan@postgresql.org>
  0 siblings, 0 replies; 19+ messages in thread

From: Nathan Bossart @ 2026-01-23 23:31 UTC (permalink / raw)

The uses of these functions do not justify the level of
micro-optimization we've done and may even hurt performance in some
cases (e.g., due to using function pointers).  This commit removes
all architecture-specific implementations of pg_popcount{32,64}()
and converts the portable ones to inlined functions in
pg_bitutils.h.  These inlined versions should produce the same code
as before (but inlined), so in theory this is a net gain for many
machines.  As an exception, for x86-64/gcc without sse4.2/popcnt,
we use a plain C version to ensure inlining because
__builtin_popcount() and __builtin_popcountl() generate function
calls for that configuration.  Our tests indicate this is still a
net win.

Suggested-by: John Naylor <johncnaylorls@gmail.com>
Reviewed-by: John Naylor <johncnaylorls@gmail.com>
Reviewed-by: Greg Burd <greg@burd.me>
Discussion: https://postgr.es/m/CANWCAZY7R%2Biy%2Br9YM_sySNydHzNqUirx1xk0tB3ej5HO62GdgQ%40mail.gmail.com
---
 src/include/port/pg_bitutils.h | 83 ++++++++++++++++++++++++----------
 src/port/pg_bitutils.c         | 65 +-------------------------
 src/port/pg_popcount_aarch64.c | 25 ----------
 src/port/pg_popcount_x86.c     | 43 +-----------------
 4 files changed, 63 insertions(+), 153 deletions(-)

diff --git a/src/include/port/pg_bitutils.h b/src/include/port/pg_bitutils.h
index c3049d71894..08b9abf5fe7 100644
--- a/src/include/port/pg_bitutils.h
+++ b/src/include/port/pg_bitutils.h
@@ -276,46 +276,83 @@ pg_ceil_log2_64(uint64 num)
 		return pg_leftmost_one_pos64(num - 1) + 1;
 }
 
-extern int	pg_popcount32_portable(uint32 word);
-extern int	pg_popcount64_portable(uint64 word);
 extern uint64 pg_popcount_portable(const char *buf, int bytes);
 extern uint64 pg_popcount_masked_portable(const char *buf, int bytes, bits8 mask);
 
-#ifdef HAVE_X86_64_POPCNTQ
+#if defined(HAVE_X86_64_POPCNTQ) || defined(USE_SVE_POPCNT_WITH_RUNTIME_CHECK)
 /*
- * Attempt to use SSE4.2 or AVX-512 instructions, but perform a runtime check
+ * Attempt to use specialized CPU instructions, but perform a runtime check
  * first.
  */
-extern PGDLLIMPORT int (*pg_popcount32) (uint32 word);
-extern PGDLLIMPORT int (*pg_popcount64) (uint64 word);
 extern PGDLLIMPORT uint64 (*pg_popcount_optimized) (const char *buf, int bytes);
 extern PGDLLIMPORT uint64 (*pg_popcount_masked_optimized) (const char *buf, int bytes, bits8 mask);
 
-#elif defined(USE_NEON)
-/* Use the Neon version of pg_popcount{32,64} without function pointer. */
-extern int	pg_popcount32(uint32 word);
-extern int	pg_popcount64(uint64 word);
-
-/*
- * We can try to use an SVE-optimized pg_popcount() on some systems  For that,
- * we do use a function pointer.
- */
-#ifdef USE_SVE_POPCNT_WITH_RUNTIME_CHECK
-extern PGDLLIMPORT uint64 (*pg_popcount_optimized) (const char *buf, int bytes);
-extern PGDLLIMPORT uint64 (*pg_popcount_masked_optimized) (const char *buf, int bytes, bits8 mask);
 #else
+/* Use a portable implementation -- no need for a function pointer. */
 extern uint64 pg_popcount_optimized(const char *buf, int bytes);
 extern uint64 pg_popcount_masked_optimized(const char *buf, int bytes, bits8 mask);
+
 #endif
 
+/*
+ * pg_popcount32
+ *		Return the number of 1 bits set in word
+ *
+ * Plain C version adapted from
+ * https://graphics.stanford.edu/~seander/bithacks.html#CountBitsSetParallel.
+ */
+static inline int
+pg_popcount32(uint32 word)
+{
+	/*
+	 * On x86, gcc generates a function call for this built-in unless the
+	 * popcnt instruction is available, so we use the plain C version in that
+	 * case to ensure inlining.
+	 */
+#if defined(HAVE__BUILTIN_POPCOUNT) && (defined(__POPCNT__) || !defined(__x86_64__))
+	return __builtin_popcount(word);
+#elif defined(_MSC_VER)
+	return __popcnt(word);
 #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_optimized(const char *buf, int bytes);
-extern uint64 pg_popcount_masked_optimized(const char *buf, int bytes, bits8 mask);
+	word -= (word >> 1) & 0x55555555;
+	word = (word & 0x33333333) + ((word >> 2) & 0x33333333);
+	return ((word + (word >> 4) & 0xf0f0f0f) * 0x1010101) >> 24;
+#endif
+}
 
+/*
+ * pg_popcount64
+ *		Return the number of 1 bits set in word
+ *
+ * Plain C version adapted from
+ * https://graphics.stanford.edu/~seander/bithacks.html#CountBitsSetParallel.
+ */
+static inline int
+pg_popcount64(uint64 word)
+{
+	/*
+	 * On x86, gcc generates a function call for this built-in unless the
+	 * popcnt instruction is available, so we use the plain C version in that
+	 * case to ensure inlining.
+	 */
+#if defined(HAVE__BUILTIN_POPCOUNT) && (defined(__POPCNT__) || !defined(__x86_64__))
+#if SIZEOF_LONG == 8
+	return __builtin_popcountl(word);
+#elif SIZEOF_LONG_LONG == 8
+	return __builtin_popcountll(word);
+#else
+#error "cannot find integer of the same size as uint64_t"
 #endif
+#elif defined(_MSC_VER)
+	return __popcnt64(word);
+#else
+	word -= (word >> 1) & UINT64CONST(0x5555555555555555);
+	word = (word & UINT64CONST(0x3333333333333333)) +
+		((word >> 2) & UINT64CONST(0x3333333333333333));
+	word = (word + (word >> 4)) & UINT64CONST(0xf0f0f0f0f0f0f0f);
+	return (word * UINT64CONST(0x101010101010101)) >> 56;
+#endif
+}
 
 /*
  * Returns the number of 1-bits in buf.
diff --git a/src/port/pg_bitutils.c b/src/port/pg_bitutils.c
index bec06c06fc3..49b130f1306 100644
--- a/src/port/pg_bitutils.c
+++ b/src/port/pg_bitutils.c
@@ -96,56 +96,6 @@ const uint8 pg_number_of_ones[256] = {
 	4, 5, 5, 6, 5, 6, 6, 7, 5, 6, 6, 7, 6, 7, 7, 8
 };
 
-/*
- * pg_popcount32_portable
- *		Return the number of 1 bits set in word
- */
-int
-pg_popcount32_portable(uint32 word)
-{
-#ifdef HAVE__BUILTIN_POPCOUNT
-	return __builtin_popcount(word);
-#else							/* !HAVE__BUILTIN_POPCOUNT */
-	int			result = 0;
-
-	while (word != 0)
-	{
-		result += pg_number_of_ones[word & 255];
-		word >>= 8;
-	}
-
-	return result;
-#endif							/* HAVE__BUILTIN_POPCOUNT */
-}
-
-/*
- * pg_popcount64_portable
- *		Return the number of 1 bits set in word
- */
-int
-pg_popcount64_portable(uint64 word)
-{
-#ifdef HAVE__BUILTIN_POPCOUNT
-#if SIZEOF_LONG == 8
-	return __builtin_popcountl(word);
-#elif SIZEOF_LONG_LONG == 8
-	return __builtin_popcountll(word);
-#else
-#error "cannot find integer of the same size as uint64_t"
-#endif
-#else							/* !HAVE__BUILTIN_POPCOUNT */
-	int			result = 0;
-
-	while (word != 0)
-	{
-		result += pg_number_of_ones[word & 255];
-		word >>= 8;
-	}
-
-	return result;
-#endif							/* HAVE__BUILTIN_POPCOUNT */
-}
-
 /*
  * pg_popcount_portable
  *		Returns the number of 1-bits in buf
@@ -163,7 +113,7 @@ pg_popcount_portable(const char *buf, int bytes)
 
 		while (bytes >= 8)
 		{
-			popcnt += pg_popcount64_portable(*words++);
+			popcnt += pg_popcount64(*words++);
 			bytes -= 8;
 		}
 
@@ -197,7 +147,7 @@ pg_popcount_masked_portable(const char *buf, int bytes, bits8 mask)
 
 		while (bytes >= 8)
 		{
-			popcnt += pg_popcount64_portable(*words++ & maskv);
+			popcnt += pg_popcount64(*words++ & maskv);
 			bytes -= 8;
 		}
 
@@ -220,17 +170,6 @@ pg_popcount_masked_portable(const char *buf, int bytes, bits8 mask)
  * actual external functions.  The compiler should be able to inline the
  * portable versions here.
  */
-int
-pg_popcount32(uint32 word)
-{
-	return pg_popcount32_portable(word);
-}
-
-int
-pg_popcount64(uint64 word)
-{
-	return pg_popcount64_portable(word);
-}
 
 /*
  * pg_popcount_optimized
diff --git a/src/port/pg_popcount_aarch64.c b/src/port/pg_popcount_aarch64.c
index ba57f2cd4bd..74f71593721 100644
--- a/src/port/pg_popcount_aarch64.c
+++ b/src/port/pg_popcount_aarch64.c
@@ -291,31 +291,6 @@ pg_popcount_masked_optimized(const char *buf, int bytes, bits8 mask)
 
 #endif							/* ! USE_SVE_POPCNT_WITH_RUNTIME_CHECK */
 
-/*
- * pg_popcount32
- *		Return number of 1 bits in word
- */
-int
-pg_popcount32(uint32 word)
-{
-	return pg_popcount64((uint64) word);
-}
-
-/*
- * pg_popcount64
- *		Return number of 1 bits in word
- */
-int
-pg_popcount64(uint64 word)
-{
-	/*
-	 * For some compilers, __builtin_popcountl() already emits Neon
-	 * instructions.  The line below should compile to the same code on those
-	 * systems.
-	 */
-	return vaddv_u8(vcnt_u8(vld1_u8((const uint8 *) &word)));
-}
-
 /*
  * pg_popcount_neon
  *		Returns number of 1 bits in buf
diff --git a/src/port/pg_popcount_x86.c b/src/port/pg_popcount_x86.c
index 7aebf69898b..6bce089432f 100644
--- a/src/port/pg_popcount_x86.c
+++ b/src/port/pg_popcount_x86.c
@@ -36,8 +36,6 @@
  * operation, but in practice this is close enough, and "sse42" seems easier to
  * follow than "popcnt" for these names.
  */
-static inline int pg_popcount32_sse42(uint32 word);
-static inline int pg_popcount64_sse42(uint64 word);
 static uint64 pg_popcount_sse42(const char *buf, int bytes);
 static uint64 pg_popcount_masked_sse42(const char *buf, int bytes, bits8 mask);
 
@@ -55,12 +53,8 @@ static uint64 pg_popcount_masked_avx512(const char *buf, int bytes, bits8 mask);
  * what the current CPU supports) and then will call the pointer to fulfill the
  * caller's request.
  */
-static int	pg_popcount32_choose(uint32 word);
-static int	pg_popcount64_choose(uint64 word);
 static uint64 pg_popcount_choose(const char *buf, int bytes);
 static uint64 pg_popcount_masked_choose(const char *buf, int bytes, bits8 mask);
-int			(*pg_popcount32) (uint32 word) = pg_popcount32_choose;
-int			(*pg_popcount64) (uint64 word) = pg_popcount64_choose;
 uint64		(*pg_popcount_optimized) (const char *buf, int bytes) = pg_popcount_choose;
 uint64		(*pg_popcount_masked_optimized) (const char *buf, int bytes, bits8 mask) = pg_popcount_masked_choose;
 
@@ -157,7 +151,7 @@ pg_popcount_avx512_available(void)
 #endif							/* USE_AVX512_POPCNT_WITH_RUNTIME_CHECK */
 
 /*
- * These functions get called on the first call to pg_popcount32 etc.
+ * These functions get called on the first call to pg_popcount(), etc.
  * They detect whether we can use the asm implementations, and replace
  * the function pointers so that subsequent calls are routed directly to
  * the chosen implementation.
@@ -167,15 +161,11 @@ choose_popcount_functions(void)
 {
 	if (pg_popcount_sse42_available())
 	{
-		pg_popcount32 = pg_popcount32_sse42;
-		pg_popcount64 = pg_popcount64_sse42;
 		pg_popcount_optimized = pg_popcount_sse42;
 		pg_popcount_masked_optimized = pg_popcount_masked_sse42;
 	}
 	else
 	{
-		pg_popcount32 = pg_popcount32_portable;
-		pg_popcount64 = pg_popcount64_portable;
 		pg_popcount_optimized = pg_popcount_portable;
 		pg_popcount_masked_optimized = pg_popcount_masked_portable;
 	}
@@ -189,20 +179,6 @@ choose_popcount_functions(void)
 #endif
 }
 
-static int
-pg_popcount32_choose(uint32 word)
-{
-	choose_popcount_functions();
-	return pg_popcount32(word);
-}
-
-static int
-pg_popcount64_choose(uint64 word)
-{
-	choose_popcount_functions();
-	return pg_popcount64(word);
-}
-
 static uint64
 pg_popcount_choose(const char *buf, int bytes)
 {
@@ -338,23 +314,6 @@ pg_popcount_masked_avx512(const char *buf, int bytes, bits8 mask)
 
 #endif							/* USE_AVX512_POPCNT_WITH_RUNTIME_CHECK */
 
-/*
- * pg_popcount32_sse42
- *		Return the number of 1 bits set in word
- */
-static inline int
-pg_popcount32_sse42(uint32 word)
-{
-#ifdef _MSC_VER
-	return __popcnt(word);
-#else
-	uint32		res;
-
-__asm__ __volatile__(" popcntl %1,%0\n":"=q"(res):"rm"(word):"cc");
-	return (int) res;
-#endif
-}
-
 /*
  * pg_popcount64_sse42
  *		Return the number of 1 bits set in word
-- 
2.50.1 (Apple Git-155)


--dIDo5IfS/hVe25hD
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
	filename=v8-0003-Make-use-of-pg_popcount-in-more-places.patch



^ permalink  raw  reply  [nested|flat] 19+ messages in thread

* [PATCH v11 2/4] Remove specialized word-length popcount implementations.
@ 2026-01-23 23:31 Nathan Bossart <nathan@postgresql.org>
  0 siblings, 0 replies; 19+ messages in thread

From: Nathan Bossart @ 2026-01-23 23:31 UTC (permalink / raw)

The uses of these functions do not justify the level of
micro-optimization we've done and may even hurt performance in some
cases (e.g., due to using function pointers).  This commit removes
all architecture-specific implementations of pg_popcount{32,64}()
and converts the portable ones to inlined functions in
pg_bitutils.h.  These inlined versions should produce the same code
as before (but inlined), so in theory this is a net gain for many
machines.  As an exception, for x86-64/gcc without sse4.2/popcnt,
we use a plain C version to ensure inlining because
__builtin_popcount() and __builtin_popcountl() generate function
calls for that configuration.  Our tests indicate this is still a
net win.

Suggested-by: John Naylor <johncnaylorls@gmail.com>
Reviewed-by: John Naylor <johncnaylorls@gmail.com>
Reviewed-by: Greg Burd <greg@burd.me>
Discussion: https://postgr.es/m/CANWCAZY7R%2Biy%2Br9YM_sySNydHzNqUirx1xk0tB3ej5HO62GdgQ%40mail.gmail.com
---
 src/include/port/pg_bitutils.h | 72 ++++++++++++++++++++++------------
 src/port/pg_bitutils.c         | 65 +-----------------------------
 src/port/pg_popcount_aarch64.c | 20 +++-------
 src/port/pg_popcount_x86.c     | 43 +-------------------
 4 files changed, 56 insertions(+), 144 deletions(-)

diff --git a/src/include/port/pg_bitutils.h b/src/include/port/pg_bitutils.h
index 20c11b79c61..3c58f6c6864 100644
--- a/src/include/port/pg_bitutils.h
+++ b/src/include/port/pg_bitutils.h
@@ -276,46 +276,70 @@ pg_ceil_log2_64(uint64 num)
 		return pg_leftmost_one_pos64(num - 1) + 1;
 }
 
-extern int	pg_popcount32_portable(uint32 word);
-extern int	pg_popcount64_portable(uint64 word);
 extern uint64 pg_popcount_portable(const char *buf, int bytes);
 extern uint64 pg_popcount_masked_portable(const char *buf, int bytes, bits8 mask);
 
-#ifdef HAVE_X86_64_POPCNTQ
+#if defined(HAVE_X86_64_POPCNTQ) || defined(USE_SVE_POPCNT_WITH_RUNTIME_CHECK)
 /*
- * Attempt to use SSE4.2 or AVX-512 instructions, but perform a runtime check
+ * Attempt to use specialized CPU instructions, but perform a runtime check
  * first.
  */
-extern PGDLLIMPORT int (*pg_popcount32) (uint32 word);
-extern PGDLLIMPORT int (*pg_popcount64) (uint64 word);
 extern PGDLLIMPORT uint64 (*pg_popcount_optimized) (const char *buf, int bytes);
 extern PGDLLIMPORT uint64 (*pg_popcount_masked_optimized) (const char *buf, int bytes, bits8 mask);
 
-#elif defined(USE_NEON)
-/* Use the Neon version of pg_popcount{32,64} without function pointer. */
-extern int	pg_popcount32(uint32 word);
-extern int	pg_popcount64(uint64 word);
-
-/*
- * We can try to use an SVE-optimized pg_popcount() on some systems  For that,
- * we do use a function pointer.
- */
-#ifdef USE_SVE_POPCNT_WITH_RUNTIME_CHECK
-extern PGDLLIMPORT uint64 (*pg_popcount_optimized) (const char *buf, int bytes);
-extern PGDLLIMPORT uint64 (*pg_popcount_masked_optimized) (const char *buf, int bytes, bits8 mask);
 #else
+/* Use a portable implementation -- no need for a function pointer. */
 extern uint64 pg_popcount_optimized(const char *buf, int bytes);
 extern uint64 pg_popcount_masked_optimized(const char *buf, int bytes, bits8 mask);
+
 #endif
 
-#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_optimized(const char *buf, int bytes);
-extern uint64 pg_popcount_masked_optimized(const char *buf, int bytes, bits8 mask);
+/*
+ * pg_popcount32
+ *		Return the number of 1 bits set in word
+ *
+ * Adapted from
+ * https://graphics.stanford.edu/~seander/bithacks.html#CountBitsSetParallel.
+ */
+static inline int
+pg_popcount32(uint32 word)
+{
+	word -= (word >> 1) & 0x55555555;
+	word = (word & 0x33333333) + ((word >> 2) & 0x33333333);
+	return (((word + (word >> 4)) & 0xf0f0f0f) * 0x1010101) >> 24;
+}
 
+/*
+ * pg_popcount64
+ *		Return the number of 1 bits set in word
+ *
+ * Plain C version adapted from
+ * https://graphics.stanford.edu/~seander/bithacks.html#CountBitsSetParallel.
+ */
+static inline int
+pg_popcount64(uint64 word)
+{
+	/*
+	 * On x86, gcc generates a function call for this built-in unless the
+	 * popcnt instruction is available, so we use the plain C version in that
+	 * case to ensure inlining.
+	 */
+#if defined(HAVE__BUILTIN_POPCOUNT) && (defined(__POPCNT__) || !defined(__x86_64__))
+#if SIZEOF_LONG == 8
+	return __builtin_popcountl(word);
+#elif SIZEOF_LONG_LONG == 8
+	return __builtin_popcountll(word);
+#else
+#error "cannot find integer of the same size as uint64_t"
 #endif
+#else
+	word -= (word >> 1) & UINT64CONST(0x5555555555555555);
+	word = (word & UINT64CONST(0x3333333333333333)) +
+		((word >> 2) & UINT64CONST(0x3333333333333333));
+	word = (word + (word >> 4)) & UINT64CONST(0xf0f0f0f0f0f0f0f);
+	return (word * UINT64CONST(0x101010101010101)) >> 56;
+#endif
+}
 
 /*
  * Returns the number of 1-bits in buf.
diff --git a/src/port/pg_bitutils.c b/src/port/pg_bitutils.c
index bec06c06fc3..49b130f1306 100644
--- a/src/port/pg_bitutils.c
+++ b/src/port/pg_bitutils.c
@@ -96,56 +96,6 @@ const uint8 pg_number_of_ones[256] = {
 	4, 5, 5, 6, 5, 6, 6, 7, 5, 6, 6, 7, 6, 7, 7, 8
 };
 
-/*
- * pg_popcount32_portable
- *		Return the number of 1 bits set in word
- */
-int
-pg_popcount32_portable(uint32 word)
-{
-#ifdef HAVE__BUILTIN_POPCOUNT
-	return __builtin_popcount(word);
-#else							/* !HAVE__BUILTIN_POPCOUNT */
-	int			result = 0;
-
-	while (word != 0)
-	{
-		result += pg_number_of_ones[word & 255];
-		word >>= 8;
-	}
-
-	return result;
-#endif							/* HAVE__BUILTIN_POPCOUNT */
-}
-
-/*
- * pg_popcount64_portable
- *		Return the number of 1 bits set in word
- */
-int
-pg_popcount64_portable(uint64 word)
-{
-#ifdef HAVE__BUILTIN_POPCOUNT
-#if SIZEOF_LONG == 8
-	return __builtin_popcountl(word);
-#elif SIZEOF_LONG_LONG == 8
-	return __builtin_popcountll(word);
-#else
-#error "cannot find integer of the same size as uint64_t"
-#endif
-#else							/* !HAVE__BUILTIN_POPCOUNT */
-	int			result = 0;
-
-	while (word != 0)
-	{
-		result += pg_number_of_ones[word & 255];
-		word >>= 8;
-	}
-
-	return result;
-#endif							/* HAVE__BUILTIN_POPCOUNT */
-}
-
 /*
  * pg_popcount_portable
  *		Returns the number of 1-bits in buf
@@ -163,7 +113,7 @@ pg_popcount_portable(const char *buf, int bytes)
 
 		while (bytes >= 8)
 		{
-			popcnt += pg_popcount64_portable(*words++);
+			popcnt += pg_popcount64(*words++);
 			bytes -= 8;
 		}
 
@@ -197,7 +147,7 @@ pg_popcount_masked_portable(const char *buf, int bytes, bits8 mask)
 
 		while (bytes >= 8)
 		{
-			popcnt += pg_popcount64_portable(*words++ & maskv);
+			popcnt += pg_popcount64(*words++ & maskv);
 			bytes -= 8;
 		}
 
@@ -220,17 +170,6 @@ pg_popcount_masked_portable(const char *buf, int bytes, bits8 mask)
  * actual external functions.  The compiler should be able to inline the
  * portable versions here.
  */
-int
-pg_popcount32(uint32 word)
-{
-	return pg_popcount32_portable(word);
-}
-
-int
-pg_popcount64(uint64 word)
-{
-	return pg_popcount64_portable(word);
-}
 
 /*
  * pg_popcount_optimized
diff --git a/src/port/pg_popcount_aarch64.c b/src/port/pg_popcount_aarch64.c
index ba57f2cd4bd..f474ef45510 100644
--- a/src/port/pg_popcount_aarch64.c
+++ b/src/port/pg_popcount_aarch64.c
@@ -292,21 +292,11 @@ pg_popcount_masked_optimized(const char *buf, int bytes, bits8 mask)
 #endif							/* ! USE_SVE_POPCNT_WITH_RUNTIME_CHECK */
 
 /*
- * pg_popcount32
+ * pg_popcount64_neon
  *		Return number of 1 bits in word
  */
-int
-pg_popcount32(uint32 word)
-{
-	return pg_popcount64((uint64) word);
-}
-
-/*
- * pg_popcount64
- *		Return number of 1 bits in word
- */
-int
-pg_popcount64(uint64 word)
+static inline int
+pg_popcount64_neon(uint64 word)
 {
 	/*
 	 * For some compilers, __builtin_popcountl() already emits Neon
@@ -383,7 +373,7 @@ pg_popcount_neon(const char *buf, int bytes)
 	 */
 	for (; bytes >= sizeof(uint64); bytes -= sizeof(uint64))
 	{
-		popcnt += pg_popcount64(*((const uint64 *) buf));
+		popcnt += pg_popcount64_neon(*((const uint64 *) buf));
 		buf += sizeof(uint64);
 	}
 
@@ -465,7 +455,7 @@ pg_popcount_masked_neon(const char *buf, int bytes, bits8 mask)
 	 */
 	for (; bytes >= sizeof(uint64); bytes -= sizeof(uint64))
 	{
-		popcnt += pg_popcount64(*((const uint64 *) buf) & mask64);
+		popcnt += pg_popcount64_neon(*((const uint64 *) buf) & mask64);
 		buf += sizeof(uint64);
 	}
 
diff --git a/src/port/pg_popcount_x86.c b/src/port/pg_popcount_x86.c
index 7aebf69898b..6bce089432f 100644
--- a/src/port/pg_popcount_x86.c
+++ b/src/port/pg_popcount_x86.c
@@ -36,8 +36,6 @@
  * operation, but in practice this is close enough, and "sse42" seems easier to
  * follow than "popcnt" for these names.
  */
-static inline int pg_popcount32_sse42(uint32 word);
-static inline int pg_popcount64_sse42(uint64 word);
 static uint64 pg_popcount_sse42(const char *buf, int bytes);
 static uint64 pg_popcount_masked_sse42(const char *buf, int bytes, bits8 mask);
 
@@ -55,12 +53,8 @@ static uint64 pg_popcount_masked_avx512(const char *buf, int bytes, bits8 mask);
  * what the current CPU supports) and then will call the pointer to fulfill the
  * caller's request.
  */
-static int	pg_popcount32_choose(uint32 word);
-static int	pg_popcount64_choose(uint64 word);
 static uint64 pg_popcount_choose(const char *buf, int bytes);
 static uint64 pg_popcount_masked_choose(const char *buf, int bytes, bits8 mask);
-int			(*pg_popcount32) (uint32 word) = pg_popcount32_choose;
-int			(*pg_popcount64) (uint64 word) = pg_popcount64_choose;
 uint64		(*pg_popcount_optimized) (const char *buf, int bytes) = pg_popcount_choose;
 uint64		(*pg_popcount_masked_optimized) (const char *buf, int bytes, bits8 mask) = pg_popcount_masked_choose;
 
@@ -157,7 +151,7 @@ pg_popcount_avx512_available(void)
 #endif							/* USE_AVX512_POPCNT_WITH_RUNTIME_CHECK */
 
 /*
- * These functions get called on the first call to pg_popcount32 etc.
+ * These functions get called on the first call to pg_popcount(), etc.
  * They detect whether we can use the asm implementations, and replace
  * the function pointers so that subsequent calls are routed directly to
  * the chosen implementation.
@@ -167,15 +161,11 @@ choose_popcount_functions(void)
 {
 	if (pg_popcount_sse42_available())
 	{
-		pg_popcount32 = pg_popcount32_sse42;
-		pg_popcount64 = pg_popcount64_sse42;
 		pg_popcount_optimized = pg_popcount_sse42;
 		pg_popcount_masked_optimized = pg_popcount_masked_sse42;
 	}
 	else
 	{
-		pg_popcount32 = pg_popcount32_portable;
-		pg_popcount64 = pg_popcount64_portable;
 		pg_popcount_optimized = pg_popcount_portable;
 		pg_popcount_masked_optimized = pg_popcount_masked_portable;
 	}
@@ -189,20 +179,6 @@ choose_popcount_functions(void)
 #endif
 }
 
-static int
-pg_popcount32_choose(uint32 word)
-{
-	choose_popcount_functions();
-	return pg_popcount32(word);
-}
-
-static int
-pg_popcount64_choose(uint64 word)
-{
-	choose_popcount_functions();
-	return pg_popcount64(word);
-}
-
 static uint64
 pg_popcount_choose(const char *buf, int bytes)
 {
@@ -338,23 +314,6 @@ pg_popcount_masked_avx512(const char *buf, int bytes, bits8 mask)
 
 #endif							/* USE_AVX512_POPCNT_WITH_RUNTIME_CHECK */
 
-/*
- * pg_popcount32_sse42
- *		Return the number of 1 bits set in word
- */
-static inline int
-pg_popcount32_sse42(uint32 word)
-{
-#ifdef _MSC_VER
-	return __popcnt(word);
-#else
-	uint32		res;
-
-__asm__ __volatile__(" popcntl %1,%0\n":"=q"(res):"rm"(word):"cc");
-	return (int) res;
-#endif
-}
-
 /*
  * pg_popcount64_sse42
  *		Return the number of 1 bits set in word
-- 
2.50.1 (Apple Git-155)


--mih0IfQp6StcW7xc
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
	filename=v11-0003-Make-use-of-pg_popcount-in-more-places.patch



^ permalink  raw  reply  [nested|flat] 19+ messages in thread

* Enforce INSERT RLS checks for FOR PORTION OF leftovers?
@ 2026-05-01 18:53 Ayush Tiwari <ayushtiwari.slg01@gmail.com>
  2026-05-04 11:29 ` Re: Enforce INSERT RLS checks for FOR PORTION OF leftovers? Ayush Tiwari <ayushtiwari.slg01@gmail.com>
  0 siblings, 1 reply; 19+ messages in thread

From: Ayush Tiwari @ 2026-05-01 18:53 UTC (permalink / raw)
  To: pgsql-hackers; Peter Eisentraut <peter@eisentraut.org>

Hi,

I found what looks like a  discrepancy where UPDATE/DELETE FOR
PORTION OF commands bypass INSERT RLS WITH CHECK
policies when inserting temporal leftover rows. Not sure if it's already
flagged (could not find it in DL).

While it is intentional that ExecForPortionOfLeftovers() skips INSERT ACL
permission checks, the leftover rows are newly inserted rows and should
still satisfy INSERT/ALL RLS policies unless I'm missing something.

Currently, the rewrite phase only
attaches UPDATE/DELETE RLS checks for the target relation, leaving
ExecInsert() without a WCO_RLS_INSERT_CHECK to enforce for the
leftovers.

Maybe we should address this in rowsecurity.c by fetching CMD_INSERT
policies and adding them as WCO_RLS_INSERT_CHECK entries for queries
with a FOR PORTION OF clause?

Something like this:

--- a/src/backend/rewrite/rowsecurity.c
+++ b/src/backend/rewrite/rowsecurity.c
@@ -393,6 +393,34 @@ get_row_security_policies(Query *root, RangeTblEntry
*rte, int rt_index,
  }
  }

+ /*
+ * UPDATE/DELETE FOR PORTION OF commands insert temporal leftovers via
+ * ExecInsert().  Those internal inserts intentionally skip INSERT ACL
+ * permission checks, but they still create new rows and must satisfy any
+ * INSERT/ALL RLS WITH CHECK policies.
+ */
+ if ((commandType == CMD_UPDATE || commandType == CMD_DELETE) &&
+ root->forPortionOf != NULL)
+ {
+ List   *insert_permissive_policies;
+ List   *insert_restrictive_policies;
+
+ /* This should be the target relation */
+ Assert(rt_index == root->resultRelation);
+
+ get_policies_for_relation(rel, CMD_INSERT, user_id,
+  &insert_permissive_policies,
+  &insert_restrictive_policies);
+
+ add_with_check_options(rel, rt_index,
+   WCO_RLS_INSERT_CHECK,
+   insert_permissive_policies,
+   insert_restrictive_policies,
+   withCheckOptions,
+   hasSubLinks,
+   false);
+ }
+

Thoughts?

Regards,
Ayush


^ permalink  raw  reply  [nested|flat] 19+ messages in thread

* Re: Enforce INSERT RLS checks for FOR PORTION OF leftovers?
  2026-05-01 18:53 Enforce INSERT RLS checks for FOR PORTION OF leftovers? Ayush Tiwari <ayushtiwari.slg01@gmail.com>
@ 2026-05-04 11:29 ` Ayush Tiwari <ayushtiwari.slg01@gmail.com>
  2026-05-16 16:50   ` Re: Enforce INSERT RLS checks for FOR PORTION OF leftovers? Paul A Jungwirth <pj@illuminatedcomputing.com>
  0 siblings, 1 reply; 19+ messages in thread

From: Ayush Tiwari @ 2026-05-04 11:29 UTC (permalink / raw)
  To: pgsql-hackers; Peter Eisentraut <peter@eisentraut.org>; Paul A Jungwirth <pj@illuminatedcomputing.com>

Hi,

On Sat, 2 May 2026 at 00:23, Ayush Tiwari <ayushtiwari.slg01@gmail.com>
wrote:

> Hi,
>
> I found what looks like a  discrepancy where UPDATE/DELETE FOR
> PORTION OF commands bypass INSERT RLS WITH CHECK
> policies when inserting temporal leftover rows. Not sure if it's already
> flagged (could not find it in DL).
>
> While it is intentional that ExecForPortionOfLeftovers() skips INSERT ACL
> permission checks, the leftover rows are newly inserted rows and should
> still satisfy INSERT/ALL RLS policies unless I'm missing something.
>
>
Sharing a SQL repro example:

CREATE ROLE u;
CREATE TABLE t (id int, valid_at daterange NOT NULL, name text);
ALTER TABLE t ENABLE ROW LEVEL SECURITY;
CREATE POLICY p_all ON t FOR ALL TO u USING (true) WITH CHECK (true);
CREATE POLICY p_ins ON t FOR INSERT TO u WITH CHECK (false);
GRANT SELECT, INSERT, UPDATE, DELETE ON t TO u;
INSERT INTO t VALUES (1, daterange('2018-01-01','2020-01-01'), 'ok');

SET ROLE u;

-- (A) Fails as expected: new row violates row-level security policy
INSERT INTO t VALUES (2, daterange('2018-01-01','2020-01-01'), 'ok');

-- (B) Should fail the same way (creates leftover rows), but silently
succeeds
UPDATE t FOR PORTION OF valid_at FROM '2019-01-01' TO '2019-06-01'
  SET name = 'ok' WHERE id = 1;

If this is expected we need to change the documentation of policy
and if it is not, should we go with something like I shared in
upthread, I can send a patch file if required.

Regards,
Ayush


^ permalink  raw  reply  [nested|flat] 19+ messages in thread

* Re: Enforce INSERT RLS checks for FOR PORTION OF leftovers?
  2026-05-01 18:53 Enforce INSERT RLS checks for FOR PORTION OF leftovers? Ayush Tiwari <ayushtiwari.slg01@gmail.com>
  2026-05-04 11:29 ` Re: Enforce INSERT RLS checks for FOR PORTION OF leftovers? Ayush Tiwari <ayushtiwari.slg01@gmail.com>
@ 2026-05-16 16:50   ` Paul A Jungwirth <pj@illuminatedcomputing.com>
  2026-06-05 16:46     ` Re: Enforce INSERT RLS checks for FOR PORTION OF leftovers? Ayush Tiwari <ayushtiwari.slg01@gmail.com>
  0 siblings, 1 reply; 19+ messages in thread

From: Paul A Jungwirth @ 2026-05-16 16:50 UTC (permalink / raw)
  To: Ayush Tiwari <ayushtiwari.slg01@gmail.com>; +Cc: pgsql-hackers; Peter Eisentraut <peter@eisentraut.org>

On Mon, May 4, 2026 at 4:29 AM Ayush Tiwari <ayushtiwari.slg01@gmail.com> wrote:
>
> On Sat, 2 May 2026 at 00:23, Ayush Tiwari <ayushtiwari.slg01@gmail.com> wrote:
>>
>> I found what looks like a  discrepancy where UPDATE/DELETE FOR
>> PORTION OF commands bypass INSERT RLS WITH CHECK
>> policies when inserting temporal leftover rows. Not sure if it's already
>> flagged (could not find it in DL).
>>
>> While it is intentional that ExecForPortionOfLeftovers() skips INSERT ACL
>> permission checks, the leftover rows are newly inserted rows and should
>> still satisfy INSERT/ALL RLS policies unless I'm missing something.
>
> Sharing a SQL repro example:
>
> CREATE ROLE u;
> CREATE TABLE t (id int, valid_at daterange NOT NULL, name text);
> ALTER TABLE t ENABLE ROW LEVEL SECURITY;
> CREATE POLICY p_all ON t FOR ALL TO u USING (true) WITH CHECK (true);
> CREATE POLICY p_ins ON t FOR INSERT TO u WITH CHECK (false);
> GRANT SELECT, INSERT, UPDATE, DELETE ON t TO u;
> INSERT INTO t VALUES (1, daterange('2018-01-01','2020-01-01'), 'ok');
>
> SET ROLE u;
>
> -- (A) Fails as expected: new row violates row-level security policy
> INSERT INTO t VALUES (2, daterange('2018-01-01','2020-01-01'), 'ok');
>
> -- (B) Should fail the same way (creates leftover rows), but silently succeeds
> UPDATE t FOR PORTION OF valid_at FROM '2019-01-01' TO '2019-06-01'
>   SET name = 'ok' WHERE id = 1;
>
> If this is expected we need to change the documentation of policy
> and if it is not, should we go with something like I shared in
> upthread, I can send a patch file if required.

Skipping the RLS checks to insert the leftovers seems like the correct
behavior to me, since we are skipping the ACL checks (per the
standard). Shouldn't it be consistent?

I think the reason we skip the checks is that semantically, the
leftovers aren't changing anything: they are preserving the history
that is already there.

I'm happy to write a doc patch to make this explicit. Does anyone
disagree about the correct behavior though?

Yours,

-- 
Paul              ~{:-)
pj@illuminatedcomputing.com





^ permalink  raw  reply  [nested|flat] 19+ messages in thread

* Re: Enforce INSERT RLS checks for FOR PORTION OF leftovers?
  2026-05-01 18:53 Enforce INSERT RLS checks for FOR PORTION OF leftovers? Ayush Tiwari <ayushtiwari.slg01@gmail.com>
  2026-05-04 11:29 ` Re: Enforce INSERT RLS checks for FOR PORTION OF leftovers? Ayush Tiwari <ayushtiwari.slg01@gmail.com>
  2026-05-16 16:50   ` Re: Enforce INSERT RLS checks for FOR PORTION OF leftovers? Paul A Jungwirth <pj@illuminatedcomputing.com>
@ 2026-06-05 16:46     ` Ayush Tiwari <ayushtiwari.slg01@gmail.com>
  2026-07-15 15:35       ` Re: Enforce INSERT RLS checks for FOR PORTION OF leftovers? Nathan Bossart <nathandbossart@gmail.com>
  0 siblings, 1 reply; 19+ messages in thread

From: Ayush Tiwari @ 2026-06-05 16:46 UTC (permalink / raw)
  To: Paul A Jungwirth <pj@illuminatedcomputing.com>; +Cc: pgsql-hackers; Peter Eisentraut <peter@eisentraut.org>

Hi,

On Sat, 16 May 2026 at 22:20, Paul A Jungwirth <pj@illuminatedcomputing.com>
wrote:

Skipping the RLS checks to insert the leftovers seems like the correct
> behavior to me, since we are skipping the ACL checks (per the
> standard). Shouldn't it be consistent?
>
> I think the reason we skip the checks is that semantically, the
> leftovers aren't changing anything: they are preserving the history
> that is already there.
>
> I'm happy to write a doc patch to make this explicit. Does anyone
> disagree about the correct behavior though?
>

I'd treat them separately.  Skipping INSERT ACL is a standard-driven
call about command privilege; RLS WITH CHECK is a per-row predicate
on the data, and the leftover really is a new heap tuple written by
the current role.

With `WITH CHECK (name <> 'denied')` and a pre-existing 'denied' row
(loaded by COPY or before the policy), UPDATE FOR PORTION OF that
touches only the range column writes leftovers the role could not
have INSERTed directly.  That's the part that bothers me, especially
since RLS tends to be deployed as a hard security boundary rather
than something users expect to opt out of per statement type.
Please correct me if I'm wrong here.

Regards,
Ayush


^ permalink  raw  reply  [nested|flat] 19+ messages in thread

* Re: Enforce INSERT RLS checks for FOR PORTION OF leftovers?
  2026-05-01 18:53 Enforce INSERT RLS checks for FOR PORTION OF leftovers? Ayush Tiwari <ayushtiwari.slg01@gmail.com>
  2026-05-04 11:29 ` Re: Enforce INSERT RLS checks for FOR PORTION OF leftovers? Ayush Tiwari <ayushtiwari.slg01@gmail.com>
  2026-05-16 16:50   ` Re: Enforce INSERT RLS checks for FOR PORTION OF leftovers? Paul A Jungwirth <pj@illuminatedcomputing.com>
  2026-06-05 16:46     ` Re: Enforce INSERT RLS checks for FOR PORTION OF leftovers? Ayush Tiwari <ayushtiwari.slg01@gmail.com>
@ 2026-07-15 15:35       ` Nathan Bossart <nathandbossart@gmail.com>
  2026-07-15 17:17         ` Re: Enforce INSERT RLS checks for FOR PORTION OF leftovers? Paul A Jungwirth <pj@illuminatedcomputing.com>
  0 siblings, 1 reply; 19+ messages in thread

From: Nathan Bossart @ 2026-07-15 15:35 UTC (permalink / raw)
  To: Ayush Tiwari <ayushtiwari.slg01@gmail.com>; +Cc: Paul A Jungwirth <pj@illuminatedcomputing.com>; pgsql-hackers; Peter Eisentraut <peter@eisentraut.org>

[RMT hat]

This is listed as an open item for v19 [0], but there's been no update to
the thread since June 5th.  Is it on track for resolution sometime soon?

[0] https://wiki.postgresql.org/wiki/PostgreSQL_19_Open_Items

-- 
nathan






^ permalink  raw  reply  [nested|flat] 19+ messages in thread

* Re: Enforce INSERT RLS checks for FOR PORTION OF leftovers?
  2026-05-01 18:53 Enforce INSERT RLS checks for FOR PORTION OF leftovers? Ayush Tiwari <ayushtiwari.slg01@gmail.com>
  2026-05-04 11:29 ` Re: Enforce INSERT RLS checks for FOR PORTION OF leftovers? Ayush Tiwari <ayushtiwari.slg01@gmail.com>
  2026-05-16 16:50   ` Re: Enforce INSERT RLS checks for FOR PORTION OF leftovers? Paul A Jungwirth <pj@illuminatedcomputing.com>
  2026-06-05 16:46     ` Re: Enforce INSERT RLS checks for FOR PORTION OF leftovers? Ayush Tiwari <ayushtiwari.slg01@gmail.com>
  2026-07-15 15:35       ` Re: Enforce INSERT RLS checks for FOR PORTION OF leftovers? Nathan Bossart <nathandbossart@gmail.com>
@ 2026-07-15 17:17         ` Paul A Jungwirth <pj@illuminatedcomputing.com>
  0 siblings, 0 replies; 19+ messages in thread

From: Paul A Jungwirth @ 2026-07-15 17:17 UTC (permalink / raw)
  To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: Ayush Tiwari <ayushtiwari.slg01@gmail.com>; pgsql-hackers; Peter Eisentraut <peter@eisentraut.org>

On Wed, Jul 15, 2026 at 8:35 AM Nathan Bossart <nathandbossart@gmail.com> wrote:
>
> [RMT hat]
>
> This is listed as an open item for v19 [0], but there's been no update to
> the thread since June 5th.  Is it on track for resolution sometime soon?

I think the patch at [1] is ready to go, but it's on a separate thread.

I updated the Open Items list in include both conversations.

[1] https://www.postgresql.org/message-id/20DF964A-C291-491A-9126-62753DCBD7CE%40gmail.com

Yours,

-- 
Paul              ~{:-)
pj@illuminatedcomputing.com






^ permalink  raw  reply  [nested|flat] 19+ messages in thread

* [PATCH v2] Allow pg_read_all_stats to see database size in \l+
@ 2026-07-22 11:16 Christoph Berg <myon@debian.org>
  0 siblings, 0 replies; 19+ messages in thread

From: Christoph Berg @ 2026-07-22 11:16 UTC (permalink / raw)

The server already allows members of pg_read_all_stats to see the size
of all databases, but psql's \l+ was too restrictive.
---
 doc/src/sgml/ref/psql-ref.sgml | 5 +++--
 src/bin/psql/describe.c        | 3 ++-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 56c2692e618..4d5ee11ddd9 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -2817,8 +2817,9 @@ SELECT
         are displayed in expanded mode.
         If <literal>+</literal> is appended to the command name, database
         sizes, default tablespaces, and descriptions are also displayed.
-        (Size information is only available for databases that the current
-        user can connect to.)
+        Except for superusers or roles with privileges of the
+        <literal>pg_read_all_stats</literal> role, size information is only
+        available for databases that the current user can connect to.
         </para>
         </listitem>
       </varlistentry>
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index a2f09c26369..ad9c8affb4f 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -986,7 +986,8 @@ listAllDbs(const char *pattern, bool verbose)
 	printACLColumn(&buf, "d.datacl");
 	if (verbose)
 		appendPQExpBuffer(&buf,
-						  ",\n  CASE WHEN pg_catalog.has_database_privilege(d.datname, 'CONNECT')\n"
+						  ",\n  CASE WHEN pg_catalog.has_database_privilege(d.datname, 'CONNECT') OR\n"
+						  "               pg_catalog.pg_has_role('pg_read_all_stats', 'USAGE')\n"
 						  "       THEN pg_catalog.pg_size_pretty(pg_catalog.pg_database_size(d.datname))\n"
 						  "       ELSE 'No Access'\n"
 						  "  END as \"%s\""
-- 
2.53.0


--GvhE+GC2PI361jLV--






^ permalink  raw  reply  [nested|flat] 19+ messages in thread


end of thread, other threads:[~2026-07-22 11:16 UTC | newest]

Thread overview: 19+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-07-09 09:54 [PATCH 1/5] Make XLogReaderInvalReadState static. Antonin Houska <ah@cybertec.at>
2023-08-07 18:37 [PATCH v3] psql: Add tab-complete for optional view parameters Christoph Heiss <christoph@c8h4.io>
2023-08-07 18:37 [PATCH v3] psql: Add tab-complete for optional view parameters Christoph Heiss <christoph@c8h4.io>
2023-08-07 18:37 [PATCH v3] psql: Add tab-complete for optional view parameters Christoph Heiss <christoph@c8h4.io>
2026-01-23 23:31 [PATCH v6 3/3] Remove specialized word-length popcount implementations. Nathan Bossart <nathan@postgresql.org>
2026-01-23 23:31 [PATCH v12 2/4] Remove specialized word-length popcount implementations. Nathan Bossart <nathan@postgresql.org>
2026-01-23 23:31 [PATCH v10 2/3] Remove specialized word-length popcount implementations. Nathan Bossart <nathan@postgresql.org>
2026-01-23 23:31 [PATCH v5 3/3] Remove specialized word-length popcount implementations. Nathan Bossart <nathan@postgresql.org>
2026-01-23 23:31 [PATCH v13 2/5] Remove specialized word-length popcount implementations. Nathan Bossart <nathan@postgresql.org>
2026-01-23 23:31 [PATCH v9 2/3] Remove specialized word-length popcount implementations. Nathan Bossart <nathan@postgresql.org>
2026-01-23 23:31 [PATCH v8 2/3] Remove specialized word-length popcount implementations. Nathan Bossart <nathan@postgresql.org>
2026-01-23 23:31 [PATCH v11 2/4] Remove specialized word-length popcount implementations. Nathan Bossart <nathan@postgresql.org>
2026-05-01 18:53 Enforce INSERT RLS checks for FOR PORTION OF leftovers? Ayush Tiwari <ayushtiwari.slg01@gmail.com>
2026-05-04 11:29 ` Re: Enforce INSERT RLS checks for FOR PORTION OF leftovers? Ayush Tiwari <ayushtiwari.slg01@gmail.com>
2026-05-16 16:50   ` Re: Enforce INSERT RLS checks for FOR PORTION OF leftovers? Paul A Jungwirth <pj@illuminatedcomputing.com>
2026-06-05 16:46     ` Re: Enforce INSERT RLS checks for FOR PORTION OF leftovers? Ayush Tiwari <ayushtiwari.slg01@gmail.com>
2026-07-15 15:35       ` Re: Enforce INSERT RLS checks for FOR PORTION OF leftovers? Nathan Bossart <nathandbossart@gmail.com>
2026-07-15 17:17         ` Re: Enforce INSERT RLS checks for FOR PORTION OF leftovers? Paul A Jungwirth <pj@illuminatedcomputing.com>
2026-07-22 11:16 [PATCH v2] Allow pg_read_all_stats to see database size in \l+ Christoph Berg <myon@debian.org>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox