Hi John,
Just a few comments on v7:
> pg_cpucap_crc32c
I like the idea of moving all CPU runtime detection to a single module outside of implementations. This makes it easy
toextend in the future and keeps the functionalities separate.
> - Rename the CRC choose*.c files to pg_cpucap_{x86,arm}.c and build them
> unconditionally for each platform
+1. Sounds perfect. We should also move the avx512 runtime detection of popcount here. The runtime detection code could
alsobe appended with function __attribute__((constructor)) so that it gets executed before main.
> I think the runtime dispatch is fairly simple for this case.
+ pg_comp_crc32c_sse42(pg_crc32c crc, const void *data, size_t len)
+ {
+ ....
+ #ifdef HAVE_PCLMUL_RUNTIME
+ if (len >= PCLMUL_THRESHOLD && (pg_cpucap & PGCPUCAP_CLMUL))
IMO, the pclmul and sse4.2 versions should be dispatched independently and the dispatch logic should be moved out of
thepg_crc32c.h to it own pg_crc32c_dispatch.c file.
Also, thank you for taking lead on developing this patch. Since the latest patch seems to be in a good shape, I'm happy
toprovide feedback and review your work, or can continue development work based on any feedback. Please let me know
whichyou'd prefer.
Raghuveer