Date: Thu, 28 Mar 2024 16:51:36 -0500
From: Nathan Bossart <nathandbossart@gmail.com>
To: "Amonson, Paul D" <paul.d.amonson@intel.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, David Rowley <dgrowleyml@gmail.com>,
	Andres Freund <andres@anarazel.de>,
	Alvaro Herrera <alvherre@alvh.no-ip.org>,
	"Shankaran, Akash" <akash.shankaran@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: <20240328215136.GA918358@nathanxps13>
References: <CAApHDvrf-oKJ-mCrFZ_g-fZpRrCXJrKcZobwtUgzi0R5aXtVxQ@mail.gmail.com>
 <BL1PR11MB530478ACABE588A836338E74DC322@BL1PR11MB5304.namprd11.prod.outlook.com>
 <BL1PR11MB53040D8E18518B2BDA4BB83ADC362@BL1PR11MB5304.namprd11.prod.outlook.com>
 <2490633.1711379522@sss.pgh.pa.us>
 <BL1PR11MB53043014C10F2D30B4231F9ADC362@BL1PR11MB5304.namprd11.prod.outlook.com>
 <BL1PR11MB53049F1796E834DBE0E05864DC362@BL1PR11MB5304.namprd11.prod.outlook.com>
 <20240325200551.GB2924132@nathanxps13>
 <20240327220010.GA231058@nathanxps13>
 <BL1PR11MB53047289F146DB43C9B2FC71DC342@BL1PR11MB5304.namprd11.prod.outlook.com>
 <20240328213854.GA916732@nathanxps13>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240328213854.GA916732@nathanxps13>
Archived-At: <https://www.postgresql.org/message-id/20240328215136.GA918358%40nathanxps13>
Precedence: bulk

On Thu, Mar 28, 2024 at 04:38:54PM -0500, Nathan Bossart wrote:
> Here is a v14 of the patch that I think is beginning to approach something
> committable.  Besides general review and testing, there are two things that
> I'd like to bring up:
> 
> * The latest patch set from Paul Amonson appeared to support MSVC in the
>   meson build, but not the autoconf one.  I don't have much expertise here,
>   so the v14 patch doesn't have any autoconf/meson support for MSVC, which
>   I thought might be okay for now.  IIUC we assume that 64-bit/MSVC builds
>   can always compile the x86_64 popcount code, but I don't know whether
>   that's safe for AVX512.
> 
> * I think we need to verify there isn't a huge performance regression for
>   smaller arrays.  IIUC those will still require an AVX512 instruction or
>   two as well as a function call, which might add some noticeable overhead.

I forgot to mention that I also want to understand whether we can actually
assume availability of XGETBV when CPUID says we support AVX512:

> +		/*
> +		 * We also need to check that the OS has enabled support for the ZMM
> +		 * registers.
> +		 */
> +#ifdef _MSC_VER
> +		return (_xgetbv(0) & 0xe0) != 0;
> +#else
> +		uint64		xcr = 0;
> +		uint32		high;
> +		uint32		low;
> +
> +__asm__ __volatile__(" xgetbv\n":"=a"(low), "=d"(high):"c"(xcr));
> +		return (low & 0xe0) != 0;
> +#endif

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com