public inbox for [email protected]
help / color / mirror / Atom feedFrom: Nathan Bossart <[email protected]>
To: Amonson, Paul D <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Cc: Shankaran, Akash <[email protected]>
Cc: Noah Misch <[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: Fri, 1 Mar 2024 15:44:57 -0600
Message-ID: <20240301214457.GA3024365@nathanxps13> (raw)
In-Reply-To: <BL1PR11MB53046A243932039BD545EBF4DC592@BL1PR11MB5304.namprd11.prod.outlook.com>
References: <BL1PR11MB5304E4A5AA87D9F0126B4502DC462@BL1PR11MB5304.namprd11.prod.outlook.com>
<[email protected]>
<BL1PR11MB53042C623F17F17469B2C053DC4B2@BL1PR11MB5304.namprd11.prod.outlook.com>
<[email protected]>
<BL1PR11MB5304F65FD8D19B608F9E8624DC482@BL1PR11MB5304.namprd11.prod.outlook.com>
<[email protected]>
<BL1PR11MB5304F0889AB4EB200AF44565DC572@BL1PR11MB5304.namprd11.prod.outlook.com>
<BL1PR11MB5304C768A3002CD2C6EF672FDC5A2@BL1PR11MB5304.namprd11.prod.outlook.com>
<BL1PR11MB53046A243932039BD545EBF4DC592@BL1PR11MB5304.namprd11.prod.outlook.com>
Thanks for the new version of the patch. I didn't see a commitfest entry
for this one, and unfortunately I think it's too late to add it for the
March commitfest. I would encourage you to add it to July's commitfest [0]
so that we can get some routine cfbot coverage.
On Tue, Feb 27, 2024 at 08:46:06PM +0000, Amonson, Paul D wrote:
> After consulting some Intel internal experts on MSVC the linking issue as
> it stood was not resolved. Instead, I created a MSVC ONLY work-around.
> This adds one extra functional call on the Windows builds (The linker
> resolves a real function just fine but not a function pointer of the same
> name). This extra latency does not exist on any of the other platforms. I
> also believe I addressed all issues raised in the previous reviews. The
> new pg_popcnt_x86_64_accel.c file is now the ONLY file compiled with the
> AVX512 compiler flags. I added support for the MSVC compiler flag as
> well. Both meson and autoconf are updated with the new refactor.
>
> I am attaching the new patch.
I think this patch might be missing the new files.
-#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
+#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
IME this means that the autoconf you are using has been patched. A quick
search on the mailing lists seems to indicate that it might be specific to
Debian [1].
-static int pg_popcount32_slow(uint32 word);
-static int pg_popcount64_slow(uint64 word);
+int pg_popcount32_slow(uint32 word);
+int pg_popcount64_slow(uint64 word);
+uint64 pg_popcount_slow(const char *buf, int bytes);
This patch appears to do a lot of refactoring. Would it be possible to
break out the refactoring parts into a prerequisite patch that could be
reviewed and committed independently from the AVX512 stuff?
-#if SIZEOF_VOID_P >= 8
+#if SIZEOF_VOID_P == 8
/* Process in 64-bit chunks if the buffer is aligned. */
- if (buf == (const char *) TYPEALIGN(8, buf))
+ if (buf == (const char *)TYPEALIGN(8, buf))
{
- const uint64 *words = (const uint64 *) buf;
+ const uint64 *words = (const uint64 *)buf;
while (bytes >= 8)
{
@@ -309,9 +213,9 @@ pg_popcount(const char *buf, int bytes)
bytes -= 8;
}
- buf = (const char *) words;
+ buf = (const char *)words;
}
-#else
+#elif SIZEOF_VOID_P == 4
/* Process in 32-bit chunks if the buffer is aligned. */
if (buf == (const char *) TYPEALIGN(4, buf))
{
Most, if not all, of these changes seem extraneous. Do we actually need to
more strictly check SIZEOF_VOID_P?
[0] https://commitfest.postgresql.org/48/
[1] https://postgr.es/m/20230211020042.uthdgj72kp3xlqam%40awork3.anarazel.de
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
view thread (8+ messages)
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], [email protected]
Subject: Re: Popcount optimization using AVX512
In-Reply-To: <20240301214457.GA3024365@nathanxps13>
* 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