Re: Optmize bitmapword macros calc (src/backend/nodes/bitmapset.c)

On Mon, Jan 29, 2024 at 04:43:32PM -0300, Ranier Vilela wrote:
> Em seg., 29 de jan. de 2024 às 16:32, Nathan Bossart <
> nathandbossart(at)gmail(dot)com> escreveu:
>> -#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.
>
> bitmapword It is the most correct and prudent option, if in the future,
> we decide to change the number of nwords to uint64.

If we change nwords to a uint64, I think there will be many other changes
required. Using uint32 probably trims the instructions further on machines
with 64-bit pointers, too (e.g., cdqe).

>> I also wonder if it's worth asserting that x is >= 0 before
>> casting here.
>>
> I don't think this would change anything.

Right, but it would offer another layer of protection against negative
integers in Bitmapsets.

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