Date: Wed, 13 Feb 2019 19:17:19 -0300
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Andrew Gierth <andrew@tao11.riddles.org.uk>
Cc: David Rowley <david.rowley@2ndquadrant.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
	Thomas Munro <thomas.munro@enterprisedb.com>
Subject: Re: Using POPCNT and other advanced bit manipulation instructions
Message-ID: <20190213221719.GA15976@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="C7zPtVaVf+AK4Oqc"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20190213205904.GA13925@alvherre.pgsql>
User-Agent: Mutt/1.9.4 (2018-02-28)
Precedence: bulk


--C7zPtVaVf+AK4Oqc
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

On 2019-Feb-13, Alvaro Herrera wrote:

> It definitely is ... plans have changed from using IndexOnly scans to
> Seqscans, which is likely fallout from the visibilitymap_count() change.

I think the problem here is that "unsigned long" is 32 bits in this
machine:
  checking whether long int is 64 bits... no

and we have defined pg_popcount64() like this:

static int
pg_popcount64_sse42(uint64 word)
{
	return __builtin_popcountl(word);
}

so it's counting bits in the lower half of the uint64.

If that's correct, then I think we need something like this patch.  But
it makes me wonder whether we need a configure test for
__builtin_popcountll() and friends.  I wonder if there's any compiler
that implements __builtin_popcountl() but not __builtin_popcountll() ...
and if not, then the test for __builtin_popcountl() should be removed,
and have everything rely on the one for __builtin_popcount().

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--C7zPtVaVf+AK4Oqc
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="uint64-is-longlong.patch"

diff --git a/src/port/pg_bitutils.c b/src/port/pg_bitutils.c
index 97422e05040..fe6afb9fba5 100644
--- a/src/port/pg_bitutils.c
+++ b/src/port/pg_bitutils.c
@@ -299,7 +299,14 @@ pg_popcount64_choose(uint64 word)
 static int
 pg_popcount64_sse42(uint64 word)
 {
+#if defined(HAVE_LONG_INT_64)
 	return __builtin_popcountl(word);
+#elif defined(HAVE_LONG_LONG_INT_64)
+	return __builtin_popcountll(word);
+#else
+	/* shouldn't happen */
+#error must have a working 64-bit integer datatype
+#endif
 }
 
 #endif
@@ -407,7 +414,14 @@ pg_rightmost_one64_choose(uint64 word)
 static int
 pg_rightmost_one64_abm(uint64 word)
 {
+#if defined(HAVE_LONG_INT_64)
 	return __builtin_ctzl(word);
+#elif defined(HAVE_LONG_LONG_INT_64)
+	return __builtin_ctzll(word);
+#else
+	/* shouldn't happen */
+#error must have a working 64-bit integer datatype
+#endif
 }
 #endif
 
@@ -493,7 +507,15 @@ pg_leftmost_one64_choose(uint64 word)
 static int
 pg_leftmost_one64_abm(uint64 word)
 {
+#if defined(HAVE_LONG_INT_64)
 	return 63 - __builtin_clzl(word);
+#elif defined(HAVE_LONG_LONG_INT_64)
+	return 63 - __builtin_clzll(word);
+#else
+	/* shouldn't happen */
+#error must have a working 64-bit integer datatype
+#endif
+
 }
 #endif
 

--C7zPtVaVf+AK4Oqc--