Date: Fri, 16 Aug 2024 13:56:05 -0500
From: Nathan Bossart <nathandbossart@gmail.com>
To: Alexander Lakhin <exclusion@gmail.com>
Cc: Joseph Koshakow <koshy44@gmail.com>,
	jian he <jian.universality@gmail.com>,
	Heikki Linnakangas <hlinnaka@iki.fi>,
	Matthew Kim <matthewkmkim@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
	Andres Freund <andres@anarazel.de>
Subject: Re: Remove dependence on integer wrapping
Message-ID: <Zr-gxcOehrmppDGk@nathan>
References: <ZrznaJTIOPgSjEJ7@nathan>
 <CAAvxfHc2Smk92cJX4aUju1AdW_RpNN8csEj5rvf29b-YFHRmfQ@mail.gmail.com>
 <Zrz0Zv4PVFxieEii@nathan>
 <CACJufxH-uLR5SnnmVbGYj7Oy90zF_pYV3G_HxfbJ6Lu3XKGXxQ@mail.gmail.com>
 <Zr5M3KxOZlEZRZIN@nathan>
 <Zr50ZpSyrBx0EHfR@nathan>
 <CAAvxfHfkHS1n0+wMbtKwj6_GE6+aSJ9JjnsCOOdZPhTKRs-Yhw@mail.gmail.com>
 <Zr-DwBxep-IaayQF@nathan>
 <bfff906f-300d-81ea-83b7-f2c93845e7f2@gmail.com>
 <Zr-b1c6x-Yf31TCZ@nathan>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <Zr-b1c6x-Yf31TCZ@nathan>
Archived-At: <https://www.postgresql.org/message-id/Zr-gxcOehrmppDGk%40nathan>
Precedence: bulk

On Fri, Aug 16, 2024 at 01:35:01PM -0500, Nathan Bossart wrote:
> On Fri, Aug 16, 2024 at 09:00:00PM +0300, Alexander Lakhin wrote:
>> #6  0x00005576cf627c68 in bms_singleton_member (a=0x5576d09f7fb0) at bitmapset.c:691
>> 691                             if (result >= 0 || HAS_MULTIPLE_ONES(w))
> 
> At a glance, this appears to be caused by the RIGHTMOST_ONE macro:
> 
> 	#define RIGHTMOST_ONE(x) ((signedbitmapword) (x) & -((signedbitmapword) (x)))

I believe hand-rolling the two's complement calculation should be
sufficient to avoid depending on -fwrapv here.  godbolt.org indicates that
it produces roughly the same code, too.

diff --git a/src/backend/nodes/bitmapset.c b/src/backend/nodes/bitmapset.c
index cd05c642b0..d37a997c0e 100644
--- a/src/backend/nodes/bitmapset.c
+++ b/src/backend/nodes/bitmapset.c
@@ -67,7 +67,7 @@
  * we get zero.
  *----------
  */
-#define RIGHTMOST_ONE(x) ((signedbitmapword) (x) & -((signedbitmapword) (x)))
+#define RIGHTMOST_ONE(x) ((bitmapword) (x) & (~((bitmapword) (x)) + 1))

 #define HAS_MULTIPLE_ONES(x)    ((bitmapword) RIGHTMOST_ONE(x) != (x))

-- 
nathan