public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v12 1/5] Correctly update contfol file at the end of archive recovery 8+ messages / 4 participants [nested] [flat]
* [PATCH v12 1/5] Correctly update contfol file at the end of archive recovery @ 2022-03-04 04:18 Kyotaro Horiguchi <[email protected]> 0 siblings, 0 replies; 8+ messages in thread From: Kyotaro Horiguchi @ 2022-03-04 04:18 UTC (permalink / raw) CreateRestartPoint runs WAL file cleanup basing on the checkpoint just have finished in the function. If the database has exited DB_IN_ARCHIVE_RECOVERY state when the function is going to update control file, the function refrains from updating the file at all then proceeds to WAL cleanup having the latest REDO LSN, which is now inconsistent with the control file. As the result, the succeeding cleanup procedure overly removes WAL files against the control file and leaves unrecoverable database until the next checkpoint finishes. Along with that fix, we remove a dead code path for the case some other process ran a simultaneous checkpoint. It seems like just a preventive measure but it's no longer useful because we are sure that checkpoint is performed only by checkpointer except single process mode. --- src/backend/access/transam/xlog.c | 69 ++++++++++++++++++++----------- 1 file changed, 44 insertions(+), 25 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 0d2bd7a357..bd962763cc 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -6899,6 +6899,9 @@ CreateRestartPoint(int flags) XLogSegNo _logSegNo; TimestampTz xtime; + /* we don't assume concurrent checkpoint/restartpoint to run */ + Assert (!IsUnderPostmaster || MyBackendType == B_CHECKPOINTER); + /* Get a local copy of the last safe checkpoint record. */ SpinLockAcquire(&XLogCtl->info_lck); lastCheckPointRecPtr = XLogCtl->lastCheckPointRecPtr; @@ -6964,7 +6967,7 @@ CreateRestartPoint(int flags) /* Also update the info_lck-protected copy */ SpinLockAcquire(&XLogCtl->info_lck); - XLogCtl->RedoRecPtr = lastCheckPoint.redo; + XLogCtl->RedoRecPtr = RedoRecPtr; SpinLockRelease(&XLogCtl->info_lck); /* @@ -6983,7 +6986,10 @@ CreateRestartPoint(int flags) /* Update the process title */ update_checkpoint_display(flags, true, false); - CheckPointGuts(lastCheckPoint.redo, flags); + CheckPointGuts(RedoRecPtr, flags); + + /* Update pg_control */ + LWLockAcquire(ControlFileLock, LW_EXCLUSIVE); /* * Remember the prior checkpoint's redo ptr for @@ -6991,30 +6997,26 @@ CreateRestartPoint(int flags) */ PriorRedoPtr = ControlFile->checkPointCopy.redo; + Assert (PriorRedoPtr < RedoRecPtr); + + ControlFile->checkPoint = lastCheckPointRecPtr; + ControlFile->checkPointCopy = lastCheckPoint; + /* - * Update pg_control, using current time. Check that it still shows - * DB_IN_ARCHIVE_RECOVERY state and an older checkpoint, else do nothing; - * this is a quick hack to make sure nothing really bad happens if somehow - * we get here after the end-of-recovery checkpoint. + * Ensure minRecoveryPoint is past the checkpoint record while archive + * recovery is still ongoing. Normally, this will have happened already + * while writing out dirty buffers, but not necessarily - e.g. because no + * buffers were dirtied. We do this because a non-exclusive base backup + * uses minRecoveryPoint to determine which WAL files must be included in + * the backup, and the file (or files) containing the checkpoint record + * must be included, at a minimum. Note that for an ordinary restart of + * recovery there's no value in having the minimum recovery point any + * earlier than this anyway, because redo will begin just after the + * checkpoint record. This is a quick hack to make sure nothing really bad + * happens if somehow we get here after the end-of-recovery checkpoint. */ - LWLockAcquire(ControlFileLock, LW_EXCLUSIVE); - if (ControlFile->state == DB_IN_ARCHIVE_RECOVERY && - ControlFile->checkPointCopy.redo < lastCheckPoint.redo) + if (ControlFile->state == DB_IN_ARCHIVE_RECOVERY) { - ControlFile->checkPoint = lastCheckPointRecPtr; - ControlFile->checkPointCopy = lastCheckPoint; - - /* - * Ensure minRecoveryPoint is past the checkpoint record. Normally, - * this will have happened already while writing out dirty buffers, - * but not necessarily - e.g. because no buffers were dirtied. We do - * this because a non-exclusive base backup uses minRecoveryPoint to - * determine which WAL files must be included in the backup, and the - * file (or files) containing the checkpoint record must be included, - * at a minimum. Note that for an ordinary restart of recovery there's - * no value in having the minimum recovery point any earlier than this - * anyway, because redo will begin just after the checkpoint record. - */ if (ControlFile->minRecoveryPoint < lastCheckPointEndPtr) { ControlFile->minRecoveryPoint = lastCheckPointEndPtr; @@ -7026,8 +7028,25 @@ CreateRestartPoint(int flags) } if (flags & CHECKPOINT_IS_SHUTDOWN) ControlFile->state = DB_SHUTDOWNED_IN_RECOVERY; - UpdateControlFile(); } + else + { + /* recovery mode is not supposed to end during shutdown restartpoint */ + Assert((flags & CHECKPOINT_IS_SHUTDOWN) == 0); + + /* + * Aarchive recovery has ended. Crash recovery ever after should + * always recover to the end of WAL + */ + ControlFile->minRecoveryPoint = InvalidXLogRecPtr; + ControlFile->minRecoveryPointTLI = 0; + + /* also update local copy */ + LocalMinRecoveryPoint = InvalidXLogRecPtr; + LocalMinRecoveryPointTLI = 0; + } + + UpdateControlFile(); LWLockRelease(ControlFileLock); /* @@ -7104,7 +7123,7 @@ CreateRestartPoint(int flags) xtime = GetLatestXTime(); ereport((log_checkpoints ? LOG : DEBUG2), (errmsg("recovery restart point at %X/%X", - LSN_FORMAT_ARGS(lastCheckPoint.redo)), + LSN_FORMAT_ARGS(RedoRecPtr)), xtime ? errdetail("Last completed transaction was at log time %s.", timestamptz_to_str(xtime)) : 0)); -- 2.27.0 ----Next_Part(Tue_Mar_15_17_23_40_2022_661)-- Content-Type: Text/X-Patch; charset=us-ascii Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="v12-0002-Add-checkpoint-and-redo-LSN-to-LogCheckpointEnd-.patch" ^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: Popcount optimization using AVX512 @ 2024-02-09 18:34 Andres Freund <[email protected]> 0 siblings, 1 reply; 8+ messages in thread From: Andres Freund @ 2024-02-09 18:34 UTC (permalink / raw) To: Amonson, Paul D <[email protected]>; +Cc: Alvaro Herrera <[email protected]>; Shankaran, Akash <[email protected]>; Nathan Bossart <[email protected]>; Noah Misch <[email protected]>; Tom Lane <[email protected]>; Matthias van de Meent <[email protected]>; [email protected] <[email protected]> Hi, On 2024-02-09 17:39:46 +0000, Amonson, Paul D wrote: > diff --git a/meson.build b/meson.build > index 8ed51b6aae..1e7a4dc942 100644 > --- a/meson.build > +++ b/meson.build > @@ -1773,6 +1773,45 @@ elif cc.links(''' > endif > > > +# XXX: The configure.ac check for __cpuidex() is broken, we don't copy that > +# here. To prevent problems due to two detection methods working, stop > +# checking after one. This seems like a bogus copy-paste. > +if cc.links(''' > + #include <cpuid.h> > + int main(int arg, char **argv) > + { > + unsigned int exx[4] = {0, 0, 0, 0}; > + __get_cpuid_count(7, 0, &exx[0], &exx[1], &exx[2], &exx[3]); > + } > + ''', name: '__get_cpuid_count', > + args: test_c_args) > + cdata.set('HAVE__GET_CPUID_COUNT', 1) > +elif cc.links(''' > + #include <intrin.h> > + int main(int arg, char **argv) > + { > + unsigned int exx[4] = {0, 0, 0, 0}; > + __cpuidex(exx, 7, 0); > + } > + ''', name: '__cpuidex', > + args: test_c_args) > + cdata.set('HAVE__CPUIDEX', 1) > +endif > + > + > +# Check for header immintrin.h > +if cc.links(''' > + #include <immintrin.h> > + int main(int arg, char **argv) > + { > + return 1701; > + } > + ''', name: '__immintrin', > + args: test_c_args) > + cdata.set('HAVE__IMMINTRIN', 1) > +endif Do these all actually have to link? Invoking the linker is slow. I think you might be able to just use cc.has_header_symbol(). > +############################################################### > +# AVX 512 POPCNT Intrinsic check > +############################################################### > +have_avx512_popcnt = false > +cflags_avx512_popcnt = [] > +if host_cpu == 'x86_64' > + prog = ''' > + #include <immintrin.h> > + #include <stdint.h> > + void main(void) > + { > + __m512i tmp __attribute__((aligned(64))); > + __m512i input = _mm512_setzero_si512(); > + __m512i output = _mm512_popcnt_epi64(input); > + uint64_t cnt = 999; > + _mm512_store_si512(&tmp, output); > + cnt = _mm512_reduce_add_epi64(tmp); > + /* return computed value, to prevent the above being optimized away */ > + return cnt == 0; > + }''' Does this work with msvc? > + if cc.links(prog, name: '_mm512_setzero_si512, _mm512_popcnt_epi64, _mm512_store_si512, and _mm512_reduce_add_epi64 with -mavx512vpopcntdq -mavx512f', That's a very long line in the output, how about using the avx feature name or something? > diff --git a/src/port/Makefile b/src/port/Makefile > index dcc8737e68..6a01a7d89a 100644 > --- a/src/port/Makefile > +++ b/src/port/Makefile > @@ -87,6 +87,11 @@ pg_crc32c_sse42.o: CFLAGS+=$(CFLAGS_CRC) > pg_crc32c_sse42_shlib.o: CFLAGS+=$(CFLAGS_CRC) > pg_crc32c_sse42_srv.o: CFLAGS+=$(CFLAGS_CRC) > > +# Newer Intel processors can use AVX-512 POPCNT Capabilities (01/30/2024) > +pg_bitutils.o: CFLAGS+=$(CFLAGS_AVX512_POPCNT) > +pg_bitutils_shlib.o: CFLAGS+=$(CFLAGS_AVX512_POPCNT) > +pg_bitutils_srv.o:CFLAGS+=$(CFLAGS_AVX512_POPCNT) > + > # all versions of pg_crc32c_armv8.o need CFLAGS_CRC > pg_crc32c_armv8.o: CFLAGS+=$(CFLAGS_CRC) > pg_crc32c_armv8_shlib.o: CFLAGS+=$(CFLAGS_CRC) > diff --git a/src/port/meson.build b/src/port/meson.build > index 69b30ab21b..1c48a3b07e 100644 > --- a/src/port/meson.build > +++ b/src/port/meson.build > @@ -184,6 +184,7 @@ foreach name, opts : pgport_variants > link_with: cflag_libs, > c_pch: pch_c_h, > kwargs: opts + { > + 'c_args': opts.get('c_args', []) + cflags_avx512_popcnt, > 'dependencies': opts['dependencies'] + [ssl], > } > ) This will build all of pgport with the avx flags, which wouldn't be correct, I think? The compiler might inject automatic uses of avx512 in places, which would cause problems, no? While you don't do the same for make, isn't even just using the avx512 for all of pg_bitutils.c broken for exactly that reson? That's why the existing code builds the files for various crc variants as their own file. Greetings, Andres Freund ^ permalink raw reply [nested|flat] 8+ messages in thread
* RE: Popcount optimization using AVX512 @ 2024-02-12 20:14 Amonson, Paul D <[email protected]> parent: Andres Freund <[email protected]> 0 siblings, 1 reply; 8+ messages in thread From: Amonson, Paul D @ 2024-02-12 20:14 UTC (permalink / raw) To: Andres Freund <[email protected]>; +Cc: Alvaro Herrera <[email protected]>; Shankaran, Akash <[email protected]>; Nathan Bossart <[email protected]>; Noah Misch <[email protected]>; Tom Lane <[email protected]>; Matthias van de Meent <[email protected]>; [email protected] <[email protected]> My responses with questions, > > +# XXX: The configure.ac check for __cpuidex() is broken, we don't > > +copy that # here. To prevent problems due to two detection methods > > +working, stop # checking after one. > > This seems like a bogus copy-paste. My bad. Will remove the offending comment. :) > > +# Check for header immintrin.h > ... > Do these all actually have to link? Invoking the linker is slow. > I think you might be able to just use cc.has_header_symbol(). I took this to mean the last of the 3 new blocks. I changed this one to the cc_has_header method. I think I do want the first 2 checking the link as well. If the don't link here they won't link in the actual build. > Does this work with msvc? I think it will work but I have no way to validate it. I propose we remove the AVX-512 popcount feature from MSVC builds. Sound ok? > That's a very long line in the output, how about using the avx feature name or something? Agree, will fix. > This will build all of pgport with the avx flags, which wouldn't be correct, I think? The compiler might inject automatic uses of avx512 in places, which would cause problems, no? This will take me some time to learn how to do this in meson. Any pointers here would be helpful. > While you don't do the same for make, isn't even just using the avx512 for all of pg_bitutils.c broken for exactly that reson? That's why the existing code builds the files for various crc variants as their own file. I don't think its broken, nothing else in pg_bitutils.c will make use of AVX-512, so I am not sure what dividing this up into multiple files will yield benefits beyond code readability as they will all be needed during compile time. I prefer to not split if the community agrees to it. If splitting still makes sense, I propose splitting into 3 files: pg_bitutils.c (entry point +sw popcnt implementation), pg_popcnt_choose.c (CPUID and xgetbv check) and pg_popcnt_x86_64_accel.c (64/512bit x86 implementations). I'm not an expert in meson, but splitting might add complexity to meson.build. Could you elaborate if there are other benefits to the split file approach? Paul -----Original Message----- From: Andres Freund <[email protected]> Sent: Friday, February 9, 2024 10:35 AM To: Amonson, Paul D <[email protected]> Cc: Alvaro Herrera <[email protected]>; Shankaran, Akash <[email protected]>; Nathan Bossart <[email protected]>; Noah Misch <[email protected]>; Tom Lane <[email protected]>; Matthias van de Meent <[email protected]>; [email protected] Subject: Re: Popcount optimization using AVX512 Hi, On 2024-02-09 17:39:46 +0000, Amonson, Paul D wrote: > diff --git a/meson.build b/meson.build index 8ed51b6aae..1e7a4dc942 > 100644 > --- a/meson.build > +++ b/meson.build > @@ -1773,6 +1773,45 @@ elif cc.links(''' > endif > > > +# XXX: The configure.ac check for __cpuidex() is broken, we don't > +copy that # here. To prevent problems due to two detection methods > +working, stop # checking after one. This seems like a bogus copy-paste. > +if cc.links(''' > + #include <cpuid.h> > + int main(int arg, char **argv) > + { > + unsigned int exx[4] = {0, 0, 0, 0}; > + __get_cpuid_count(7, 0, &exx[0], &exx[1], &exx[2], &exx[3]); > + } > + ''', name: '__get_cpuid_count', > + args: test_c_args) > + cdata.set('HAVE__GET_CPUID_COUNT', 1) elif cc.links(''' > + #include <intrin.h> > + int main(int arg, char **argv) > + { > + unsigned int exx[4] = {0, 0, 0, 0}; > + __cpuidex(exx, 7, 0); > + } > + ''', name: '__cpuidex', > + args: test_c_args) > + cdata.set('HAVE__CPUIDEX', 1) > +endif > + > + > +# Check for header immintrin.h > +if cc.links(''' > + #include <immintrin.h> > + int main(int arg, char **argv) > + { > + return 1701; > + } > + ''', name: '__immintrin', > + args: test_c_args) > + cdata.set('HAVE__IMMINTRIN', 1) > +endif Do these all actually have to link? Invoking the linker is slow. I think you might be able to just use cc.has_header_symbol(). > +############################################################### > +# AVX 512 POPCNT Intrinsic check > +############################################################### > +have_avx512_popcnt = false > +cflags_avx512_popcnt = [] > +if host_cpu == 'x86_64' > + prog = ''' > + #include <immintrin.h> > + #include <stdint.h> > + void main(void) > + { > + __m512i tmp __attribute__((aligned(64))); > + __m512i input = _mm512_setzero_si512(); > + __m512i output = _mm512_popcnt_epi64(input); > + uint64_t cnt = 999; > + _mm512_store_si512(&tmp, output); > + cnt = _mm512_reduce_add_epi64(tmp); > + /* return computed value, to prevent the above being optimized away */ > + return cnt == 0; > + }''' Does this work with msvc? > + if cc.links(prog, name: '_mm512_setzero_si512, > + _mm512_popcnt_epi64, _mm512_store_si512, and _mm512_reduce_add_epi64 > + with -mavx512vpopcntdq -mavx512f', That's a very long line in the output, how about using the avx feature name or something? > diff --git a/src/port/Makefile b/src/port/Makefile index > dcc8737e68..6a01a7d89a 100644 > --- a/src/port/Makefile > +++ b/src/port/Makefile > @@ -87,6 +87,11 @@ pg_crc32c_sse42.o: CFLAGS+=$(CFLAGS_CRC) > pg_crc32c_sse42_shlib.o: CFLAGS+=$(CFLAGS_CRC) > pg_crc32c_sse42_srv.o: CFLAGS+=$(CFLAGS_CRC) > > +# Newer Intel processors can use AVX-512 POPCNT Capabilities > +(01/30/2024) > +pg_bitutils.o: CFLAGS+=$(CFLAGS_AVX512_POPCNT) > +pg_bitutils_shlib.o: CFLAGS+=$(CFLAGS_AVX512_POPCNT) > +pg_bitutils_srv.o:CFLAGS+=$(CFLAGS_AVX512_POPCNT) > + > # all versions of pg_crc32c_armv8.o need CFLAGS_CRC > pg_crc32c_armv8.o: CFLAGS+=$(CFLAGS_CRC) > pg_crc32c_armv8_shlib.o: CFLAGS+=$(CFLAGS_CRC) diff --git > a/src/port/meson.build b/src/port/meson.build index > 69b30ab21b..1c48a3b07e 100644 > --- a/src/port/meson.build > +++ b/src/port/meson.build > @@ -184,6 +184,7 @@ foreach name, opts : pgport_variants > link_with: cflag_libs, > c_pch: pch_c_h, > kwargs: opts + { > + 'c_args': opts.get('c_args', []) + cflags_avx512_popcnt, > 'dependencies': opts['dependencies'] + [ssl], > } > ) This will build all of pgport with the avx flags, which wouldn't be correct, I think? The compiler might inject automatic uses of avx512 in places, which would cause problems, no? While you don't do the same for make, isn't even just using the avx512 for all of pg_bitutils.c broken for exactly that reson? That's why the existing code builds the files for various crc variants as their own file. Greetings, Andres Freund ^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: Popcount optimization using AVX512 @ 2024-02-12 20:37 Andres Freund <[email protected]> parent: Amonson, Paul D <[email protected]> 0 siblings, 1 reply; 8+ messages in thread From: Andres Freund @ 2024-02-12 20:37 UTC (permalink / raw) To: Amonson, Paul D <[email protected]>; +Cc: Alvaro Herrera <[email protected]>; Shankaran, Akash <[email protected]>; Nathan Bossart <[email protected]>; Noah Misch <[email protected]>; Tom Lane <[email protected]>; Matthias van de Meent <[email protected]>; [email protected] <[email protected]> Hi, On 2024-02-12 20:14:06 +0000, Amonson, Paul D wrote: > > > +# Check for header immintrin.h > > ... > > Do these all actually have to link? Invoking the linker is slow. > > I think you might be able to just use cc.has_header_symbol(). > > I took this to mean the last of the 3 new blocks. Yep. > > Does this work with msvc? > > I think it will work but I have no way to validate it. I propose we remove the AVX-512 popcount feature from MSVC builds. Sound ok? CI [1], whould be able to test at least building. Including via cfbot, automatically run for each commitfest entry - you can see prior runs at [2]. They run on Zen 3 epyc instances, so unfortunately runtime won't be tested. If you look at [3], you can see that currently it doesn't seem to be considered supported at configure time: ... [00:23:48.480] Checking if "__get_cpuid" : links: NO [00:23:48.480] Checking if "__cpuid" : links: YES ... [00:23:48.492] Checking if "x86_64: popcntq instruction" compiles: NO ... Unfortunately CI currently is configured to not upload the build logs if the build succeeds, so we don't have enough details to see why. > > This will build all of pgport with the avx flags, which wouldn't be correct, I think? The compiler might inject automatic uses of avx512 in places, which would cause problems, no? > > This will take me some time to learn how to do this in meson. Any pointers > here would be helpful. Should be fairly simple, add it to the replace_funcs_pos and add the relevant cflags to pgport_cflags, similar to how it's done for crc. > > While you don't do the same for make, isn't even just using the avx512 for all of pg_bitutils.c broken for exactly that reson? That's why the existing code builds the files for various crc variants as their own file. > > I don't think its broken, nothing else in pg_bitutils.c will make use of > AVX-512 You can't really guarantee that compiler auto-vectorization won't decide to do so, no? I wouldn't call it likely, but it's also hard to be sure it won't happen at some point. > If splitting still makes sense, I propose splitting into 3 files: pg_bitutils.c (entry point +sw popcnt implementation), pg_popcnt_choose.c (CPUID and xgetbv check) and pg_popcnt_x86_64_accel.c (64/512bit x86 implementations). > I'm not an expert in meson, but splitting might add complexity to meson.build. > > Could you elaborate if there are other benefits to the split file approach? It won't lead to SIGILLs ;) Greetings, Andres Freund [1] https://github.com/postgres/postgres/blob/master/src/tools/ci/README [2] https://cirrus-ci.com/github/postgresql-cfbot/postgresql/commitfest%2F47%2F4675 [3] https://cirrus-ci.com/task/5645112189911040 ^ permalink raw reply [nested|flat] 8+ messages in thread
* RE: Popcount optimization using AVX512 @ 2024-02-21 17:35 Amonson, Paul D <[email protected]> parent: Andres Freund <[email protected]> 0 siblings, 1 reply; 8+ messages in thread From: Amonson, Paul D @ 2024-02-21 17:35 UTC (permalink / raw) To: Andres Freund <[email protected]>; +Cc: Alvaro Herrera <[email protected]>; Shankaran, Akash <[email protected]>; Nathan Bossart <[email protected]>; Noah Misch <[email protected]>; Tom Lane <[email protected]>; Matthias van de Meent <[email protected]>; [email protected] <[email protected]> Hi, I am encountering a problem that I don't think I understand. I cannot get the MSVC build to link in CI. I added 2 files to the build, but the linker is complaining about the original pg_bitutils.c file is missing (specifically symbol 'pg_popcount'). To my knowledge my changes did not change linking for the offending file and I see the compiles for pg_bitutils.c in all 3 libs in the build. All other builds are compiling. Any help on this issue would be greatly appreciated. My fork is at https://github.com/paul-amonson/postgresql/tree/popcnt_patch and the CI build is at https://cirrus-ci.com/task/4927666021728256. Thanks, Paul -----Original Message----- From: Andres Freund <[email protected]> Sent: Monday, February 12, 2024 12:37 PM To: Amonson, Paul D <[email protected]> Cc: Alvaro Herrera <[email protected]>; Shankaran, Akash <[email protected]>; Nathan Bossart <[email protected]>; Noah Misch <[email protected]>; Tom Lane <[email protected]>; Matthias van de Meent <[email protected]>; [email protected] Subject: Re: Popcount optimization using AVX512 Hi, On 2024-02-12 20:14:06 +0000, Amonson, Paul D wrote: > > > +# Check for header immintrin.h > > ... > > Do these all actually have to link? Invoking the linker is slow. > > I think you might be able to just use cc.has_header_symbol(). > > I took this to mean the last of the 3 new blocks. Yep. > > Does this work with msvc? > > I think it will work but I have no way to validate it. I propose we remove the AVX-512 popcount feature from MSVC builds. Sound ok? CI [1], whould be able to test at least building. Including via cfbot, automatically run for each commitfest entry - you can see prior runs at [2]. They run on Zen 3 epyc instances, so unfortunately runtime won't be tested. If you look at [3], you can see that currently it doesn't seem to be considered supported at configure time: ... [00:23:48.480] Checking if "__get_cpuid" : links: NO [00:23:48.480] Checking if "__cpuid" : links: YES ... [00:23:48.492] Checking if "x86_64: popcntq instruction" compiles: NO ... Unfortunately CI currently is configured to not upload the build logs if the build succeeds, so we don't have enough details to see why. > > This will build all of pgport with the avx flags, which wouldn't be correct, I think? The compiler might inject automatic uses of avx512 in places, which would cause problems, no? > > This will take me some time to learn how to do this in meson. Any > pointers here would be helpful. Should be fairly simple, add it to the replace_funcs_pos and add the relevant cflags to pgport_cflags, similar to how it's done for crc. > > While you don't do the same for make, isn't even just using the avx512 for all of pg_bitutils.c broken for exactly that reson? That's why the existing code builds the files for various crc variants as their own file. > > I don't think its broken, nothing else in pg_bitutils.c will make use > of > AVX-512 You can't really guarantee that compiler auto-vectorization won't decide to do so, no? I wouldn't call it likely, but it's also hard to be sure it won't happen at some point. > If splitting still makes sense, I propose splitting into 3 files: pg_bitutils.c (entry point +sw popcnt implementation), pg_popcnt_choose.c (CPUID and xgetbv check) and pg_popcnt_x86_64_accel.c (64/512bit x86 implementations). > I'm not an expert in meson, but splitting might add complexity to meson.build. > > Could you elaborate if there are other benefits to the split file approach? It won't lead to SIGILLs ;) Greetings, Andres Freund [1] https://github.com/postgres/postgres/blob/master/src/tools/ci/README [2] https://cirrus-ci.com/github/postgresql-cfbot/postgresql/commitfest%2F47%2F4675 [3] https://cirrus-ci.com/task/5645112189911040 ^ permalink raw reply [nested|flat] 8+ messages in thread
* RE: Popcount optimization using AVX512 @ 2024-02-26 17:56 Amonson, Paul D <[email protected]> parent: Amonson, Paul D <[email protected]> 0 siblings, 1 reply; 8+ messages in thread From: Amonson, Paul D @ 2024-02-26 17:56 UTC (permalink / raw) To: Amonson, Paul D <[email protected]>; Andres Freund <[email protected]>; +Cc: Alvaro Herrera <[email protected]>; Shankaran, Akash <[email protected]>; Nathan Bossart <[email protected]>; Noah Misch <[email protected]>; Tom Lane <[email protected]>; Matthias van de Meent <[email protected]>; [email protected] <[email protected]> Hello again, This is now a blocking issue. I can find no reason for the failing behavior of the MSVC build. All other languages build fine in CI including the Mac. Since the master branch builds, I assume I changed something critical to linking, but I can't figure out what that would be. Can someone with Windows/MSVC experience help me? * Code: https://github.com/paul-amonson/postgresql/tree/popcnt_patch * CI build: https://cirrus-ci.com/task/4927666021728256 Thanks, Paul -----Original Message----- From: Amonson, Paul D <[email protected]> Sent: Wednesday, February 21, 2024 9:36 AM To: Andres Freund <[email protected]> Cc: Alvaro Herrera <[email protected]>; Shankaran, Akash <[email protected]>; Nathan Bossart <[email protected]>; Noah Misch <[email protected]>; Tom Lane <[email protected]>; Matthias van de Meent <[email protected]>; [email protected] Subject: RE: Popcount optimization using AVX512 Hi, I am encountering a problem that I don't think I understand. I cannot get the MSVC build to link in CI. I added 2 files to the build, but the linker is complaining about the original pg_bitutils.c file is missing (specifically symbol 'pg_popcount'). To my knowledge my changes did not change linking for the offending file and I see the compiles for pg_bitutils.c in all 3 libs in the build. All other builds are compiling. Any help on this issue would be greatly appreciated. My fork is at https://github.com/paul-amonson/postgresql/tree/popcnt_patch and the CI build is at https://cirrus-ci.com/task/4927666021728256. Thanks, Paul -----Original Message----- From: Andres Freund <[email protected]> Sent: Monday, February 12, 2024 12:37 PM To: Amonson, Paul D <[email protected]> Cc: Alvaro Herrera <[email protected]>; Shankaran, Akash <[email protected]>; Nathan Bossart <[email protected]>; Noah Misch <[email protected]>; Tom Lane <[email protected]>; Matthias van de Meent <[email protected]>; [email protected] Subject: Re: Popcount optimization using AVX512 Hi, On 2024-02-12 20:14:06 +0000, Amonson, Paul D wrote: > > > +# Check for header immintrin.h > > ... > > Do these all actually have to link? Invoking the linker is slow. > > I think you might be able to just use cc.has_header_symbol(). > > I took this to mean the last of the 3 new blocks. Yep. > > Does this work with msvc? > > I think it will work but I have no way to validate it. I propose we remove the AVX-512 popcount feature from MSVC builds. Sound ok? CI [1], whould be able to test at least building. Including via cfbot, automatically run for each commitfest entry - you can see prior runs at [2]. They run on Zen 3 epyc instances, so unfortunately runtime won't be tested. If you look at [3], you can see that currently it doesn't seem to be considered supported at configure time: ... [00:23:48.480] Checking if "__get_cpuid" : links: NO [00:23:48.480] Checking if "__cpuid" : links: YES ... [00:23:48.492] Checking if "x86_64: popcntq instruction" compiles: NO ... Unfortunately CI currently is configured to not upload the build logs if the build succeeds, so we don't have enough details to see why. > > This will build all of pgport with the avx flags, which wouldn't be correct, I think? The compiler might inject automatic uses of avx512 in places, which would cause problems, no? > > This will take me some time to learn how to do this in meson. Any > pointers here would be helpful. Should be fairly simple, add it to the replace_funcs_pos and add the relevant cflags to pgport_cflags, similar to how it's done for crc. > > While you don't do the same for make, isn't even just using the avx512 for all of pg_bitutils.c broken for exactly that reson? That's why the existing code builds the files for various crc variants as their own file. > > I don't think its broken, nothing else in pg_bitutils.c will make use > of > AVX-512 You can't really guarantee that compiler auto-vectorization won't decide to do so, no? I wouldn't call it likely, but it's also hard to be sure it won't happen at some point. > If splitting still makes sense, I propose splitting into 3 files: pg_bitutils.c (entry point +sw popcnt implementation), pg_popcnt_choose.c (CPUID and xgetbv check) and pg_popcnt_x86_64_accel.c (64/512bit x86 implementations). > I'm not an expert in meson, but splitting might add complexity to meson.build. > > Could you elaborate if there are other benefits to the split file approach? It won't lead to SIGILLs ;) Greetings, Andres Freund [1] https://github.com/postgres/postgres/blob/master/src/tools/ci/README [2] https://cirrus-ci.com/github/postgresql-cfbot/postgresql/commitfest%2F47%2F4675 [3] https://cirrus-ci.com/task/5645112189911040 ^ permalink raw reply [nested|flat] 8+ messages in thread
* RE: Popcount optimization using AVX512 @ 2024-02-27 20:46 Amonson, Paul D <[email protected]> parent: Amonson, Paul D <[email protected]> 0 siblings, 1 reply; 8+ messages in thread From: Amonson, Paul D @ 2024-02-27 20:46 UTC (permalink / raw) To: Andres Freund <[email protected]>; +Cc: Alvaro Herrera <[email protected]>; Shankaran, Akash <[email protected]>; Nathan Bossart <[email protected]>; Noah Misch <[email protected]>; Tom Lane <[email protected]>; Matthias van de Meent <[email protected]>; [email protected] <[email protected]> Andres, 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. Paul -----Original Message----- From: Amonson, Paul D <[email protected]> Sent: Monday, February 26, 2024 9:57 AM To: Amonson, Paul D <[email protected]>; Andres Freund <[email protected]> Cc: Alvaro Herrera <[email protected]>; Shankaran, Akash <[email protected]>; Nathan Bossart <[email protected]>; Noah Misch <[email protected]>; Tom Lane <[email protected]>; Matthias van de Meent <[email protected]>; [email protected] Subject: RE: Popcount optimization using AVX512 Hello again, This is now a blocking issue. I can find no reason for the failing behavior of the MSVC build. All other languages build fine in CI including the Mac. Since the master branch builds, I assume I changed something critical to linking, but I can't figure out what that would be. Can someone with Windows/MSVC experience help me? * Code: https://github.com/paul-amonson/postgresql/tree/popcnt_patch * CI build: https://cirrus-ci.com/task/4927666021728256 Thanks, Paul -----Original Message----- From: Amonson, Paul D <[email protected]> Sent: Wednesday, February 21, 2024 9:36 AM To: Andres Freund <[email protected]> Cc: Alvaro Herrera <[email protected]>; Shankaran, Akash <[email protected]>; Nathan Bossart <[email protected]>; Noah Misch <[email protected]>; Tom Lane <[email protected]>; Matthias van de Meent <[email protected]>; [email protected] Subject: RE: Popcount optimization using AVX512 Hi, I am encountering a problem that I don't think I understand. I cannot get the MSVC build to link in CI. I added 2 files to the build, but the linker is complaining about the original pg_bitutils.c file is missing (specifically symbol 'pg_popcount'). To my knowledge my changes did not change linking for the offending file and I see the compiles for pg_bitutils.c in all 3 libs in the build. All other builds are compiling. Any help on this issue would be greatly appreciated. My fork is at https://github.com/paul-amonson/postgresql/tree/popcnt_patch and the CI build is at https://cirrus-ci.com/task/4927666021728256. Thanks, Paul -----Original Message----- From: Andres Freund <[email protected]> Sent: Monday, February 12, 2024 12:37 PM To: Amonson, Paul D <[email protected]> Cc: Alvaro Herrera <[email protected]>; Shankaran, Akash <[email protected]>; Nathan Bossart <[email protected]>; Noah Misch <[email protected]>; Tom Lane <[email protected]>; Matthias van de Meent <[email protected]>; [email protected] Subject: Re: Popcount optimization using AVX512 Hi, On 2024-02-12 20:14:06 +0000, Amonson, Paul D wrote: > > > +# Check for header immintrin.h > > ... > > Do these all actually have to link? Invoking the linker is slow. > > I think you might be able to just use cc.has_header_symbol(). > > I took this to mean the last of the 3 new blocks. Yep. > > Does this work with msvc? > > I think it will work but I have no way to validate it. I propose we remove the AVX-512 popcount feature from MSVC builds. Sound ok? CI [1], whould be able to test at least building. Including via cfbot, automatically run for each commitfest entry - you can see prior runs at [2]. They run on Zen 3 epyc instances, so unfortunately runtime won't be tested. If you look at [3], you can see that currently it doesn't seem to be considered supported at configure time: ... [00:23:48.480] Checking if "__get_cpuid" : links: NO [00:23:48.480] Checking if "__cpuid" : links: YES ... [00:23:48.492] Checking if "x86_64: popcntq instruction" compiles: NO ... Unfortunately CI currently is configured to not upload the build logs if the build succeeds, so we don't have enough details to see why. > > This will build all of pgport with the avx flags, which wouldn't be correct, I think? The compiler might inject automatic uses of avx512 in places, which would cause problems, no? > > This will take me some time to learn how to do this in meson. Any > pointers here would be helpful. Should be fairly simple, add it to the replace_funcs_pos and add the relevant cflags to pgport_cflags, similar to how it's done for crc. > > While you don't do the same for make, isn't even just using the avx512 for all of pg_bitutils.c broken for exactly that reson? That's why the existing code builds the files for various crc variants as their own file. > > I don't think its broken, nothing else in pg_bitutils.c will make use > of > AVX-512 You can't really guarantee that compiler auto-vectorization won't decide to do so, no? I wouldn't call it likely, but it's also hard to be sure it won't happen at some point. > If splitting still makes sense, I propose splitting into 3 files: pg_bitutils.c (entry point +sw popcnt implementation), pg_popcnt_choose.c (CPUID and xgetbv check) and pg_popcnt_x86_64_accel.c (64/512bit x86 implementations). > I'm not an expert in meson, but splitting might add complexity to meson.build. > > Could you elaborate if there are other benefits to the split file approach? It won't lead to SIGILLs ;) Greetings, Andres Freund [1] https://github.com/postgres/postgres/blob/master/src/tools/ci/README [2] https://cirrus-ci.com/github/postgresql-cfbot/postgresql/commitfest%2F47%2F4675 [3] https://cirrus-ci.com/task/5645112189911040 Attachments: [application/octet-stream] v5-0001-Add-support-for-AVX512-implemented-POPCNT.patch (24.0K, ../../BL1PR11MB53046A243932039BD545EBF4DC592@BL1PR11MB5304.namprd11.prod.outlook.com/2-v5-0001-Add-support-for-AVX512-implemented-POPCNT.patch) download | inline diff: diff --git a/config/c-compiler.m4 b/config/c-compiler.m4 index 5db02b2ab7..a5a3246199 100644 --- a/config/c-compiler.m4 +++ b/config/c-compiler.m4 @@ -694,3 +694,36 @@ if test x"$Ac_cachevar" = x"yes"; then fi undefine([Ac_cachevar])dnl ])# PGAC_LOONGARCH_CRC32C_INTRINSICS + +# PGAC_AVX512_POPCNT_INTRINSICS +# --------------------------- +# Check if the compiler supports the x86_64 AVX512 POPCNT instructions using +# intrinsics used in CPUID features AVX512F and AVX512VPOPCNTDQ. +# +# Optional compiler flags can be passed as argument (e.g. -mavx512vpopcntdq). +# If the intrinsics are supported then pgac_avx512_popcnt_intrinsics and +# CFLAGS_AVX512_POPCNT are set. +AC_DEFUN([PGAC_AVX512_POPCNT_INTRINSICS], +[define([Ac_cachevar], [AS_TR_SH([pgac_cv_avx512_popcnt_intrinsics_$1])])dnl +AC_CACHE_CHECK([for _mm512_popcnt_epi64 with CFLAGS=$1], [Ac_cachevar], +[pgac_save_CFLAGS=$CFLAGS +CFLAGS="$pgac_save_CFLAGS $1" +AC_LINK_IFELSE([AC_LANG_PROGRAM([#include <immintrin.h> +#include <stdint.h>], + [__m512i tmp __attribute__((aligned(64))); + __m512i input = _mm512_setzero_si512(); + __m512i output = _mm512_popcnt_epi64(input); + uint64_t cnt = 999; + _mm512_store_si512(&tmp, output); + cnt = _mm512_reduce_add_epi64(tmp); + /* return computed value, to prevent the above being optimized away */ + return cnt == 0;])], + [Ac_cachevar=yes], + [Ac_cachevar=no]) +CFLAGS="$pgac_save_CFLAGS"]) +if test x"$Ac_cachevar" = x"yes"; then + CFLAGS_AVX512_POPCNT="$1" + pgac_avx512_popcnt_intrinsics=yes +fi +undefine([Ac_cachevar])dnl +])# PGAC_AVX512_POPCNT_INTRINSICS diff --git a/configure b/configure index 6b87e5c9a8..0252dab6d5 100755 --- a/configure +++ b/configure @@ -647,6 +647,7 @@ MSGFMT_FLAGS MSGFMT PG_CRC32C_OBJS CFLAGS_CRC +CFLAGS_AVX512_POPCNT LIBOBJS OPENSSL ZSTD @@ -15209,7 +15210,7 @@ else We can't simply define LARGE_OFF_T to be 9223372036854775807, since some C++ compilers masquerading as C compilers incorrectly reject 9223372036854775807. */ -#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)) int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721 && LARGE_OFF_T % 2147483647 == 1) ? 1 : -1]; @@ -15255,7 +15256,7 @@ else We can't simply define LARGE_OFF_T to be 9223372036854775807, since some C++ compilers masquerading as C compilers incorrectly reject 9223372036854775807. */ -#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)) int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721 && LARGE_OFF_T % 2147483647 == 1) ? 1 : -1]; @@ -15279,7 +15280,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext We can't simply define LARGE_OFF_T to be 9223372036854775807, since some C++ compilers masquerading as C compilers incorrectly reject 9223372036854775807. */ -#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)) int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721 && LARGE_OFF_T % 2147483647 == 1) ? 1 : -1]; @@ -15324,7 +15325,7 @@ else We can't simply define LARGE_OFF_T to be 9223372036854775807, since some C++ compilers masquerading as C compilers incorrectly reject 9223372036854775807. */ -#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)) int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721 && LARGE_OFF_T % 2147483647 == 1) ? 1 : -1]; @@ -15348,7 +15349,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext We can't simply define LARGE_OFF_T to be 9223372036854775807, since some C++ compilers masquerading as C compilers incorrectly reject 9223372036854775807. */ -#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)) int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721 && LARGE_OFF_T % 2147483647 == 1) ? 1 : -1]; @@ -17708,6 +17709,41 @@ $as_echo "#define HAVE__GET_CPUID 1" >>confdefs.h fi +# Check for x86 cpuid_count instruction +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for __get_cpuid_count" >&5 +$as_echo_n "checking for __get_cpuid_count... " >&6; } +if ${pgac_cv__get_cpuid_count+:} false; then : + $as_echo_n "(cached) " >&6 +else + cat confdefs.h - <<_ACEOF >conftest.$ac_ext +/* end confdefs.h. */ +#include <cpuid.h> +int +main () +{ +unsigned int exx[4] = {0, 0, 0, 0}; + __get_cpuid_count(7, 0, &exx[0], &exx[1], &exx[2], &exx[3]); + + ; + return 0; +} +_ACEOF +if ac_fn_c_try_link "$LINENO"; then : + pgac_cv__get_cpuid_count="yes" +else + pgac_cv__get_cpuid_count="no" +fi +rm -f core conftest.err conftest.$ac_objext \ + conftest$ac_exeext conftest.$ac_ext +fi +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv__get_cpuid_count" >&5 +$as_echo "$pgac_cv__get_cpuid_count" >&6; } +if test x"$pgac_cv__get_cpuid_count" = x"yes"; then + +$as_echo "#define HAVE__GET_CPUID_COUNT 1" >>confdefs.h + +fi + { $as_echo "$as_me:${as_lineno-$LINENO}: checking for __cpuid" >&5 $as_echo_n "checking for __cpuid... " >&6; } if ${pgac_cv__cpuid+:} false; then : @@ -17742,6 +17778,164 @@ $as_echo "#define HAVE__CPUID 1" >>confdefs.h fi +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for __cpuidex" >&5 +$as_echo_n "checking for __cpuidex... " >&6; } +if ${pgac_cv__cpuidex+:} false; then : + $as_echo_n "(cached) " >&6 +else + cat confdefs.h - <<_ACEOF >conftest.$ac_ext +/* end confdefs.h. */ +#include <intrin.h> +int +main () +{ +unsigned int exx[4] = {0, 0, 0, 0}; + __get_cpuidex(exx[0], 7, 0); + + ; + return 0; +} +_ACEOF +if ac_fn_c_try_link "$LINENO"; then : + pgac_cv__cpuidex="yes" +else + pgac_cv__cpuidex="no" +fi +rm -f core conftest.err conftest.$ac_objext \ + conftest$ac_exeext conftest.$ac_ext +fi +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv__cpuidex" >&5 +$as_echo "$pgac_cv__cpuidex" >&6; } +if test x"$pgac_cv__cpuidex" = x"yes"; then + +$as_echo "#define HAVE__CPUIDEX 1" >>confdefs.h + +fi + +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for __immintrin" >&5 +$as_echo_n "checking for __immintrin... " >&6; } +if ${pgac_cv__immintrin+:} false; then : + $as_echo_n "(cached) " >&6 +else + cat confdefs.h - <<_ACEOF >conftest.$ac_ext +/* end confdefs.h. */ +#include <immintrin.h> +int +main () +{ +/* Don't exclude code so added return. */ + return 1701; + + ; + return 0; +} +_ACEOF +if ac_fn_c_try_link "$LINENO"; then : + pgac_cv__immintrin="yes" +else + pgac_cv__immintrin="no" +fi +rm -f core conftest.err conftest.$ac_objext \ + conftest$ac_exeext conftest.$ac_ext +fi +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv__immintrin" >&5 +$as_echo "$pgac_cv__immintrin" >&6; } +if test x"$pgac_cv__immintrin" = x"yes"; then + +$as_echo "#define HAVE__IMMINTRIN 1" >>confdefs.h + +fi + +# Check for Intel AVX512 intrinsics to do POPCNT calculations. +# +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for _mm512_popcnt_epi64 with CFLAGS=" >&5 +$as_echo_n "checking for _mm512_popcnt_epi64 with CFLAGS=... " >&6; } +if ${pgac_cv_avx512_popcnt_intrinsics_+:} false; then : + $as_echo_n "(cached) " >&6 +else + pgac_save_CFLAGS=$CFLAGS +CFLAGS="$pgac_save_CFLAGS " +cat confdefs.h - <<_ACEOF >conftest.$ac_ext +/* end confdefs.h. */ +#include <immintrin.h> +#include <stdint.h> +int +main () +{ +__m512i tmp __attribute__((aligned(64))); + __m512i input = _mm512_setzero_si512(); + __m512i output = _mm512_popcnt_epi64(input); + uint64_t cnt = 999; + _mm512_store_si512(&tmp, output); + cnt = _mm512_reduce_add_epi64(tmp); + /* return computed value, to prevent the above being optimized away */ + return cnt == 0; + ; + return 0; +} +_ACEOF +if ac_fn_c_try_link "$LINENO"; then : + pgac_cv_avx512_popcnt_intrinsics_=yes +else + pgac_cv_avx512_popcnt_intrinsics_=no +fi +rm -f core conftest.err conftest.$ac_objext \ + conftest$ac_exeext conftest.$ac_ext +CFLAGS="$pgac_save_CFLAGS" +fi +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_avx512_popcnt_intrinsics_" >&5 +$as_echo "$pgac_cv_avx512_popcnt_intrinsics_" >&6; } +if test x"$pgac_cv_avx512_popcnt_intrinsics_" = x"yes"; then + CFLAGS_AVX512_POPCNT="" + pgac_avx512_popcnt_intrinsics=yes +fi + +if test x"$pgac_avx512_popcnt_intrinsics" != x"yes"; then + { $as_echo "$as_me:${as_lineno-$LINENO}: checking for _mm512_popcnt_epi64 with CFLAGS=-mavx512vpopcntdq -mavx512f" >&5 +$as_echo_n "checking for _mm512_popcnt_epi64 with CFLAGS=-mavx512vpopcntdq -mavx512f... " >&6; } +if ${pgac_cv_avx512_popcnt_intrinsics__mavx512vpopcntdq__mavx512f+:} false; then : + $as_echo_n "(cached) " >&6 +else + pgac_save_CFLAGS=$CFLAGS +CFLAGS="$pgac_save_CFLAGS -mavx512vpopcntdq -mavx512f" +cat confdefs.h - <<_ACEOF >conftest.$ac_ext +/* end confdefs.h. */ +#include <immintrin.h> +#include <stdint.h> +int +main () +{ +__m512i tmp __attribute__((aligned(64))); + __m512i input = _mm512_setzero_si512(); + __m512i output = _mm512_popcnt_epi64(input); + uint64_t cnt = 999; + _mm512_store_si512(&tmp, output); + cnt = _mm512_reduce_add_epi64(tmp); + /* return computed value, to prevent the above being optimized away */ + return cnt == 0; + ; + return 0; +} +_ACEOF +if ac_fn_c_try_link "$LINENO"; then : + pgac_cv_avx512_popcnt_intrinsics__mavx512vpopcntdq__mavx512f=yes +else + pgac_cv_avx512_popcnt_intrinsics__mavx512vpopcntdq__mavx512f=no +fi +rm -f core conftest.err conftest.$ac_objext \ + conftest$ac_exeext conftest.$ac_ext +CFLAGS="$pgac_save_CFLAGS" +fi +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_avx512_popcnt_intrinsics__mavx512vpopcntdq__mavx512f" >&5 +$as_echo "$pgac_cv_avx512_popcnt_intrinsics__mavx512vpopcntdq__mavx512f" >&6; } +if test x"$pgac_cv_avx512_popcnt_intrinsics__mavx512vpopcntdq__mavx512f" = x"yes"; then + CFLAGS_AVX512_POPCNT="-mavx512vpopcntdq -mavx512f" + pgac_avx512_popcnt_intrinsics=yes +fi + +fi + + # Check for Intel SSE 4.2 intrinsics to do CRC calculations. # # First check if the _mm_crc32_u8 and _mm_crc32_u64 intrinsics can be used diff --git a/configure.ac b/configure.ac index 6e64ece11d..8fcf635b08 100644 --- a/configure.ac +++ b/configure.ac @@ -2068,6 +2068,18 @@ if test x"$pgac_cv__get_cpuid" = x"yes"; then AC_DEFINE(HAVE__GET_CPUID, 1, [Define to 1 if you have __get_cpuid.]) fi +# Check for x86 cpuid_count instruction +AC_CACHE_CHECK([for __get_cpuid_count], [pgac_cv__get_cpuid_count], +[AC_LINK_IFELSE([AC_LANG_PROGRAM([#include <cpuid.h>], + [[unsigned int exx[4] = {0, 0, 0, 0}; + __get_cpuid_count(7, 0, &exx[0], &exx[1], &exx[2], &exx[3]); + ]])], + [pgac_cv__get_cpuid_count="yes"], + [pgac_cv__get_cpuid_count="no"])]) +if test x"$pgac_cv__get_cpuid_count" = x"yes"; then + AC_DEFINE(HAVE__GET_CPUID_COUNT, 1, [Define to 1 if you have __get_cpuid.]) +fi + AC_CACHE_CHECK([for __cpuid], [pgac_cv__cpuid], [AC_LINK_IFELSE([AC_LANG_PROGRAM([#include <intrin.h>], [[unsigned int exx[4] = {0, 0, 0, 0}; @@ -2079,6 +2091,36 @@ if test x"$pgac_cv__cpuid" = x"yes"; then AC_DEFINE(HAVE__CPUID, 1, [Define to 1 if you have __cpuid.]) fi +AC_CACHE_CHECK([for __cpuidex], [pgac_cv__cpuidex], +[AC_LINK_IFELSE([AC_LANG_PROGRAM([#include <intrin.h>], + [[unsigned int exx[4] = {0, 0, 0, 0}; + __get_cpuidex(exx[0], 7, 0); + ]])], + [pgac_cv__cpuidex="yes"], + [pgac_cv__cpuidex="no"])]) +if test x"$pgac_cv__cpuidex" = x"yes"; then + AC_DEFINE(HAVE__CPUIDEX, 1, [Define to 1 if you have __cpuidex.]) +fi + +AC_CACHE_CHECK([for __immintrin], [pgac_cv__immintrin], +[AC_LINK_IFELSE([AC_LANG_PROGRAM([#include <immintrin.h>], + [[/* Don't exclude code so added return. */ + return 1701; + ]])], + [pgac_cv__immintrin="yes"], + [pgac_cv__immintrin="no"])]) +if test x"$pgac_cv__immintrin" = x"yes"; then + AC_DEFINE(HAVE__IMMINTRIN, 1, [Define to 1 if you have immintrin.]) +fi + +# Check for Intel AVX512 intrinsics to do POPCNT calculations. +# +PGAC_AVX512_POPCNT_INTRINSICS([]) +if test x"$pgac_avx512_popcnt_intrinsics" != x"yes"; then + PGAC_AVX512_POPCNT_INTRINSICS([-mavx512vpopcntdq -mavx512f]) +fi +AC_SUBST(CFLAGS_AVX512_POPCNT) + # Check for Intel SSE 4.2 intrinsics to do CRC calculations. # # First check if the _mm_crc32_u8 and _mm_crc32_u64 intrinsics can be used diff --git a/meson.build b/meson.build index 8ed51b6aae..f64c10c496 100644 --- a/meson.build +++ b/meson.build @@ -1773,6 +1773,37 @@ elif cc.links(''' endif +if cc.links(''' + #include <cpuid.h> + int main(int arg, char **argv) + { + unsigned int exx[4] = {0, 0, 0, 0}; + __get_cpuid_count(7, 0, &exx[0], &exx[1], &exx[2], &exx[3]); + } + ''', name: '__get_cpuid_count', + args: test_c_args) + cdata.set('HAVE__GET_CPUID_COUNT', 1) +elif cc.links(''' + #include <intrin.h> + int main(int arg, char **argv) + { + unsigned int exx[4] = {0, 0, 0, 0}; + __cpuidex(exx, 7, 0); + } + ''', name: '__cpuidex', + args: test_c_args) + cdata.set('HAVE__CPUIDEX', 1) +endif + + +# Check for header immintrin.h +if cc.has_header('immintrin.h', + include_directories: postgres_inc, args: test_c_args) + cdata.set('HAVE__IMMINTRIN', 1, + description: 'Define to 1 if you have the immintrin.h header file.') +endif + + # Defend against clang being used on x86-32 without SSE2 enabled. As current # versions of clang do not understand -fexcess-precision=standard, the use of # x87 floating point operations leads to problems like isinf possibly returning @@ -2146,6 +2177,38 @@ elif host_cpu == 'ppc' or host_cpu == 'ppc64' endif endif +############################################################### +# AVX 512 POPCNT Intrinsic check +############################################################### +have_avx512_popcnt = false +cflags_avx512_popcnt = [] +if host_cpu == 'x86_64' + prog = ''' + #include <immintrin.h> + #include <stdint.h> + void main(void) + { + __m512i tmp __attribute__((aligned(64))); + __m512i input = _mm512_setzero_si512(); + __m512i output = _mm512_popcnt_epi64(input); + uint64_t cnt = 999; + _mm512_store_si512(&tmp, output); + cnt = _mm512_reduce_add_epi64(tmp); + /* return computed value, to prevent the above being optimized away */ + return cnt == 0; + }''' + if cc.links(prog, name: '_mm512_* methods with -mavx512vpopcntdq -mavx512f', + args: test_c_args + ['-mavx512vpopcntdq', '-mavx512f']) + have_avx512_popcnt = true + cdata.set('USE_AVX512_POPCNT_WITH_RUNTIME_CHECK', 1) + cflags_avx512_popcnt = ['-mavx512vpopcntdq', '-mavx512f'] + else + have_avx512_popcnt = false + cdata.set('USE_AVX512_POPCNT_WITH_RUNTIME_CHECK', 1) + cflags_avx512_popcnt = [] + endif # compile/link test +endif # host_cpu check + ############################################################### diff --git a/src/Makefile.global.in b/src/Makefile.global.in index 8b3f8c24e0..089f49b7f3 100644 --- a/src/Makefile.global.in +++ b/src/Makefile.global.in @@ -263,6 +263,7 @@ CXXFLAGS_SL_MODULE = @CXXFLAGS_SL_MODULE@ CFLAGS_UNROLL_LOOPS = @CFLAGS_UNROLL_LOOPS@ CFLAGS_VECTORIZE = @CFLAGS_VECTORIZE@ CFLAGS_CRC = @CFLAGS_CRC@ +CFLAGS_AVX512_POPCNT = @CFLAGS_AVX512_POPCNT@ PERMIT_DECLARATION_AFTER_STATEMENT = @PERMIT_DECLARATION_AFTER_STATEMENT@ CXXFLAGS = @CXXFLAGS@ diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in index 07e73567dc..20e14c6499 100644 --- a/src/include/pg_config.h.in +++ b/src/include/pg_config.h.in @@ -555,6 +555,12 @@ /* Define to 1 if you have __get_cpuid. */ #undef HAVE__GET_CPUID +/* Define to 1 if you have __get_cpuid_count. */ +#undef HAVE__GET_CPUID_COUNT + +/* Define to 1 if you have immintrin. */ +#undef HAVE__IMMINTRIN + /* Define to 1 if your compiler understands _Static_assert. */ #undef HAVE__STATIC_ASSERT diff --git a/src/include/port/pg_bitutils.h b/src/include/port/pg_bitutils.h index 799f70d052..438470348a 100644 --- a/src/include/port/pg_bitutils.h +++ b/src/include/port/pg_bitutils.h @@ -303,16 +303,18 @@ pg_ceil_log2_64(uint64 num) extern int (*pg_popcount32) (uint32 word); extern int (*pg_popcount64) (uint64 word); +extern uint64 (*pg_popcount)(const char *buf, int bytes); + #else /* Use a portable implementation -- no need for a function pointer. */ extern int pg_popcount32(uint32 word); extern int pg_popcount64(uint64 word); -#endif /* TRY_POPCNT_FAST */ - /* Count the number of one-bits in a byte array */ extern uint64 pg_popcount(const char *buf, int bytes); +#endif /* TRY_POPCNT_FAST */ + /* * Rotate the bits of "word" to the right/left by n bits. */ diff --git a/src/makefiles/meson.build b/src/makefiles/meson.build index b0f4178b3d..ee3647282e 100644 --- a/src/makefiles/meson.build +++ b/src/makefiles/meson.build @@ -100,6 +100,7 @@ pgxs_kv = { ' '.join(cflags_no_decl_after_statement), 'CFLAGS_CRC': ' '.join(cflags_crc), + 'CFLAGS_AVX512_POPCNT': ' '.join(cflags_avx512_popcnt), 'CFLAGS_UNROLL_LOOPS': ' '.join(unroll_loops_cflags), 'CFLAGS_VECTORIZE': ' '.join(vectorize_cflags), diff --git a/src/port/Makefile b/src/port/Makefile index dcc8737e68..ef6c02a6bf 100644 --- a/src/port/Makefile +++ b/src/port/Makefile @@ -43,6 +43,8 @@ OBJS = \ inet_net_ntop.o \ noblock.o \ path.o \ + pg_popcnt_choose.o \ + pg_popcnt_x86_64_accel.o \ pg_bitutils.o \ pg_strong_random.o \ pgcheckdir.o \ @@ -87,6 +89,11 @@ pg_crc32c_sse42.o: CFLAGS+=$(CFLAGS_CRC) pg_crc32c_sse42_shlib.o: CFLAGS+=$(CFLAGS_CRC) pg_crc32c_sse42_srv.o: CFLAGS+=$(CFLAGS_CRC) +# Newer Intel processors can use AVX-512 POPCNT Capabilities (01/30/2024) +pg_popcnt_x86_64_accel.o: CFLAGS+=$(CFLAGS_AVX512_POPCNT) +pg_popcnt_x86_64_accel_shlib.o: CFLAGS+=$(CFLAGS_AVX512_POPCNT) +pg_popcnt_x86_64_accel_srv.o: CFLAGS+=$(CFLAGS_AVX512_POPCNT) + # all versions of pg_crc32c_armv8.o need CFLAGS_CRC pg_crc32c_armv8.o: CFLAGS+=$(CFLAGS_CRC) pg_crc32c_armv8_shlib.o: CFLAGS+=$(CFLAGS_CRC) diff --git a/src/port/meson.build b/src/port/meson.build index 92b593e6ef..d7930672cb 100644 --- a/src/port/meson.build +++ b/src/port/meson.build @@ -7,6 +7,7 @@ pgport_sources = [ 'noblock.c', 'path.c', 'pg_bitutils.c', + 'pg_popcnt_choose.c', 'pg_strong_random.c', 'pgcheckdir.c', 'pgmkdirp.c', @@ -84,6 +85,7 @@ replace_funcs_pos = [ ['pg_crc32c_sse42', 'USE_SSE42_CRC32C_WITH_RUNTIME_CHECK', 'crc'], ['pg_crc32c_sse42_choose', 'USE_SSE42_CRC32C_WITH_RUNTIME_CHECK'], ['pg_crc32c_sb8', 'USE_SSE42_CRC32C_WITH_RUNTIME_CHECK'], + ['pg_popcnt_x86_64_accel', 'USE_AVX512_POPCNT_WITH_RUNTIME_CHECK', 'avx512'], # arm / aarch64 ['pg_crc32c_armv8', 'USE_ARMV8_CRC32C'], @@ -98,8 +100,8 @@ replace_funcs_pos = [ ['pg_crc32c_sb8', 'USE_SLICING_BY_8_CRC32C'], ] -pgport_cflags = {'crc': cflags_crc} -pgport_sources_cflags = {'crc': []} +pgport_cflags = {'crc': cflags_crc, 'avx512': cflags_avx512_popcnt} +pgport_sources_cflags = {'crc': [], 'avx512': []} foreach f : replace_funcs_neg func = f.get(0) diff --git a/src/port/pg_bitutils.c b/src/port/pg_bitutils.c index 640a89561a..90cfa65333 100644 --- a/src/port/pg_bitutils.c +++ b/src/port/pg_bitutils.c @@ -12,16 +12,8 @@ */ #include "c.h" -#ifdef HAVE__GET_CPUID -#include <cpuid.h> -#endif -#ifdef HAVE__CPUID -#include <intrin.h> -#endif - #include "port/pg_bitutils.h" - /* * Array giving the position of the left-most set bit for each possible * byte value. We count the right-most position as the 0th bit, and the @@ -78,6 +70,7 @@ const uint8 pg_rightmost_one_pos[256] = { 4, 0, 1, 0, 2, 0, 1, 0, 3, 0, 1, 0, 2, 0, 1, 0 }; + /* * Array giving the number of 1-bits in each possible byte value. * @@ -103,123 +96,27 @@ const uint8 pg_number_of_ones[256] = { 4, 5, 5, 6, 5, 6, 6, 7, 5, 6, 6, 7, 6, 7, 7, 8 }; -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); #ifdef TRY_POPCNT_FAST -static bool pg_popcount_available(void); -static int pg_popcount32_choose(uint32 word); -static int pg_popcount64_choose(uint64 word); -static int pg_popcount32_fast(uint32 word); -static int pg_popcount64_fast(uint64 word); +extern int pg_popcount32_choose(uint32 word); +extern int pg_popcount64_choose(uint64 word); +extern uint64 pg_popcount_choose(const char *buf, int bytes); int (*pg_popcount32) (uint32 word) = pg_popcount32_choose; int (*pg_popcount64) (uint64 word) = pg_popcount64_choose; -#endif /* TRY_POPCNT_FAST */ - -#ifdef TRY_POPCNT_FAST - -/* - * Return true if CPUID indicates that the POPCNT instruction is available. - */ -static bool -pg_popcount_available(void) -{ - unsigned int exx[4] = {0, 0, 0, 0}; - -#if defined(HAVE__GET_CPUID) - __get_cpuid(1, &exx[0], &exx[1], &exx[2], &exx[3]); -#elif defined(HAVE__CPUID) - __cpuid(exx, 1); -#else -#error cpuid instruction not available -#endif - - return (exx[2] & (1 << 23)) != 0; /* POPCNT */ -} - -/* - * These functions get called on the first call to pg_popcount32 etc. - * They detect whether we can use the asm implementations, and replace - * the function pointers so that subsequent calls are routed directly to - * the chosen implementation. - */ -static int -pg_popcount32_choose(uint32 word) -{ - if (pg_popcount_available()) - { - pg_popcount32 = pg_popcount32_fast; - pg_popcount64 = pg_popcount64_fast; - } - else - { - pg_popcount32 = pg_popcount32_slow; - pg_popcount64 = pg_popcount64_slow; - } - - return pg_popcount32(word); -} - -static int -pg_popcount64_choose(uint64 word) -{ - if (pg_popcount_available()) - { - pg_popcount32 = pg_popcount32_fast; - pg_popcount64 = pg_popcount64_fast; - } - else - { - pg_popcount32 = pg_popcount32_slow; - pg_popcount64 = pg_popcount64_slow; - } - - return pg_popcount64(word); -} - -/* - * pg_popcount32_fast - * Return the number of 1 bits set in word - */ -static int -pg_popcount32_fast(uint32 word) -{ -#ifdef _MSC_VER - return __popcnt(word); -#else - uint32 res; - -__asm__ __volatile__(" popcntl %1,%0\n":"=q"(res):"rm"(word):"cc"); - return (int) res; -#endif -} - -/* - * pg_popcount64_fast - * Return the number of 1 bits set in word - */ -static int -pg_popcount64_fast(uint64 word) -{ -#ifdef _MSC_VER - return __popcnt64(word); -#else - uint64 res; - -__asm__ __volatile__(" popcntq %1,%0\n":"=q"(res):"rm"(word):"cc"); - return (int) res; -#endif -} - -#endif /* TRY_POPCNT_FAST */ - +uint64 (*pg_popcount) (const char *buf, int bytes) = pg_popcount_choose; +#else /* TRY_POPCNT_FAST */ +uint64 pg_popcount(const char *buf, int bytes); +#endif /* TRY_POPCNT_FAST */ /* * pg_popcount32_slow * Return the number of 1 bits set in word */ -static int +int pg_popcount32_slow(uint32 word) { #ifdef HAVE__BUILTIN_POPCOUNT @@ -241,7 +138,7 @@ pg_popcount32_slow(uint32 word) * pg_popcount64_slow * Return the number of 1 bits set in word */ -static int +int pg_popcount64_slow(uint64 word) { #ifdef HAVE__BUILTIN_POPCOUNT @@ -286,22 +183,29 @@ pg_popcount64(uint64 word) return pg_popcount64_slow(word); } +uint64 +pg_popcount(const char *buf, int bytes) +{ + return pg_popcount_slow(buf, bytes); +} + #endif /* !TRY_POPCNT_FAST */ /* * pg_popcount - * Returns the number of 1-bits in buf + * Returns the number of 1-bits in buf using either 32 or 64 bit loops + * or fallback to __builtin_* or pure software. */ uint64 -pg_popcount(const char *buf, int bytes) +pg_popcount_slow(const char *buf, int bytes) { uint64 popcnt = 0; -#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)) { ^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: Popcount optimization using AVX512 @ 2024-03-01 21:44 Nathan Bossart <[email protected]> parent: Amonson, Paul D <[email protected]> 0 siblings, 0 replies; 8+ messages in thread From: Nathan Bossart @ 2024-03-01 21:44 UTC (permalink / raw) To: Amonson, Paul D <[email protected]>; +Cc: Andres Freund <[email protected]>; Alvaro Herrera <[email protected]>; Shankaran, Akash <[email protected]>; Noah Misch <[email protected]>; Tom Lane <[email protected]>; Matthias van de Meent <[email protected]>; [email protected] <[email protected]> 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 ^ permalink raw reply [nested|flat] 8+ messages in thread
end of thread, other threads:[~2024-03-01 21:44 UTC | newest] Thread overview: 8+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2022-03-04 04:18 [PATCH v12 1/5] Correctly update contfol file at the end of archive recovery Kyotaro Horiguchi <[email protected]> 2024-02-09 18:34 Re: Popcount optimization using AVX512 Andres Freund <[email protected]> 2024-02-12 20:14 ` RE: Popcount optimization using AVX512 Amonson, Paul D <[email protected]> 2024-02-12 20:37 ` Re: Popcount optimization using AVX512 Andres Freund <[email protected]> 2024-02-21 17:35 ` RE: Popcount optimization using AVX512 Amonson, Paul D <[email protected]> 2024-02-26 17:56 ` RE: Popcount optimization using AVX512 Amonson, Paul D <[email protected]> 2024-02-27 20:46 ` RE: Popcount optimization using AVX512 Amonson, Paul D <[email protected]> 2024-03-01 21:44 ` Re: Popcount optimization using AVX512 Nathan Bossart <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox