Re: Optmize bitmapword macros calc (src/backend/nodes/bitmapset.c) - Mailing list pgsql-hackers

From Nathan Bossart
Subject Re: Optmize bitmapword macros calc (src/backend/nodes/bitmapset.c)
Date
Msg-id 20240129193203.GA3671663@nathanxps13
Whole thread Raw
In response to Optmize bitmapword macros calc (src/backend/nodes/bitmapset.c)  (Ranier Vilela <ranier.vf@gmail.com>)
Responses Re: Optmize bitmapword macros calc (src/backend/nodes/bitmapset.c)
Re: Optmize bitmapword macros calc (src/backend/nodes/bitmapset.c)
List pgsql-hackers
On Mon, Jan 29, 2024 at 01:30:47PM -0300, Ranier Vilela wrote:
> IMO I believe that bitmapset can obtain an optimization in the calculation
> of the WORDNUM and BITNUM macros.
> 
> As you know, in bitmapset, negative members are not allowed.
> 
> if (x < 0)
> elog(ERROR, "negative bitmapset member not allowed");
> 
> Then, allow the compiler to optimize and do the calculations in unsigned.

I'm currently +0.1 for this change.  I don't see any huge problem with
trimming a few instructions, but I'm dubious there's any measurable impact.
However, a cycle saved is a cycle earned...

-#define WORDNUM(x)    ((x) / BITS_PER_BITMAPWORD)
-#define BITNUM(x)    ((x) % BITS_PER_BITMAPWORD)
+#define WORDNUM(x)    ((bitmapword)(x) / BITS_PER_BITMAPWORD)
+#define BITNUM(x)    ((bitmapword)(x) % BITS_PER_BITMAPWORD)

I'm curious why we'd cast to bitmapword and not straight to uint32.  I
don't think the intent is that callers will provide a bitmapword to these
macros.  I also wonder if it's worth asserting that x is >= 0 before
casting here.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com



pgsql-hackers by date:

Previous
From: "Euler Taveira"
Date:
Subject: Re: Should we remove -Wdeclaration-after-statement?
Next
From: Ranier Vilela
Date:
Subject: Re: Optmize bitmapword macros calc (src/backend/nodes/bitmapset.c)