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@gmail.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