From: | Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> |
---|---|
To: | David Rowley <david(dot)rowley(at)2ndquadrant(dot)com> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com> |
Subject: | Re: Using POPCNT and other advanced bit manipulation instructions |
Date: | 2019-01-31 22:45:02 |
Message-ID: | 201901312245.y4phq2m6f3gy@alvherre.pgsql |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
I only have cosmetic suggestions for this patch. For one thing, I think
the .c file should be in src/port and its header should be in
src/include/port/, right beside the likes of pg_bswap.h and pg_crc32c.h.
For another, I think the arrangement of all those "ifdef
HAVE_THIS_OR_THAT" in the bitutils.c file is a bit hard to read. I'd
lay them out like this:
#ifdef HAVE__BUILTIN_CTZ
int (*pg_rightmost_one32) (uint32 word) = pg_rightmost_one32_choose;
#else
int (*pg_rightmost_one32) (uint32 word) = pg_rightmost_one32_slow;
#endif
#ifdef HAVE__BUILTIN_CTZ
/*
* This gets called on the first call. It replaces the function pointer
* so that subsequent calls are routed directly to the chosen implementation.
*/
static int
pg_rightmost_one32_choose(uint32 word)
{
...
(You need declarations for the "choose" variants at the top of the file,
but that seems okay.)
Finally, the part in bitmapset.c is repetitive on the #ifdefs; I'd just
put at the top of the file something like
#if bms are 32 bits
#define pg_rightmost_one(x) pg_rightmost_one32(x)
#define pg_popcount(x) pg_popcount32(x)
#elif they are 64 bits
#define ...
#else
#error ...
#endif
This way, each place that uses the functions does not need the ifdefs.
Other than those minor changes, I think we should just get this pushed
and see what the buildfarm thinks. In the words of a famous PG hacker:
if a platform ain't in the buildfarm, we don't support it.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From | Date | Subject | |
---|---|---|---|
Next Message | Alvaro Herrera | 2019-01-31 23:00:56 | Re: ATTACH/DETACH PARTITION CONCURRENTLY |
Previous Message | David Rowley | 2019-01-31 21:47:42 | Re: Delay locking partitions during INSERT and UPDATE |