Em seg., 29 de jan. de 2024 às 16:32, Nathan Bossart < nathandboss...@gmail.com> escreveu:
> 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... > Bitmapset is extensively used. > -#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. > I also wonder if it's worth asserting that x is >= 0 before > casting here. > I don't think this would change anything. best regards, Ranier Vilela
