Hi Fabian-san,

I reviewed 'pgbench-prp-func/pgbench-prp-func-4.patch'.

I could apply it and did the TAP test successfully. I could not find typo in the documentations and comments.

To make sure, I checked the new routine which contains the __builtin_popcountll() function on Linux + gcc 7.3.1 and I confirmed that it works well.

I thinks this patch is fine.


Best regards,


On 2018/09/16 21:05, Fabien COELHO wrote:

Hello Hironobu-san,

Here is a v4, based on our out-of-list discussion:
  - the mask function is factored out
  - the popcount builtin is used if available

Attached a v3, based on your fix, plus some additional changes:
- explicitly declare unsigned variables where appropriate, to avoid casts
- use smaller 24 bits primes instead of 27-29 bits
- add a shortcut for multiplier below 24 bits and y value below 40 bits,
  which should avoid the manually implemented multiplication in most
  practical cases (tables with over 2^40 rows are pretty rare...).
- change the existing shortcut to look a the number of bits instead of
  using 32 limits.
- add a test for minimal code coverage with over 40 bits sizes
- attempt to improve the documentation
- some comments were updates, hopefully for the better



Reply via email to