Peter Eisentraut wrote: > + /* > + * ceil(VARBITLEN(ARG1)/BITS_PER_BYTE) > + * done to minimize branches and instructions. > + */ > > I don't know what that is supposed to mean or why that kind of tuning > would be necessary for a user-callable function.
Also, the formula just below looks wrong in the current patch (v3): + /* + * ceil(VARBITLEN(ARG1)/BITS_PER_BYTE) + * done to minimize branches and instructions. + */ + len = (VARBITLEN(arg1) + BITS_PER_BYTE + 1) / BITS_PER_BYTE; + p = VARBITS(arg1); + + popcount = pg_popcount((const char *)p, len); It should be len = (VARBITLEN(arg1) + BITS_PER_BYTE - 1) / BITS_PER_BYTE If we add 1 to BITS_PER_BYTE instead of subtracting 1, when VARBITLEN(arg1) is a multiple of BITS_PER_BYTE, "len" is one byte too large. The correct formula is already used in include/utils/varbit.h near the definition of VARBITLEN: #define VARBITTOTALLEN(BITLEN) (((BITLEN) + BITS_PER_BYTE-1)/BITS_PER_BYTE + \ VARHDRSZ + VARBITHDRSZ) Best regards, -- Daniel Vérité PostgreSQL-powered mailer: https://www.manitou-mail.org Twitter: @DanielVerite