Date: Fri, 15 Feb 2019 11:28:42 -0300
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>
Cc: tgl@sss.pgh.pa.us, andres@anarazel.de, thomas.munro@enterprisedb.com,
	andrew@tao11.riddles.org.uk, david.rowley@2ndquadrant.com,
	pgsql-hackers@lists.postgresql.org
Subject: Re: Using POPCNT and other advanced bit manipulation instructions
Message-ID: <20190215142842.GA29812@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20190215.104601.261252390.horiguchi.kyotaro@lab.ntt.co.jp>
User-Agent: Mutt/1.9.4 (2018-02-28)
Precedence: bulk

On 2019-Feb-15, Kyotaro HORIGUCHI wrote:

> I understand that the behavior of __builtin_c[tl]z(0) is
> undefined from the reason, they convert to bs[rf]. So if we use
> these builtins, additional check is required.
> 
> https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html

Okay -- the functions check for a 0 argument:

+static inline int
+pg_rightmost_one32(uint32 word)
+{
+   int         result = 0;
+
+   Assert(word != 0);
+
+#ifdef HAVE__BUILTIN_CTZ
+   result = __builtin_ctz(word);
+#else
+   while ((word & 255) == 0)
+   {
+       word >>= 8;
+       result += 8;
+   }
+   result += rightmost_one_pos[word & 255];
+#endif                         /* HAVE__BUILTIN_CTZ */
+
+   return result;
+}

so we're fine.

> And even worse lzcntx is accidentially "fallback"s to bsrx on
> unsupported CPUs, which leads to bogus results.
> __builtin_clzll(8) = 3, which should be 60.

I'm not sure I understand this point.  Are you saying that if we use
-mlzcnt to compile, then the compiler builtin is broken in platforms
that don't support the lzcnt instruction?  That's horrible.  Let's stay
away from -mlzcnt then.

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