Date: Tue, 30 Jul 2024 20:20:34 -0500
From: Nathan Bossart <nathandbossart@gmail.com>
To: Andres Freund <andres@anarazel.de>
Cc: "Devulapalli, Raghuveer" <raghuveer.devulapalli@intel.com>,
	"Shankaran, Akash" <akash.shankaran@intel.com>,
	Tom Lane <tgl@sss.pgh.pa.us>, David Rowley <dgrowleyml@gmail.com>,
	Ants Aasma <ants.aasma@cybertec.at>,
	Alvaro Herrera <alvherre@alvh.no-ip.org>,
	"Amonson, Paul D" <paul.d.amonson@intel.com>,
	Noah Misch <noah@leadboat.com>,
	Matthias van de Meent <boekewurm+postgres@gmail.com>,
	"pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>
Subject: Re: Popcount optimization using AVX512
Message-ID: <ZqmRYh3iikm1Kh3D@nathan>
References: 
 <PH8PR11MB8286E1E9D1ADD14300A267CAFB0E2@PH8PR11MB8286.namprd11.prod.outlook.com>
 <20240418210158.GA3776258@nathanxps13>
 <PH8PR11MB8286DB91D214478FB16EE616FB0E2@PH8PR11MB8286.namprd11.prod.outlook.com>
 <20240418215902.GA3830246@nathanxps13>
 <PH8PR11MB82863AB8040AD090909DC18BFB0E2@PH8PR11MB8286.namprd11.prod.outlook.com>
 <20240418221358.GA3927188@nathanxps13>
 <20240423160207.GA841237@nathanxps13>
 <20240730210701.oyn2wr4zduu2efrg@awork3.anarazel.de>
 <Zqlb1_-BWlzVMbZv@nathan>
 <20240731004959.6ys24432n6xlgemk@awork3.anarazel.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240731004959.6ys24432n6xlgemk@awork3.anarazel.de>
Archived-At: <https://www.postgresql.org/message-id/ZqmRYh3iikm1Kh3D%40nathan>
Precedence: bulk

On Tue, Jul 30, 2024 at 05:49:59PM -0700, Andres Freund wrote:
> Ah, I somehow thought we'd avoid the runtime check in case we determine at
> compile time we don't need any extra flags to enable the AVX512 stuff (similar
> to how we deal with crc32). But it looks like that's not the case - which
> seems pretty odd to me:
> 
> This turns something that can be a single instruction into an indirect
> function call, even if we could know that it's guaranteed to be available for
> the compilation target, due to -march=....
> 
> It's one thing for the avx512 path to have that overhead, but it's
> particularly absurd for pg_popcount32/pg_popcount64, where
> 
> a) The function call overhead is a larger proportion of the cost.
> b) the instruction is almost universally available, including in the
>    architecture baseline x86-64-v2, which several distros are using as the
>    x86-64 baseline.

Yeah, pg_popcount32/64 have been doing this since v12 (02a6a54).  Until v17
(cc4826d), pg_popcount() repeatedly calls these function pointers, too.  I
think it'd be awesome if we could start requiring some of these "almost
universally available" instructions, but AFAICT that brings its own
complexity [0].

> Why are we actually checking for xsave? We're not using xsave itself and I
> couldn't find a comment in 792752af4eb5 explaining what we're using it as a
> proxy for?  Is that just to know if _xgetbv() exists?  Is it actually possible
> that xsave isn't available when avx512 is?

Yes, it's to verify we have XGETBV, which IIUC requires support from both
the processor and the OS (see 598e011 and upthread discussion).  AFAIK the
way we are detecting AVX-512 support is quite literally by-the-book unless
I've gotten something wrong.

[0] https://postgr.es/m/ZmpG2ZzT30Q75BZO%40nathan

-- 
nathan