RE: Popcount optimization using AVX512

> -----Original Message-----
> From: Nathan Bossart <nathandbossart(at)gmail(dot)com>
> Sent: Wednesday, March 13, 2024 9:39 AM
> To: Amonson, Paul D <paul(dot)d(dot)amonson(at)intel(dot)com>

> +extern int pg_popcount32_slow(uint32 word); extern int
> +pg_popcount64_slow(uint64 word);
>
> +/* In pg_popcnt_*_accel source file. */ extern int
> +pg_popcount32_fast(uint32 word); extern int pg_popcount64_fast(uint64
> +word);
>
> Can these prototypes be moved to a header file (maybe pg_bitutils.h)? It
> looks like these are defined twice in the patch, and while I'm not positive that
> it's against project policy to declare extern function prototypes in .c files, it
> appears to be pretty rare.

Originally, I intentionally did not put these in the header file as I want them to be private, but they are not defined in this .c file hence extern. Now I realize the "extern" part is not needed to accomplish my goal. Will fix by removing the "extern" keyword.

> + 'pg_popcnt_choose.c',
> + 'pg_popcnt_x86_64_accel.c',
>
> I think we want these to be architecture-specific, i.e., only built for
> x86_64 if the compiler knows how to use the relevant instructions. There is a
> good chance that we'll want to add similar support for other systems.
> The CRC32C files are probably a good reference point for how to do this.

I will look at this for the 'pg_popcnt_x86_64_accel.c' file but the 'pg_popcnt_choose.c' file is intended to be for any platform that may need accelerators including a possible future ARM accelerator.

> +#ifdef TRY_POPCNT_FAST
>
> IIUC this macro can be set if either 1) the popcntq test in the autoconf/meson
> scripts passes or 2) we're building with MSVC on x86_64. I wonder if it would
> be better to move the MSVC/x86_64 check to the autoconf/meson scripts so
> that we could avoid surrounding large portions of the popcount code with this
> macro. This might even be a necessary step towards building these files in an
> architecture-specific fashion.

I see the point here; however, this will take some time to get right especially since I don't have a Windows box to do compiles on. Should I attempt to do this in this patch?

Thanks,
Paul