Hi,

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 did a demonstration in compiler explorer.
https://godbolt.org/z/c68o43Eq1

It is demonstrated here as well.

Generated instructions: GCC 13.2

#define WORDNUM_f1(x) ((x) / BITS_PER_BITMAPWORD)
         mov eax, DWORD PTR [rbp-20]
         lea edx, [rax+63]
         test eax, eax
         cmovs eax, edx
         sar eax, 6
         mov DWORD PTR [rbp-4], eax

#define WORDNUM_f2(x) ((bitmapword) (x) / BITS_PER_BITMAPWORD)
         mov eax, DWORD PTR [rbp-20]
         cdqe
         shr rax, 6
         mov DWORD PTR [rbp-4], eax


#define BITNUM_f1(x) ((x) % BITS_PER_BITMAPWORD)
         mov edx, DWORD PTR [rbp-20]
         mov eax, edx
         sar eax, 31
         shr eax, 26
         add edx, eax
         and edx, 63
         sub edx, eax
         mov DWORD PTR [rbp-4], edx

#define BITNUM_f2(x) ((bitmapword) (x) % BITS_PER_BITMAPWORD)
        mov eax, DWORD PTR [rbp-20]
         and eax, 63
         mov DWORD PTR [rbp-4], eax

As you can see, when calculations are done in unsigned,
the generated instructions are optimized.

Patch attached.

Best regards,
Ranier Vilela
diff --git a/src/backend/nodes/bitmapset.c b/src/backend/nodes/bitmapset.c
index 65805d4527..0765dd42c2 100644
--- a/src/backend/nodes/bitmapset.c
+++ b/src/backend/nodes/bitmapset.c
@@ -44,8 +44,8 @@
 #include "port/pg_bitutils.h"
 
 
-#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)
 
 #define BITMAPSET_SIZE(nwords)	\
 	(offsetof(Bitmapset, words) + (nwords) * sizeof(bitmapword))

Reply via email to