public inbox for [email protected]
help / color / mirror / Atom feedFrom: Alvaro Herrera <[email protected]>
To: Shankaran, Akash <[email protected]>
Cc: Nathan Bossart <[email protected]>
Cc: Noah Misch <[email protected]>
Cc: Amonson, Paul D <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: Matthias van de Meent <[email protected]>
Cc: [email protected] <[email protected]>
Subject: Re: Popcount optimization using AVX512
Date: Thu, 25 Jan 2024 10:49:09 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <PH0PR11MB5000C2258BF2804AAF7AAE27F27A2@PH0PR11MB5000.namprd11.prod.outlook.com>
On 2024-Jan-25, Shankaran, Akash wrote:
> With the updated patch, we observed significant improvements and
> handily beat the previous popcount algorithm performance. No
> regressions in any scenario are observed:
> Platform: Intel Xeon Platinum 8360Y (Icelake) for data sizes 1kb - 64kb.
> Microbenchmark: 2x - 3x gains presently vs 19% previously, on the same
> microbenchmark described initially in this thread.
These are great results.
However, it would be much better if the improved code were available for
all relevant builds and activated if a CPUID test determines that the
relevant instructions are available, instead of requiring a compile-time
flag -- which most builds are not going to use, thus wasting the
opportunity for running the optimized code.
I suppose this would require patching pg_popcount64_choose() to be more
specific. Looking at the existing code, I would also consider renaming
the "_fast" variants to something like pg_popcount32_asml/
pg_popcount64_asmq so that you can name the new one pg_popcount64_asmdq
or such. (Or maybe leave the 32-bit version alone as "fast/slow", since
there's no third option for that one -- or do I misread?)
I also think this needs to move the CFLAGS-decision-making elsewhere;
asking the user to get it right is too much of a burden. Is it workable
to simply verify compiler support for the additional flags needed, and
if so add them to a new CFLAGS_BITUTILS variable or such? We already
have the CFLAGS_CRC model that should be easy to follow. Should be easy
enough to mostly copy what's in configure.ac and meson.build, right?
Finally, the matter of using ifunc as proposed by Noah seems to be still
in the air, with no patches offered for the popcount family. Given that
Nathan reports [1] a performance decrease, maybe we should set that
thought aside for now and continue to use function pointers. It's worth
keeping in mind that popcount is already using function pointers (at
least in the case where we try to use POPCNT directly), so patching to
select between three options instead of between two wouldn't be a
regression.
[1] https://postgr.es/m/20231107201441.GA898662@nathanxps13
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"Nunca se desea ardientemente lo que solo se desea por razón" (F. Alexandre)
view thread (37+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Popcount optimization using AVX512
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox