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 to extend 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 also be 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 the pg_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 to provide feedback and review
your work, or can continue development work based on any feedback. Please let
me know which you'd prefer.
Raghuveer
