public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v4 5/7] Row pattern recognition patch (docs).
26+ messages / 6 participants
[nested] [flat]
* [PATCH v4 5/7] Row pattern recognition patch (docs).
@ 2023-08-09 07:56 Tatsuo Ishii <[email protected]>
0 siblings, 0 replies; 26+ messages in thread
From: Tatsuo Ishii @ 2023-08-09 07:56 UTC (permalink / raw)
---
doc/src/sgml/advanced.sgml | 52 ++++++++++++++++++++++++++++++++++
doc/src/sgml/func.sgml | 54 ++++++++++++++++++++++++++++++++++++
doc/src/sgml/ref/select.sgml | 38 +++++++++++++++++++++++--
3 files changed, 142 insertions(+), 2 deletions(-)
diff --git a/doc/src/sgml/advanced.sgml b/doc/src/sgml/advanced.sgml
index 755c9f1485..eda3612822 100644
--- a/doc/src/sgml/advanced.sgml
+++ b/doc/src/sgml/advanced.sgml
@@ -537,6 +537,58 @@ WHERE pos < 3;
<literal>rank</literal> less than 3.
</para>
+ <para>
+ Row pattern common syntax can be used with row pattern common syntax to
+ perform row pattern recognition in a query. Row pattern common syntax
+ includes two sub clauses. <literal>DEFINE</literal> defines definition
+ variables along with an expression. The expression must be a logical
+ expression, which means it must
+ return <literal>TRUE</literal>, <literal>FALSE</literal>
+ or <literal>NULL</literal>. Moreover if the expression comprises a column
+ reference, it must be the argument of <function>rpr</function>. An example
+ of <literal>DEFINE</literal> is as follows.
+
+<programlisting>
+DEFINE
+ LOWPRICE AS price <= 100,
+ UP AS price > PREV(price),
+ DOWN AS price < PREV(price)
+</programlisting>
+
+ Note that <function>PREV</function> returns price column in the previous
+ row if it's called in a context of row pattern recognition. So in the
+ second line means the definition variable "UP" is <literal>TRUE</literal>
+ when price column in the current row is greater than the price column in
+ the previous row. Likewise, "DOWN" is <literal>TRUE</literal> when when
+ price column in the current row is lower than the price column in the
+ previous row.
+ </para>
+ <para>
+ Once <literal>DEFINE</literal> exists, <literal>PATTERN</literal> can be
+ used. <literal>PATTERN</literal> defines a sequence of rows that satisfies
+ certain conditions. For example following <literal>PATTERN</literal>
+ defines that a row starts with the condition "LOWPRICE", then one or more
+ rows satisfy "UP" and finally one or more rows satisfy "DOWN". If a
+ sequence of rows found, rpr returns the column at the starting row.
+ Example of a <literal>SELECT</literal> using the <literal>DEFINE</literal>
+ and <literal>PATTERN</literal> clause is as follows.
+
+<programlisting>
+SELECT company, tdate, price, max(price) OVER w FROM stock
+ WINDOW w AS (
+ PARTITION BY company
+ ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
+ AFTER MATCH SKIP PAST LAST ROW
+ INITIAL
+ PATTERN (LOWPRICE UP+ DOWN+)
+ DEFINE
+ LOWPRICE AS price <= 100,
+ UP AS price > PREV(price),
+ DOWN AS price < PREV(price)
+);
+</programlisting>
+ </para>
+
<para>
When a query involves multiple window functions, it is possible to write
out each one with a separate <literal>OVER</literal> clause, but this is
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index be2f54c914..6cda164522 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -21715,6 +21715,7 @@ SELECT count(*) FROM sometable;
returns <literal>NULL</literal> if there is no such row.
</para></entry>
</row>
+
</tbody>
</tgroup>
</table>
@@ -21754,6 +21755,59 @@ SELECT count(*) FROM sometable;
Other frame specifications can be used to obtain other effects.
</para>
+ <para>
+ Row pattern recognition navigation functions are listed in
+ <xref linkend="functions-rpr-navigation-table"/>. These functions
+ can be used to describe DEFINE clause of Row pattern recognition.
+ </para>
+
+ <table id="functions-rpr-navigation-table">
+ <title>Row Pattern Navigation Functions</title>
+ <tgroup cols="1">
+ <thead>
+ <row>
+ <entry role="func_table_entry"><para role="func_signature">
+ Function
+ </para>
+ <para>
+ Description
+ </para></entry>
+ </row>
+ </thead>
+
+ <tbody>
+ <row>
+ <entry role="func_table_entry"><para role="func_signature">
+ <indexterm>
+ <primary>prev</primary>
+ </indexterm>
+ <function>prev</function> ( <parameter>value</parameter> <type>anyelement</type> )
+ <returnvalue>anyelement</returnvalue>
+ </para>
+ <para>
+ Returns the column value at the previous row;
+ returns NULL if there is no previous row in the window frame.
+ </para></entry>
+ </row>
+
+ <row>
+ <entry role="func_table_entry"><para role="func_signature">
+ <indexterm>
+ <primary>next</primary>
+ </indexterm>
+ <function>next</function> ( <parameter>value</parameter> <type>anyelement</type> )
+ <returnvalue>anyelement</returnvalue>
+ </para>
+ <para>
+ Returns the column value at the next row;
+ returns NULL if there is no next row in the window frame.
+ </para></entry>
+ </row>
+
+ </tbody>
+ </tgroup>
+ </table>
+
<note>
<para>
The SQL standard defines a <literal>RESPECT NULLS</literal> or
diff --git a/doc/src/sgml/ref/select.sgml b/doc/src/sgml/ref/select.sgml
index 0ee0cc7e64..8d3becd57a 100644
--- a/doc/src/sgml/ref/select.sgml
+++ b/doc/src/sgml/ref/select.sgml
@@ -966,8 +966,8 @@ WINDOW <replaceable class="parameter">window_name</replaceable> AS ( <replaceabl
The <replaceable class="parameter">frame_clause</replaceable> can be one of
<synopsis>
-{ RANGE | ROWS | GROUPS } <replaceable>frame_start</replaceable> [ <replaceable>frame_exclusion</replaceable> ]
-{ RANGE | ROWS | GROUPS } BETWEEN <replaceable>frame_start</replaceable> AND <replaceable>frame_end</replaceable> [ <replaceable>frame_exclusion</replaceable> ]
+{ RANGE | ROWS | GROUPS } <replaceable>frame_start</replaceable> [ <replaceable>frame_exclusion</replaceable> ] [row_pattern_common_syntax]
+{ RANGE | ROWS | GROUPS } BETWEEN <replaceable>frame_start</replaceable> AND <replaceable>frame_end</replaceable> [ <replaceable>frame_exclusion</replaceable> ] [row_pattern_common_syntax]
</synopsis>
where <replaceable>frame_start</replaceable>
@@ -1074,6 +1074,40 @@ EXCLUDE NO OTHERS
a given peer group will be in the frame or excluded from it.
</para>
+ <para>
+ The
+ optional <replaceable class="parameter">row_pattern_common_syntax</replaceable>
+ defines the <firstterm>row pattern recognition condition</firstterm> for
+ this
+ window. <replaceable class="parameter">row_pattern_common_syntax</replaceable>
+ includes following subclauses. <literal>AFTER MATCH SKIP PAST LAST
+ ROW</literal> or <literal>AFTER MATCH SKIP TO NEXT ROW</literal> controls
+ how to proceed to next row position after a match
+ found. With <literal>AFTER MATCH SKIP PAST LAST ROW</literal> (the
+ default) next row position is next to the last row of previous match. On
+ the other hand, with <literal>AFTER MATCH SKIP TO NEXT ROW</literal> next
+ row position is always next to the last row of previous
+ match. <literal>DEFINE</literal> defines definition variables along with a
+ boolean expression. <literal>PATTERN</literal> defines a sequence of rows
+ that satisfies certain conditions using variables defined
+ in <literal>DEFINE</literal> clause. If the variable is not defined in
+ the <literal>DEFINE</literal> clause, it is implicitly assumed
+ following is defined in the <literal>DEFINE</literal> clause.
+
+<synopsis>
+<literal>variable_name</literal> AS TRUE
+</synopsis>
+
+ Note that the maximu number of variables defined
+ in <literal>DEFINE</literal> clause is 26.
+
+<synopsis>
+[ AFTER MATCH SKIP PAST LAST ROW | AFTER MATCH SKIP TO NEXT ROW ]
+PATTERN <replaceable class="parameter">pattern_variable_name</replaceable>[+] [, ...]
+DEFINE <replaceable class="parameter">definition_varible_name</replaceable> AS <replaceable class="parameter">expression</replaceable> [, ...]
+</synopsis>
+ </para>
+
<para>
The purpose of a <literal>WINDOW</literal> clause is to specify the
behavior of <firstterm>window functions</firstterm> appearing in the query's
--
2.25.1
----Next_Part(Wed_Aug__9_17_41_12_2023_134)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="v4-0006-Row-pattern-recognition-patch-tests.patch"
^ permalink raw reply [nested|flat] 26+ messages in thread
* RE: Popcount optimization using AVX512
@ 2024-03-04 21:39 Amonson, Paul D <[email protected]>
0 siblings, 2 replies; 26+ messages in thread
From: Amonson, Paul D @ 2024-03-04 21:39 UTC (permalink / raw)
To: Nathan Bossart <[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]>
Hi,
First, apologies on the patch. Find re-attached updated version.
Now I have some questions....
#1
> -#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].
I am not sure what the ask is here? I made changes to the configure.ac and ran autoconf2.69 to get builds to succeed. Do you have a separate feedback here?
#2
As for the refactoring, this was done to satisfy previous review feedback about applying the AVX512 CFLAGS to the entire pg_bitutils.c file. Mainly to avoid segfault due to the AVX512 flags. If its ok, I would prefer to make a single commit as the change is pretty small and straight forward.
#3
I am not sure I understand the comment about the SIZE_VOID_P checks. Aren't they necessary to choose which functions to call based on 32 or 64 bit architectures?
#4
Would this change qualify for Workflow A as described in [0] and can be picked up by a committer, given it has been reviewed by multiple committers so far? The scope of the change is pretty contained as well.
[0] https://wiki.postgresql.org/wiki/Submitting_a_Patch
Thanks,
Paul
-----Original Message-----
From: Nathan Bossart <[email protected]>
Sent: Friday, March 1, 2024 1:45 PM
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]
Subject: Re: Popcount optimization using AVX512
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
Attachments:
[application/octet-stream] v5-0001-Add-support-for-AVX512-implemented-POPCNT.patch (31.7K, ../../BL1PR11MB5304BFE26BAC25624508CFD2DC232@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..bd297d9fa9 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,43 @@ 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'
+ test_flags = ['-mavx512vpopcntdq', '-mavx512f']
+ if host_system == 'windows'
+ test_flags = ['/arch:AVX512']
+ endif
+ 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 + test_flags)
+ have_avx512_popcnt = true
+ cdata.set('USE_AVX512_POPCNT_WITH_RUNTIME_CHECK', 1)
+ cdata.set('HAVE__AVX512_POPCNT', 1)
+ cflags_avx512_popcnt = test_flags
+ 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..caca78d805 100644
--- a/src/include/port/pg_bitutils.h
+++ b/src/include/port/pg_bitutils.h
@@ -303,16 +303,23 @@ pg_ceil_log2_64(uint64 num)
extern int (*pg_popcount32) (uint32 word);
extern int (*pg_popcount64) (uint64 word);
+#if defined(_MSC_VER)
+extern uint64 pg_popcount(const char *buf, int bytes);
+extern uint64 (*pg_popcount_indirect)(const char *buf, int bytes);
+#else
+extern uint64 (*pg_popcount)(const char *buf, int bytes);
+#endif
+
#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..942e396141 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,35 @@ 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)
+#if defined(_MSC_VER)
+uint64 (*pg_popcount_indirect)(const char *buf, int bytes) = pg_popcount_choose;
+uint64 pg_popcount(const char *buf, int bytes)
{
-#ifdef _MSC_VER
- return __popcnt(word);
-#else
- uint32 res;
-
-__asm__ __volatile__(" popcntl %1,%0\n":"=q"(res):"rm"(word):"cc");
- return (int) res;
-#endif
+ return pg_popcount_indirect(buf, bytes);
}
-
-/*
- * 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;
+uint64 (*pg_popcount) (const char *buf, int bytes) = pg_popcount_choose;
#endif
-}
-
-#endif /* TRY_POPCNT_FAST */
-
+#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 +146,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 +191,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 +221,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))
{
diff --git a/src/port/pg_popcnt_choose.c b/src/port/pg_popcnt_choose.c
new file mode 100644
index 0000000000..e170e16ff9
--- /dev/null
+++ b/src/port/pg_popcnt_choose.c
@@ -0,0 +1,168 @@
+/*-------------------------------------------------------------------------
+ *
+ * pg_popcnt_x86_64_choose.c
+ * Miscellaneous functions for bit-wise operations.
+ *
+ * Copyright (c) 2024, PostgreSQL Global Development Group
+ *
+ * IDENTIFICATION
+ * src/port/pg_popcnt_x86_64_choose.c
+ *
+ *-------------------------------------------------------------------------
+ */
+#include "c.h"
+
+#include "port/pg_bitutils.h"
+
+#ifdef TRY_POPCNT_FAST
+
+#ifdef HAVE__GET_CPUID
+#include <cpuid.h>
+#endif
+
+#ifdef HAVE__CPUID
+#include <intrin.h>
+#endif
+
+static bool pg_popcount_available(void);
+int pg_popcount32_choose(uint32 word);
+int pg_popcount64_choose(uint64 word);
+uint64 pg_popcount_choose(const char *buf, int bytes);
+
+extern int pg_popcount32_fast(uint32 word);
+extern int pg_popcount64_fast(uint64 word);
+extern int pg_popcount32_slow(uint32 word);
+extern int pg_popcount64_slow(uint64 word);
+extern uint64 pg_popcount512_fast(const char *buf, int bytes);
+extern uint64 pg_popcount_slow(const char *buf, int bytes);
+extern uint64 (*pg_popcount_indirect)(const char *buf, int bytes);
+
+extern int (*pg_popcount32)(uint32 word);
+extern int (*pg_popcount64)(uint64 word);
+
+/*
+ * 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 */
+}
+
+/*
+ * Return true if CPUID indicates that the AVX512_POPCNT instruction is
+ * available. This is similar to the method above; see
+ * https://en.wikipedia.org/wiki/CPUID#EAX=7,_ECX=0:_Extended_Features
+ *
+ * Finally, we make sure the xgetbv result is consistent with the CPUID
+ * results.
+ */
+static bool
+pg_popcount512_available(void)
+{
+ unsigned int exx[4] = {0, 0, 0, 0};
+
+ /* Check for AVX512VPOPCNTDQ and AVX512F */
+#if defined(HAVE__GET_CPUID_COUNT)
+ __get_cpuid_count(7, 0, &exx[0], &exx[1], &exx[2], &exx[3]);
+#elif defined(HAVE__CPUIDEX)
+ __cpuidex(exx, 7, 0);
+#endif
+
+ if ((exx[2] & (0x00004000)) != 0 && (exx[1] & (0x00010000)) != 0)
+ {
+ /*
+ * CPUID succeeded, does the current running OS support the
+ * ZMM registers which are required for AVX512? This check is
+ * required to make sure an old OS on a new CPU is correctly
+ * checked or a VM hypervisor is not excluding AVX512 ZMM
+ * support in the VM; see "5.1.9 Detection of AVX Instructions"
+ * https://www.intel.com/content/www/us/en/content-details/671488/intel-64-and-ia-32-architectures-optimization-reference-manual-volume-1.html
+ */
+ uint64 xcr = 0;
+#ifdef _MSC_VER
+ uint64 highlow = _xgetbv(xcr);
+
+ return (highlow & 0xE0) != 0;
+#else
+ uint32 high;
+ uint32 low;
+
+ __asm__ __volatile__("xgetbv\t\n" : "=a"(low), "=d"(high) : "c"(xcr));
+ return (low & 0xE0) != 0;
+#endif
+ } /* POPCNT 512 */
+ return false;
+}
+
+/*
+ * 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 void set_up_function_pointers()
+{
+ if (pg_popcount512_available())
+ {
+#if defined(_MSC_VER)
+ pg_popcount_indirect = pg_popcount512_fast;
+#else
+ pg_popcount = pg_popcount512_fast;
+#endif
+ }
+ else
+ {
+#if defined(_MSC_VER)
+ pg_popcount_indirect = pg_popcount_slow;
+#else
+ pg_popcount = pg_popcount_slow;
+#endif
+ }
+ 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;
+ }
+}
+
+int pg_popcount32_choose(uint32 word)
+{
+ set_up_function_pointers();
+ return pg_popcount32(word);
+}
+
+int
+pg_popcount64_choose(uint64 word)
+{
+ set_up_function_pointers();
+ return pg_popcount64(word);
+}
+
+uint64
+pg_popcount_choose(const char *buf, int bytes)
+{
+ set_up_function_pointers();
+#if defined(_MSC_VER)
+ return pg_popcount_indirect(buf, bytes);
+#else
+ return pg_popcount(buf, bytes);
+#endif
+}
+
+#endif /* TRY_POPCNT_FAST */
diff --git a/src/port/pg_popcnt_x86_64_accel.c b/src/port/pg_popcnt_x86_64_accel.c
new file mode 100644
index 0000000000..aef32c1174
--- /dev/null
+++ b/src/port/pg_popcnt_x86_64_accel.c
@@ -0,0 +1,93 @@
+/*-------------------------------------------------------------------------
+ *
+ * pg_popcnt_x86_64_accel.c
+ * Miscellaneous functions for bit-wise operations.
+ *
+ * Copyright (c) 2024, PostgreSQL Global Development Group
+ *
+ * IDENTIFICATION
+ * src/port/pg_popcnt_x86_64_accel.c
+ *
+ *-------------------------------------------------------------------------
+ */
+#include "c.h"
+
+#if defined(HAVE__IMMINTRIN)
+#include <immintrin.h>
+#endif
+
+#include "port/pg_bitutils.h"
+
+#ifdef TRY_POPCNT_FAST
+extern const uint8 pg_number_of_ones[256];
+extern uint64 pg_popcount_slow(const char *buf, int bytes);
+uint64 pg_popcount512_fast(const char *buf, int bytes);
+int pg_popcount32_fast(uint32 word);
+int pg_popcount64_fast(uint64 word);
+
+/*
+ * pg_popcount32_fast
+ * Return the number of 1 bits set in word
+ */
+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
+ */
+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
+}
+
+/*
+ * Use AVX-512 Intrinsics for supported Intel CPUs or fall back the the software
+ * loop in pg_bunutils.c and use the best 32 or 64 bit fast methods. If no fast
+ * methods are used this will fall back to __builtin_* or pure software.
+ */
+uint64
+pg_popcount512_fast(const char *buf, int bytes)
+{
+#if defined(HAVE__IMMINTRIN) && HAVE__AVX512_POPCNT == 1
+ uint64 popcnt = 0;
+ __m512i accumulator = _mm512_setzero_si512();
+
+ while (bytes >= 64)
+ {
+ const __m512i v = _mm512_loadu_si512((const __m512i *)buf);
+ const __m512i p = _mm512_popcnt_epi64(v);
+
+ accumulator = _mm512_add_epi64(accumulator, p);
+ bytes -= 64;
+ buf += 64;
+ }
+
+ popcnt = _mm512_reduce_add_epi64(accumulator);
+
+ /* Process any remaining bytes */
+ while (bytes--)
+ popcnt += pg_number_of_ones[(unsigned char)*buf++];
+ return popcnt;
+#else
+ return pg_popcount_slow(buf, bytes);
+#endif /* USE_AVX512_CODE */
+}
+#endif /* TRY_POPCNT_FAST */
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: Popcount optimization using AVX512
@ 2024-03-04 22:21 Nathan Bossart <[email protected]>
parent: Amonson, Paul D <[email protected]>
1 sibling, 1 reply; 26+ messages in thread
From: Nathan Bossart @ 2024-03-04 22:21 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]>
(Please don't top-post on the Postgres lists.)
On Mon, Mar 04, 2024 at 09:39:36PM +0000, Amonson, Paul D wrote:
> First, apologies on the patch. Find re-attached updated version.
Thanks for the new version of the patch.
>> -#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].
>
> I am not sure what the ask is here? I made changes to the configure.ac
> and ran autoconf2.69 to get builds to succeed. Do you have a separate
> feedback here?
These LARGE_OFF_T changes are unrelated to the patch at hand and should be
removed. This likely means that you are using a patched autoconf that is
making these extra changes.
> As for the refactoring, this was done to satisfy previous review feedback
> about applying the AVX512 CFLAGS to the entire pg_bitutils.c file. Mainly
> to avoid segfault due to the AVX512 flags. If its ok, I would prefer to
> make a single commit as the change is pretty small and straight forward.
Okay. The only reason I suggest this is to ease review. For example, if
there is some required refactoring that doesn't involve any functionality
changes, it can be advantageous to get that part reviewed and committed
first so that reviewers can better focus on the code for the new feature.
But, of course, that isn't necessary and/or isn't possible in all cases.
> I am not sure I understand the comment about the SIZE_VOID_P checks.
> Aren't they necessary to choose which functions to call based on 32 or 64
> bit architectures?
Yes. My comment was that the patch appeared to make unnecessary changes to
this code. Perhaps I am misunderstanding something here.
> Would this change qualify for Workflow A as described in [0] and can be
> picked up by a committer, given it has been reviewed by multiple
> committers so far? The scope of the change is pretty contained as well.
I think so. I would still encourage you to create an entry for this so
that it is automatically tested via cfbot [0].
[0] http://commitfest.cputube.org/
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 26+ messages in thread
* RE: Popcount optimization using AVX512
@ 2024-03-05 16:31 Amonson, Paul D <[email protected]>
parent: Nathan Bossart <[email protected]>
0 siblings, 1 reply; 26+ messages in thread
From: Amonson, Paul D @ 2024-03-05 16:31 UTC (permalink / raw)
To: Nathan Bossart <[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]>
Hi,
I am not sure what "top-post" means but I am not doing anything different but using "reply to all" in Outlook. Please enlighten me. 😊
This is the new patch with the hand edit to remove the offending lines from the patch file. I did a basic test to make the patch would apply and build. It succeeded.
Thanks,
Paul
-----Original Message-----
From: Nathan Bossart <[email protected]>
Sent: Monday, March 4, 2024 2:21 PM
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]
Subject: Re: Popcount optimization using AVX512
(Please don't top-post on the Postgres lists.)
On Mon, Mar 04, 2024 at 09:39:36PM +0000, Amonson, Paul D wrote:
> First, apologies on the patch. Find re-attached updated version.
Thanks for the new version of the patch.
>> -#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].
>
> I am not sure what the ask is here? I made changes to the
> configure.ac and ran autoconf2.69 to get builds to succeed. Do you
> have a separate feedback here?
These LARGE_OFF_T changes are unrelated to the patch at hand and should be removed. This likely means that you are using a patched autoconf that is making these extra changes.
> As for the refactoring, this was done to satisfy previous review
> feedback about applying the AVX512 CFLAGS to the entire pg_bitutils.c
> file. Mainly to avoid segfault due to the AVX512 flags. If its ok, I
> would prefer to make a single commit as the change is pretty small and straight forward.
Okay. The only reason I suggest this is to ease review. For example, if there is some required refactoring that doesn't involve any functionality changes, it can be advantageous to get that part reviewed and committed first so that reviewers can better focus on the code for the new feature.
But, of course, that isn't necessary and/or isn't possible in all cases.
> I am not sure I understand the comment about the SIZE_VOID_P checks.
> Aren't they necessary to choose which functions to call based on 32 or
> 64 bit architectures?
Yes. My comment was that the patch appeared to make unnecessary changes to this code. Perhaps I am misunderstanding something here.
> Would this change qualify for Workflow A as described in [0] and can
> be picked up by a committer, given it has been reviewed by multiple
> committers so far? The scope of the change is pretty contained as well.
I think so. I would still encourage you to create an entry for this so that it is automatically tested via cfbot [0].
[0] http://commitfest.cputube.org/
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
Attachments:
[application/octet-stream] v6-0001-Add-support-for-AVX512-implemented-POPCNT.patch (29.3K, ../../BL1PR11MB53047C28DDBA6A2E77B9420ADC222@BL1PR11MB5304.namprd11.prod.outlook.com/2-v6-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
@@ -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..bd297d9fa9 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,43 @@ 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'
+ test_flags = ['-mavx512vpopcntdq', '-mavx512f']
+ if host_system == 'windows'
+ test_flags = ['/arch:AVX512']
+ endif
+ 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 + test_flags)
+ have_avx512_popcnt = true
+ cdata.set('USE_AVX512_POPCNT_WITH_RUNTIME_CHECK', 1)
+ cdata.set('HAVE__AVX512_POPCNT', 1)
+ cflags_avx512_popcnt = test_flags
+ 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..caca78d805 100644
--- a/src/include/port/pg_bitutils.h
+++ b/src/include/port/pg_bitutils.h
@@ -303,16 +303,23 @@ pg_ceil_log2_64(uint64 num)
extern int (*pg_popcount32) (uint32 word);
extern int (*pg_popcount64) (uint64 word);
+#if defined(_MSC_VER)
+extern uint64 pg_popcount(const char *buf, int bytes);
+extern uint64 (*pg_popcount_indirect)(const char *buf, int bytes);
+#else
+extern uint64 (*pg_popcount)(const char *buf, int bytes);
+#endif
+
#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..942e396141 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,35 @@ 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)
+#if defined(_MSC_VER)
+uint64 (*pg_popcount_indirect)(const char *buf, int bytes) = pg_popcount_choose;
+uint64 pg_popcount(const char *buf, int bytes)
{
-#ifdef _MSC_VER
- return __popcnt(word);
-#else
- uint32 res;
-
-__asm__ __volatile__(" popcntl %1,%0\n":"=q"(res):"rm"(word):"cc");
- return (int) res;
-#endif
+ return pg_popcount_indirect(buf, bytes);
}
-
-/*
- * 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;
+uint64 (*pg_popcount) (const char *buf, int bytes) = pg_popcount_choose;
#endif
-}
-
-#endif /* TRY_POPCNT_FAST */
-
+#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 +146,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 +191,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 +221,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))
{
diff --git a/src/port/pg_popcnt_choose.c b/src/port/pg_popcnt_choose.c
new file mode 100644
index 0000000000..e170e16ff9
--- /dev/null
+++ b/src/port/pg_popcnt_choose.c
@@ -0,0 +1,168 @@
+/*-------------------------------------------------------------------------
+ *
+ * pg_popcnt_x86_64_choose.c
+ * Miscellaneous functions for bit-wise operations.
+ *
+ * Copyright (c) 2024, PostgreSQL Global Development Group
+ *
+ * IDENTIFICATION
+ * src/port/pg_popcnt_x86_64_choose.c
+ *
+ *-------------------------------------------------------------------------
+ */
+#include "c.h"
+
+#include "port/pg_bitutils.h"
+
+#ifdef TRY_POPCNT_FAST
+
+#ifdef HAVE__GET_CPUID
+#include <cpuid.h>
+#endif
+
+#ifdef HAVE__CPUID
+#include <intrin.h>
+#endif
+
+static bool pg_popcount_available(void);
+int pg_popcount32_choose(uint32 word);
+int pg_popcount64_choose(uint64 word);
+uint64 pg_popcount_choose(const char *buf, int bytes);
+
+extern int pg_popcount32_fast(uint32 word);
+extern int pg_popcount64_fast(uint64 word);
+extern int pg_popcount32_slow(uint32 word);
+extern int pg_popcount64_slow(uint64 word);
+extern uint64 pg_popcount512_fast(const char *buf, int bytes);
+extern uint64 pg_popcount_slow(const char *buf, int bytes);
+extern uint64 (*pg_popcount_indirect)(const char *buf, int bytes);
+
+extern int (*pg_popcount32)(uint32 word);
+extern int (*pg_popcount64)(uint64 word);
+
+/*
+ * 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 */
+}
+
+/*
+ * Return true if CPUID indicates that the AVX512_POPCNT instruction is
+ * available. This is similar to the method above; see
+ * https://en.wikipedia.org/wiki/CPUID#EAX=7,_ECX=0:_Extended_Features
+ *
+ * Finally, we make sure the xgetbv result is consistent with the CPUID
+ * results.
+ */
+static bool
+pg_popcount512_available(void)
+{
+ unsigned int exx[4] = {0, 0, 0, 0};
+
+ /* Check for AVX512VPOPCNTDQ and AVX512F */
+#if defined(HAVE__GET_CPUID_COUNT)
+ __get_cpuid_count(7, 0, &exx[0], &exx[1], &exx[2], &exx[3]);
+#elif defined(HAVE__CPUIDEX)
+ __cpuidex(exx, 7, 0);
+#endif
+
+ if ((exx[2] & (0x00004000)) != 0 && (exx[1] & (0x00010000)) != 0)
+ {
+ /*
+ * CPUID succeeded, does the current running OS support the
+ * ZMM registers which are required for AVX512? This check is
+ * required to make sure an old OS on a new CPU is correctly
+ * checked or a VM hypervisor is not excluding AVX512 ZMM
+ * support in the VM; see "5.1.9 Detection of AVX Instructions"
+ * https://www.intel.com/content/www/us/en/content-details/671488/intel-64-and-ia-32-architectures-optimization-reference-manual-volume-1.html
+ */
+ uint64 xcr = 0;
+#ifdef _MSC_VER
+ uint64 highlow = _xgetbv(xcr);
+
+ return (highlow & 0xE0) != 0;
+#else
+ uint32 high;
+ uint32 low;
+
+ __asm__ __volatile__("xgetbv\t\n" : "=a"(low), "=d"(high) : "c"(xcr));
+ return (low & 0xE0) != 0;
+#endif
+ } /* POPCNT 512 */
+ return false;
+}
+
+/*
+ * 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 void set_up_function_pointers()
+{
+ if (pg_popcount512_available())
+ {
+#if defined(_MSC_VER)
+ pg_popcount_indirect = pg_popcount512_fast;
+#else
+ pg_popcount = pg_popcount512_fast;
+#endif
+ }
+ else
+ {
+#if defined(_MSC_VER)
+ pg_popcount_indirect = pg_popcount_slow;
+#else
+ pg_popcount = pg_popcount_slow;
+#endif
+ }
+ 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;
+ }
+}
+
+int pg_popcount32_choose(uint32 word)
+{
+ set_up_function_pointers();
+ return pg_popcount32(word);
+}
+
+int
+pg_popcount64_choose(uint64 word)
+{
+ set_up_function_pointers();
+ return pg_popcount64(word);
+}
+
+uint64
+pg_popcount_choose(const char *buf, int bytes)
+{
+ set_up_function_pointers();
+#if defined(_MSC_VER)
+ return pg_popcount_indirect(buf, bytes);
+#else
+ return pg_popcount(buf, bytes);
+#endif
+}
+
+#endif /* TRY_POPCNT_FAST */
diff --git a/src/port/pg_popcnt_x86_64_accel.c b/src/port/pg_popcnt_x86_64_accel.c
new file mode 100644
index 0000000000..aef32c1174
--- /dev/null
+++ b/src/port/pg_popcnt_x86_64_accel.c
@@ -0,0 +1,93 @@
+/*-------------------------------------------------------------------------
+ *
+ * pg_popcnt_x86_64_accel.c
+ * Miscellaneous functions for bit-wise operations.
+ *
+ * Copyright (c) 2024, PostgreSQL Global Development Group
+ *
+ * IDENTIFICATION
+ * src/port/pg_popcnt_x86_64_accel.c
+ *
+ *-------------------------------------------------------------------------
+ */
+#include "c.h"
+
+#if defined(HAVE__IMMINTRIN)
+#include <immintrin.h>
+#endif
+
+#include "port/pg_bitutils.h"
+
+#ifdef TRY_POPCNT_FAST
+extern const uint8 pg_number_of_ones[256];
+extern uint64 pg_popcount_slow(const char *buf, int bytes);
+uint64 pg_popcount512_fast(const char *buf, int bytes);
+int pg_popcount32_fast(uint32 word);
+int pg_popcount64_fast(uint64 word);
+
+/*
+ * pg_popcount32_fast
+ * Return the number of 1 bits set in word
+ */
+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
+ */
+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
+}
+
+/*
+ * Use AVX-512 Intrinsics for supported Intel CPUs or fall back the the software
+ * loop in pg_bunutils.c and use the best 32 or 64 bit fast methods. If no fast
+ * methods are used this will fall back to __builtin_* or pure software.
+ */
+uint64
+pg_popcount512_fast(const char *buf, int bytes)
+{
+#if defined(HAVE__IMMINTRIN) && HAVE__AVX512_POPCNT == 1
+ uint64 popcnt = 0;
+ __m512i accumulator = _mm512_setzero_si512();
+
+ while (bytes >= 64)
+ {
+ const __m512i v = _mm512_loadu_si512((const __m512i *)buf);
+ const __m512i p = _mm512_popcnt_epi64(v);
+
+ accumulator = _mm512_add_epi64(accumulator, p);
+ bytes -= 64;
+ buf += 64;
+ }
+
+ popcnt = _mm512_reduce_add_epi64(accumulator);
+
+ /* Process any remaining bytes */
+ while (bytes--)
+ popcnt += pg_number_of_ones[(unsigned char)*buf++];
+ return popcnt;
+#else
+ return pg_popcount_slow(buf, bytes);
+#endif /* USE_AVX512_CODE */
+}
+#endif /* TRY_POPCNT_FAST */
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: Popcount optimization using AVX512
@ 2024-03-05 16:37 Nathan Bossart <[email protected]>
parent: Amonson, Paul D <[email protected]>
0 siblings, 1 reply; 26+ messages in thread
From: Nathan Bossart @ 2024-03-05 16:37 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]>
On Tue, Mar 05, 2024 at 04:31:15PM +0000, Amonson, Paul D wrote:
> I am not sure what "top-post" means but I am not doing anything different
> but using "reply to all" in Outlook. Please enlighten me. 😊
The following link provides some more information:
https://wiki.postgresql.org/wiki/Mailing_Lists#Email_etiquette_mechanics
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 26+ messages in thread
* RE: Popcount optimization using AVX512
@ 2024-03-05 16:52 Amonson, Paul D <[email protected]>
parent: Nathan Bossart <[email protected]>
0 siblings, 2 replies; 26+ messages in thread
From: Amonson, Paul D @ 2024-03-05 16:52 UTC (permalink / raw)
To: Nathan Bossart <[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]>
-----Original Message-----
>From: Nathan Bossart <[email protected]>
>Sent: Tuesday, March 5, 2024 8:38 AM
>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]
>Subject: Re: Popcount optimization using AVX512
>
>On Tue, Mar 05, 2024 at 04:31:15PM +0000, Amonson, Paul D wrote:
>> I am not sure what "top-post" means but I am not doing anything
>> different but using "reply to all" in Outlook. Please enlighten me. 😊
>
>The following link provides some more information:
>
> https://wiki.postgresql.org/wiki/Mailing_Lists#Email_etiquette_mechanics
>
>--
>Nathan Bossart
>Amazon Web Services: https://aws.amazon.com
Ahhhh.....Ok... guess it's time to thank Microsoft then. ;) Noted I will try to do the "reduced" bottom-posting. I might slip up occasionally because it's an Intel habit. Is there a way to make Outlook do the leading ">" in a reply for the previous message?
BTW: Created the commit-fest submission.
Paul
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: Popcount optimization using AVX512
@ 2024-03-05 22:18 Bruce Momjian <[email protected]>
parent: Amonson, Paul D <[email protected]>
1 sibling, 0 replies; 26+ messages in thread
From: Bruce Momjian @ 2024-03-05 22:18 UTC (permalink / raw)
To: Amonson, Paul D <[email protected]>; +Cc: Nathan Bossart <[email protected]>; 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]>
On Tue, Mar 5, 2024 at 04:52:23PM +0000, Amonson, Paul D wrote:
> -----Original Message-----
> >From: Nathan Bossart <[email protected]>
> >Sent: Tuesday, March 5, 2024 8:38 AM
> >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]
> >Subject: Re: Popcount optimization using AVX512
> >
> >On Tue, Mar 05, 2024 at 04:31:15PM +0000, Amonson, Paul D wrote:
> >> I am not sure what "top-post" means but I am not doing anything
> >> different but using "reply to all" in Outlook. Please enlighten me. 😊
> >
> >The following link provides some more information:
> >
> > https://wiki.postgresql.org/wiki/Mailing_Lists#Email_etiquette_mechanics
> >
> >--
> >Nathan Bossart
> >Amazon Web Services: https://aws.amazon.com
>
> Ahhhh.....Ok... guess it's time to thank Microsoft then. ;) Noted I will try to do the "reduced" bottom-posting. I might slip up occasionally because it's an Intel habit. Is there a way to make Outlook do the leading ">" in a reply for the previous message?
Here is a blog post about how complex email posting can be:
https://momjian.us/main/blogs/pgblog/2023.html#September_8_2023
--
Bruce Momjian <[email protected]> https://momjian.us
EDB https://enterprisedb.com
Only you can decide what is important to you.
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: Popcount optimization using AVX512
@ 2024-03-07 17:33 Nathan Bossart <[email protected]>
parent: Amonson, Paul D <[email protected]>
1 sibling, 1 reply; 26+ messages in thread
From: Nathan Bossart @ 2024-03-07 17:33 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]>
On Tue, Mar 05, 2024 at 04:52:23PM +0000, Amonson, Paul D wrote:
> Noted I will try to do the "reduced" bottom-posting. I might slip up
> occasionally because it's an Intel habit.
No worries.
> Is there a way to make Outlook do the leading ">" in a reply for the
> previous message?
I do not know, sorry. I personally use mutt for the lists.
> BTW: Created the commit-fest submission.
Thanks. I intend to provide a more detailed review shortly, as I am aiming
to get this one committed for v17.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: Popcount optimization using AVX512
@ 2024-03-07 17:53 Alvaro Herrera <[email protected]>
parent: Amonson, Paul D <[email protected]>
1 sibling, 1 reply; 26+ messages in thread
From: Alvaro Herrera @ 2024-03-07 17:53 UTC (permalink / raw)
To: Amonson, Paul D <[email protected]>; +Cc: Nathan Bossart <[email protected]>; Andres Freund <[email protected]>; Shankaran, Akash <[email protected]>; Noah Misch <[email protected]>; Tom Lane <[email protected]>; Matthias van de Meent <[email protected]>; [email protected] <[email protected]>
On 2024-Mar-04, Amonson, Paul D wrote:
> > -#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].
>
> I am not sure what the ask is here? I made changes to the
> configure.ac and ran autoconf2.69 to get builds to succeed. Do you
> have a separate feedback here?
So what happens here is that autoconf-2.69 as shipped by Debian contains
some patches on top of the one released by GNU. We use the latter, so
if you run Debian's, then the generated configure script will contain
the differences coming from Debian's version.
Really, I don't think this is very important as a review point, because
if the configure.ac file is changed in the patch, it's best for the
committer to run autoconf on their own, using a pristine GNU autoconf;
the configure file in the submitted patch is not relevant, only
configure.ac matters.
What committers do (or should do) is keep an install of autoconf-2.69
straight from GNU.
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: Popcount optimization using AVX512
@ 2024-03-07 17:59 Nathan Bossart <[email protected]>
parent: Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 26+ messages in thread
From: Nathan Bossart @ 2024-03-07 17:59 UTC (permalink / raw)
To: Alvaro Herrera <[email protected]>; +Cc: Amonson, Paul D <[email protected]>; Andres Freund <[email protected]>; Shankaran, Akash <[email protected]>; Noah Misch <[email protected]>; Tom Lane <[email protected]>; Matthias van de Meent <[email protected]>; [email protected] <[email protected]>
On Thu, Mar 07, 2024 at 06:53:12PM +0100, Alvaro Herrera wrote:
> Really, I don't think this is very important as a review point, because
> if the configure.ac file is changed in the patch, it's best for the
> committer to run autoconf on their own, using a pristine GNU autoconf;
> the configure file in the submitted patch is not relevant, only
> configure.ac matters.
Agreed. I didn't intend for this to be a major review point, and I
apologize for the extra noise.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: Popcount optimization using AVX512
@ 2024-03-07 21:36 Nathan Bossart <[email protected]>
parent: Nathan Bossart <[email protected]>
0 siblings, 1 reply; 26+ messages in thread
From: Nathan Bossart @ 2024-03-07 21:36 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]>
As promised...
> +# 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)
I'm curious why we need both -mavx512vpopcntdq and -mavx512f. On my
machine, -mavx512vpopcntdq alone is enough to pass this test, so if there
are other instructions required that need -mavx512f, then we might need to
expand the test.
> 13 files changed, 657 insertions(+), 119 deletions(-)
I still think it's worth breaking this change into at least 2 patches. In
particular, I think there's an opportunity to do the refactoring into
pg_popcnt_choose.c and pg_popcnt_x86_64_accel.c prior to adding the AVX512
stuff. These changes are likely straightforward, and getting them out of
the way early would make it easier to focus on the more interesting
changes. IMHO there are a lot of moving parts in this patch.
> +#undef HAVE__GET_CPUID_COUNT
> +
> +/* Define to 1 if you have immintrin. */
> +#undef HAVE__IMMINTRIN
Is this missing HAVE__CPUIDEX?
> 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))
> {
> @@ -311,7 +224,7 @@ pg_popcount(const char *buf, int bytes)
>
> 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))
> {
Apologies for harping on this, but I'm still not seeing the need for these
SIZEOF_VOID_P changes. While it's unlikely that this makes any practical
difference, I see no reason to more strictly check SIZEOF_VOID_P here.
> + /* Process any remaining bytes */
> + while (bytes--)
> + popcnt += pg_number_of_ones[(unsigned char) *buf++];
> + return popcnt;
> +#else
> + return pg_popcount_slow(buf, bytes);
> +#endif /* USE_AVX512_CODE */
nitpick: Could we call pg_popcount_slow() in a common section for these
"remaining bytes?"
> +#if defined(_MSC_VER)
> + pg_popcount_indirect = pg_popcount512_fast;
> +#else
> + pg_popcount = pg_popcount512_fast;
> +#endif
These _MSC_VER sections are interesting. I'm assuming this is the
workaround for the MSVC linking issue you mentioned above. I haven't
looked too closely, but I wonder if the CRC32C code (see
src/include/port/pg_crc32c.h) is doing something different to avoid this
issue.
Upthread, Alvaro suggested a benchmark [0] that might be useful. I scanned
through this thread and didn't see any recent benchmark results for the
latest form of the patch. I think it's worth verifying that we are still
seeing the expected improvements.
[0] https://postgr.es/m/202402071953.5c4z7t6kl7ts%40alvherre.pgsql
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 26+ messages in thread
* RE: Popcount optimization using AVX512
@ 2024-03-11 21:59 Amonson, Paul D <[email protected]>
parent: Nathan Bossart <[email protected]>
0 siblings, 1 reply; 26+ messages in thread
From: Amonson, Paul D @ 2024-03-11 21:59 UTC (permalink / raw)
To: Nathan Bossart <[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]>
> -----Original Message-----
> From: Nathan Bossart <[email protected]>
> Sent: Thursday, March 7, 2024 1:36 PM
> Subject: Re: Popcount optimization using AVX512
I will be splitting the request into 2 patches. I am attaching the first patch (refactoring only) and I updated the commitfest entry to match this patch. I have a question however:
Do I need to wait for the refactor patch to be merged before I post the AVX portion of this feature in this thread?
> > + PGAC_AVX512_POPCNT_INTRINSICS([-mavx512vpopcntdq -mavx512f])
>
> I'm curious why we need both -mavx512vpopcntdq and -mavx512f. On my
> machine, -mavx512vpopcntdq alone is enough to pass this test, so if there are
> other instructions required that need -mavx512f, then we might need to
> expand the test.
First, nice catch on the required flags to build! When I changed my algorithm, dependence on the -mavx512f flag was no longer needed, In the second patch (AVX specific) I will fix this.
> I still think it's worth breaking this change into at least 2 patches. In particular,
> I think there's an opportunity to do the refactoring into pg_popcnt_choose.c
> and pg_popcnt_x86_64_accel.c prior to adding the AVX512 stuff. These
> changes are likely straightforward, and getting them out of the way early
> would make it easier to focus on the more interesting changes. IMHO there
> are a lot of moving parts in this patch.
As stated above I am doing this in 2 patches. :)
> > +#undef HAVE__GET_CPUID_COUNT
> > +
> > +/* Define to 1 if you have immintrin. */ #undef HAVE__IMMINTRIN
>
> Is this missing HAVE__CPUIDEX?
Yes I missed it, I will include in the second patch (AVX specific) of the 2 patches.
> > 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))
> > {
> > @@ -311,7 +224,7 @@ pg_popcount(const char *buf, int bytes)
> >
> > 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))
> > {
>
> Apologies for harping on this, but I'm still not seeing the need for these
> SIZEOF_VOID_P changes. While it's unlikely that this makes any practical
> difference, I see no reason to more strictly check SIZEOF_VOID_P here.
I got rid of the second occurrence as I agree it is not needed but unless you see something I don't how to know which function to call between a 32-bit and 64-bit architecture? Maybe I am missing something obvious? What exactly do you suggest here? I am happy to always call either pg_popcount32() or pg_popcount64() with the understanding that it may not be optimal, but I do need to know which to use.
> > + /* Process any remaining bytes */
> > + while (bytes--)
> > + popcnt += pg_number_of_ones[(unsigned char) *buf++];
> > + return popcnt;
> > +#else
> > + return pg_popcount_slow(buf, bytes);
> > +#endif /* USE_AVX512_CODE */
>
> nitpick: Could we call pg_popcount_slow() in a common section for these
> "remaining bytes?"
Agreed, will fix in the second patch as well.
> > +#if defined(_MSC_VER)
> > + pg_popcount_indirect = pg_popcount512_fast; #else
> > + pg_popcount = pg_popcount512_fast; #endif
> These _MSC_VER sections are interesting. I'm assuming this is the
> workaround for the MSVC linking issue you mentioned above. I haven't
> looked too closely, but I wonder if the CRC32C code (see
> src/include/port/pg_crc32c.h) is doing something different to avoid this issue.
Using the latest master branch, I see what the needed changes are, I will implement using PGDLLIMPORT macro in the second patch.
> Upthread, Alvaro suggested a benchmark [0] that might be useful. I scanned
> through this thread and didn't see any recent benchmark results for the latest
> form of the patch. I think it's worth verifying that we are still seeing the
> expected improvements.
I will get new benchmarks using the same process I used before (from Akash) so I get apples to apples. These are pending completion of the second patch which is still in progress.
Just a reminder, I asked questions above about 1) multi-part dependent patches and, 2) What specifically to do about the SIZE_VOID_P checks. :)
Thanks,
Paul
Attachments:
[application/octet-stream] v7-0001-Refactor-POPCNT.patch (7.7K, ../../BL1PR11MB5304165E7A81E0107E70B069DC242@BL1PR11MB5304.namprd11.prod.outlook.com/2-v7-0001-Refactor-POPCNT.patch)
download | inline diff:
diff --git a/src/port/Makefile b/src/port/Makefile
index dcc8737e68..12c56b0ba7 100644
--- a/src/port/Makefile
+++ b/src/port/Makefile
@@ -44,6 +44,8 @@ OBJS = \
noblock.o \
path.o \
pg_bitutils.o \
+ pg_popcnt_choose.o \
+ pg_popcnt_x86_64_accel.o \
pg_strong_random.o \
pgcheckdir.o \
pgmkdirp.o \
diff --git a/src/port/meson.build b/src/port/meson.build
index 92b593e6ef..ed8828c739 100644
--- a/src/port/meson.build
+++ b/src/port/meson.build
@@ -7,6 +7,8 @@ pgport_sources = [
'noblock.c',
'path.c',
'pg_bitutils.c',
+ 'pg_popcnt_choose.c',
+ 'pg_popcnt_x86_64_accel.c',
'pg_strong_random.c',
'pgcheckdir.c',
'pgmkdirp.c',
diff --git a/src/port/pg_bitutils.c b/src/port/pg_bitutils.c
index 640a89561a..5ace0a5b13 100644
--- a/src/port/pg_bitutils.c
+++ b/src/port/pg_bitutils.c
@@ -12,13 +12,6 @@
*/
#include "c.h"
-#ifdef HAVE__GET_CPUID
-#include <cpuid.h>
-#endif
-#ifdef HAVE__CPUID
-#include <intrin.h>
-#endif
-
#include "port/pg_bitutils.h"
@@ -103,123 +96,14 @@ 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);
-
-#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);
-
-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 */
-
+extern int pg_popcount32_slow(uint32 word);
+extern int pg_popcount64_slow(uint64 word);
/*
* 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 +125,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
diff --git a/src/port/pg_popcnt_choose.c b/src/port/pg_popcnt_choose.c
new file mode 100644
index 0000000000..70c70f5742
--- /dev/null
+++ b/src/port/pg_popcnt_choose.c
@@ -0,0 +1,97 @@
+/*-------------------------------------------------------------------------
+ *
+ * pg_popcnt_choose.c
+ * For FAST operations, these methods do runtime checks and set the
+ * appropriate function pointers.
+ *
+ * Copyright (c) 2019-2024, PostgreSQL Global Development Group
+ *
+ * IDENTIFICATION
+ * src/port/pg_popcnt_choose.c
+ *
+ *-------------------------------------------------------------------------
+ */
+#include "c.h"
+
+#ifdef HAVE__GET_CPUID
+#include <cpuid.h>
+#endif
+#ifdef HAVE__CPUID
+#include <intrin.h>
+#endif
+
+#include "port/pg_bitutils.h"
+
+
+/* In pg_bitutils.c file */
+extern int pg_popcount32_slow(uint32 word);
+extern int pg_popcount64_slow(uint64 word);
+
+#ifdef TRY_POPCNT_FAST
+static bool pg_popcount_available(void);
+static int pg_popcount32_choose(uint32 word);
+static int pg_popcount64_choose(uint64 word);
+
+/* In pg_popcnt_*_accel source file. */
+extern int pg_popcount32_fast(uint32 word);
+extern int pg_popcount64_fast(uint64 word);
+
+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 void setup_function_pointers()
+{
+ 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;
+ }
+}
+
+static int
+pg_popcount32_choose(uint32 word)
+{
+ setup_function_pointers();
+ return pg_popcount32(word);
+}
+
+static int
+pg_popcount64_choose(uint64 word)
+{
+ setup_function_pointers();
+ return pg_popcount64(word);
+}
+#endif /* TRY_POPCNT_FAST */
diff --git a/src/port/pg_popcnt_x86_64_accel.c b/src/port/pg_popcnt_x86_64_accel.c
new file mode 100644
index 0000000000..6e36e90e16
--- /dev/null
+++ b/src/port/pg_popcnt_x86_64_accel.c
@@ -0,0 +1,55 @@
+/*-------------------------------------------------------------------------
+ *
+ * pg_popcnt_x86_64_accel.c
+ * Fast POPCNT methods for x86_64.
+ *
+ * Copyright (c) 2019-2024, PostgreSQL Global Development Group
+ *
+ * IDENTIFICATION
+ * src/port/pg_popcnt_x86_64_accel.c
+ *
+ *-------------------------------------------------------------------------
+ */
+#include "c.h"
+
+#include "port/pg_bitutils.h"
+
+extern int pg_popcount32_fast(uint32 word);
+extern int pg_popcount64_fast(uint64 word);
+
+#ifdef TRY_POPCNT_FAST
+/*
+ * pg_popcount32_fast
+ * Return the number of 1 bits set in word
+ */
+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
+ */
+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 */
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: Popcount optimization using AVX512
@ 2024-03-12 01:34 Nathan Bossart <[email protected]>
parent: Amonson, Paul D <[email protected]>
0 siblings, 2 replies; 26+ messages in thread
From: Nathan Bossart @ 2024-03-12 01:34 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]>
On Mon, Mar 11, 2024 at 09:59:53PM +0000, Amonson, Paul D wrote:
> I will be splitting the request into 2 patches. I am attaching the first
> patch (refactoring only) and I updated the commitfest entry to match this
> patch. I have a question however:
> Do I need to wait for the refactor patch to be merged before I post the
> AVX portion of this feature in this thread?
Thanks. There's no need to wait to post the AVX portion. I recommend
using "git format-patch" to construct the patch set for the lists.
>> Apologies for harping on this, but I'm still not seeing the need for these
>> SIZEOF_VOID_P changes. While it's unlikely that this makes any practical
>> difference, I see no reason to more strictly check SIZEOF_VOID_P here.
>
> I got rid of the second occurrence as I agree it is not needed but unless
> you see something I don't how to know which function to call between a
> 32-bit and 64-bit architecture? Maybe I am missing something obvious?
> What exactly do you suggest here? I am happy to always call either
> pg_popcount32() or pg_popcount64() with the understanding that it may not
> be optimal, but I do need to know which to use.
I'm recommending that we don't change any of the code in the pg_popcount()
function (which is renamed to pg_popcount_slow() in your v6 patch). If
pointers are 8 or more bytes, we'll try to process the buffer in 64-bit
chunks. Else, we'll try to process it in 32-bit chunks. Any remaining
bytes will be processed one-by-one.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: Popcount optimization using AVX512
@ 2024-03-13 16:39 Nathan Bossart <[email protected]>
parent: Nathan Bossart <[email protected]>
1 sibling, 1 reply; 26+ messages in thread
From: Nathan Bossart @ 2024-03-13 16:39 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]>
A couple of thoughts on v7-0001:
+extern int pg_popcount32_slow(uint32 word);
+extern int pg_popcount64_slow(uint64 word);
+/* In pg_popcnt_*_accel source file. */
+extern int pg_popcount32_fast(uint32 word);
+extern int pg_popcount64_fast(uint64 word);
Can these prototypes be moved to a header file (maybe pg_bitutils.h)? It
looks like these are defined twice in the patch, and while I'm not positive
that it's against project policy to declare extern function prototypes in
.c files, it appears to be pretty rare.
+ 'pg_popcnt_choose.c',
+ 'pg_popcnt_x86_64_accel.c',
I think we want these to be architecture-specific, i.e., only built for
x86_64 if the compiler knows how to use the relevant instructions. There
is a good chance that we'll want to add similar support for other systems.
The CRC32C files are probably a good reference point for how to do this.
+#ifdef TRY_POPCNT_FAST
IIUC this macro can be set if either 1) the popcntq test in the
autoconf/meson scripts passes or 2) we're building with MSVC on x86_64. I
wonder if it would be better to move the MSVC/x86_64 check to the
autoconf/meson scripts so that we could avoid surrounding large portions of
the popcount code with this macro. This might even be a necessary step
towards building these files in an architecture-specific fashion.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 26+ messages in thread
* RE: Popcount optimization using AVX512
@ 2024-03-13 17:52 Amonson, Paul D <[email protected]>
parent: Nathan Bossart <[email protected]>
0 siblings, 1 reply; 26+ messages in thread
From: Amonson, Paul D @ 2024-03-13 17:52 UTC (permalink / raw)
To: Nathan Bossart <[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]>
> -----Original Message-----
> From: Nathan Bossart <[email protected]>
> Sent: Wednesday, March 13, 2024 9:39 AM
> To: Amonson, Paul D <[email protected]>
> +extern int pg_popcount32_slow(uint32 word); extern int
> +pg_popcount64_slow(uint64 word);
>
> +/* In pg_popcnt_*_accel source file. */ extern int
> +pg_popcount32_fast(uint32 word); extern int pg_popcount64_fast(uint64
> +word);
>
> Can these prototypes be moved to a header file (maybe pg_bitutils.h)? It
> looks like these are defined twice in the patch, and while I'm not positive that
> it's against project policy to declare extern function prototypes in .c files, it
> appears to be pretty rare.
Originally, I intentionally did not put these in the header file as I want them to be private, but they are not defined in this .c file hence extern. Now I realize the "extern" part is not needed to accomplish my goal. Will fix by removing the "extern" keyword.
> + 'pg_popcnt_choose.c',
> + 'pg_popcnt_x86_64_accel.c',
>
> I think we want these to be architecture-specific, i.e., only built for
> x86_64 if the compiler knows how to use the relevant instructions. There is a
> good chance that we'll want to add similar support for other systems.
> The CRC32C files are probably a good reference point for how to do this.
I will look at this for the 'pg_popcnt_x86_64_accel.c' file but the 'pg_popcnt_choose.c' file is intended to be for any platform that may need accelerators including a possible future ARM accelerator.
> +#ifdef TRY_POPCNT_FAST
>
> IIUC this macro can be set if either 1) the popcntq test in the autoconf/meson
> scripts passes or 2) we're building with MSVC on x86_64. I wonder if it would
> be better to move the MSVC/x86_64 check to the autoconf/meson scripts so
> that we could avoid surrounding large portions of the popcount code with this
> macro. This might even be a necessary step towards building these files in an
> architecture-specific fashion.
I see the point here; however, this will take some time to get right especially since I don't have a Windows box to do compiles on. Should I attempt to do this in this patch?
Thanks,
Paul
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: Popcount optimization using AVX512
@ 2024-03-13 18:38 Nathan Bossart <[email protected]>
parent: Amonson, Paul D <[email protected]>
0 siblings, 0 replies; 26+ messages in thread
From: Nathan Bossart @ 2024-03-13 18:38 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]>
On Wed, Mar 13, 2024 at 05:52:14PM +0000, Amonson, Paul D wrote:
>> I think we want these to be architecture-specific, i.e., only built for
>> x86_64 if the compiler knows how to use the relevant instructions. There is a
>> good chance that we'll want to add similar support for other systems.
>> The CRC32C files are probably a good reference point for how to do this.
>
> I will look at this for the 'pg_popcnt_x86_64_accel.c' file but the
> 'pg_popcnt_choose.c' file is intended to be for any platform that may
> need accelerators including a possible future ARM accelerator.
I worry that using the same file for *_choose.c for all architectures would
become rather #ifdef heavy. Since we are already separating out this code
into new files, IMO we might as well try to avoid too many #ifdefs, too.
But this is admittedly less important right now because there's almost no
chance of any new architecture support here for v17.
>> +#ifdef TRY_POPCNT_FAST
>>
>> IIUC this macro can be set if either 1) the popcntq test in the autoconf/meson
>> scripts passes or 2) we're building with MSVC on x86_64. I wonder if it would
>> be better to move the MSVC/x86_64 check to the autoconf/meson scripts so
>> that we could avoid surrounding large portions of the popcount code with this
>> macro. This might even be a necessary step towards building these files in an
>> architecture-specific fashion.
>
> I see the point here; however, this will take some time to get right
> especially since I don't have a Windows box to do compiles on. Should I
> attempt to do this in this patch?
This might also be less important given the absence of any imminent new
architecture support in this area. I'm okay with it, given we are just
maintaining the status quo.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 26+ messages in thread
* RE: Popcount optimization using AVX512
@ 2024-03-14 19:50 Amonson, Paul D <[email protected]>
parent: Nathan Bossart <[email protected]>
1 sibling, 1 reply; 26+ messages in thread
From: Amonson, Paul D @ 2024-03-14 19:50 UTC (permalink / raw)
To: Nathan Bossart <[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]>
> -----Original Message-----
> From: Nathan Bossart <[email protected]>
> Sent: Monday, March 11, 2024 6:35 PM
> To: Amonson, Paul D <[email protected]>
> Thanks. There's no need to wait to post the AVX portion. I recommend using
> "git format-patch" to construct the patch set for the lists.
After exploring git format-patch command I think I understand what you need. Attached.
> > What exactly do you suggest here? I am happy to always call either
> > pg_popcount32() or pg_popcount64() with the understanding that it may
> > not be optimal, but I do need to know which to use.
>
> I'm recommending that we don't change any of the code in the pg_popcount()
> function (which is renamed to pg_popcount_slow() in your v6 patch). If
> pointers are 8 or more bytes, we'll try to process the buffer in 64-bit chunks.
> Else, we'll try to process it in 32-bit chunks. Any remaining bytes will be
> processed one-by-one.
Ok, we are on the same page now. :) It is already fixed that way in the refactor patch #1.
As for new performance numbers: I just ran a full suite like I did earlier in the process. My latest results an equivalent to a pgbench scale factor 10 DB with the target column having varying column widths and appropriate random data are 1.2% improvement with a 2.2% Margin of Error at a 98% confidence level. Still seeing improvement and no regressions.
As stated in the previous separate chain I updated the code removing the extra "extern" keywords.
Thanks,
Paul
Attachments:
[application/octet-stream] v8-0001-Refactor-POPCNT-code-refactored-for-future-accelerat.patch (8.4K, ../../BL1PR11MB53042473B7B53B96E43648A7DC292@BL1PR11MB5304.namprd11.prod.outlook.com/2-v8-0001-Refactor-POPCNT-code-refactored-for-future-accelerat.patch)
download | inline diff:
From 57256412ce11b006ad383fc689a0fd28716632e0 Mon Sep 17 00:00:00 2001
From: Paul Amonson <[email protected]>
Date: Wed, 13 Mar 2024 11:56:44 -0700
Subject: [PATCH 1/2] [Refactor] POPCNT code refactored for future acceleration
of the function.
Signed-off-by: Paul Amonson <[email protected]>
---
src/port/Makefile | 2 +
src/port/meson.build | 2 +
src/port/pg_bitutils.c | 124 +-----------------------------
src/port/pg_popcnt_choose.c | 97 +++++++++++++++++++++++
src/port/pg_popcnt_x86_64_accel.c | 55 +++++++++++++
5 files changed, 160 insertions(+), 120 deletions(-)
create mode 100644 src/port/pg_popcnt_choose.c
create mode 100644 src/port/pg_popcnt_x86_64_accel.c
diff --git a/src/port/Makefile b/src/port/Makefile
index dcc8737e68..12c56b0ba7 100644
--- a/src/port/Makefile
+++ b/src/port/Makefile
@@ -44,6 +44,8 @@ OBJS = \
noblock.o \
path.o \
pg_bitutils.o \
+ pg_popcnt_choose.o \
+ pg_popcnt_x86_64_accel.o \
pg_strong_random.o \
pgcheckdir.o \
pgmkdirp.o \
diff --git a/src/port/meson.build b/src/port/meson.build
index 92b593e6ef..ed8828c739 100644
--- a/src/port/meson.build
+++ b/src/port/meson.build
@@ -7,6 +7,8 @@ pgport_sources = [
'noblock.c',
'path.c',
'pg_bitutils.c',
+ 'pg_popcnt_choose.c',
+ 'pg_popcnt_x86_64_accel.c',
'pg_strong_random.c',
'pgcheckdir.c',
'pgmkdirp.c',
diff --git a/src/port/pg_bitutils.c b/src/port/pg_bitutils.c
index 640a89561a..d8b045d0a4 100644
--- a/src/port/pg_bitutils.c
+++ b/src/port/pg_bitutils.c
@@ -12,13 +12,6 @@
*/
#include "c.h"
-#ifdef HAVE__GET_CPUID
-#include <cpuid.h>
-#endif
-#ifdef HAVE__CPUID
-#include <intrin.h>
-#endif
-
#include "port/pg_bitutils.h"
@@ -103,123 +96,14 @@ 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);
-
-#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);
-
-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 */
-
+int pg_popcount32_slow(uint32 word);
+int pg_popcount64_slow(uint64 word);
/*
* 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 +125,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
diff --git a/src/port/pg_popcnt_choose.c b/src/port/pg_popcnt_choose.c
new file mode 100644
index 0000000000..89fcf2609c
--- /dev/null
+++ b/src/port/pg_popcnt_choose.c
@@ -0,0 +1,97 @@
+/*-------------------------------------------------------------------------
+ *
+ * pg_popcnt_choose.c
+ * For FAST operations, these methods do runtime checks and set the
+ * appropriate function pointers.
+ *
+ * Copyright (c) 2019-2024, PostgreSQL Global Development Group
+ *
+ * IDENTIFICATION
+ * src/port/pg_popcnt_choose.c
+ *
+ *-------------------------------------------------------------------------
+ */
+#include "c.h"
+
+#ifdef HAVE__GET_CPUID
+#include <cpuid.h>
+#endif
+#ifdef HAVE__CPUID
+#include <intrin.h>
+#endif
+
+#include "port/pg_bitutils.h"
+
+
+/* In pg_bitutils.c file */
+int pg_popcount32_slow(uint32 word);
+int pg_popcount64_slow(uint64 word);
+
+#ifdef TRY_POPCNT_FAST
+static bool pg_popcount_available(void);
+static int pg_popcount32_choose(uint32 word);
+static int pg_popcount64_choose(uint64 word);
+
+/* In pg_popcnt_*_accel source file. */
+int pg_popcount32_fast(uint32 word);
+int pg_popcount64_fast(uint64 word);
+
+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 void setup_function_pointers()
+{
+ 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;
+ }
+}
+
+static int
+pg_popcount32_choose(uint32 word)
+{
+ setup_function_pointers();
+ return pg_popcount32(word);
+}
+
+static int
+pg_popcount64_choose(uint64 word)
+{
+ setup_function_pointers();
+ return pg_popcount64(word);
+}
+#endif /* TRY_POPCNT_FAST */
diff --git a/src/port/pg_popcnt_x86_64_accel.c b/src/port/pg_popcnt_x86_64_accel.c
new file mode 100644
index 0000000000..2e9b2ee774
--- /dev/null
+++ b/src/port/pg_popcnt_x86_64_accel.c
@@ -0,0 +1,55 @@
+/*-------------------------------------------------------------------------
+ *
+ * pg_popcnt_x86_64_accel.c
+ * Fast POPCNT methods for x86_64.
+ *
+ * Copyright (c) 2019-2024, PostgreSQL Global Development Group
+ *
+ * IDENTIFICATION
+ * src/port/pg_popcnt_x86_64_accel.c
+ *
+ *-------------------------------------------------------------------------
+ */
+#include "c.h"
+
+#include "port/pg_bitutils.h"
+
+int pg_popcount32_fast(uint32 word);
+int pg_popcount64_fast(uint64 word);
+
+#ifdef TRY_POPCNT_FAST
+/*
+ * pg_popcount32_fast
+ * Return the number of 1 bits set in word
+ */
+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
+ */
+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 */
--
2.34.1
[application/octet-stream] v8-0002-Feat-Add-AVX-512-POPCNT-support-initial-checkin.patch (24.5K, ../../BL1PR11MB53042473B7B53B96E43648A7DC292@BL1PR11MB5304.namprd11.prod.outlook.com/3-v8-0002-Feat-Add-AVX-512-POPCNT-support-initial-checkin.patch)
download | inline diff:
From 67a887d8fe8be389c709fdc87b196cd2ad5d2bf7 Mon Sep 17 00:00:00 2001
From: Paul Amonson <[email protected]>
Date: Wed, 13 Mar 2024 12:49:57 -0700
Subject: [PATCH 2/2] [Feat] Add AVX-512 POPCNT support initial checkin.
Signed-off-by: Paul Amonson <[email protected]>
---
config/c-compiler.m4 | 37 ++++++
configure | 205 ++++++++++++++++++++++++++++++
configure.ac | 44 +++++++
meson.build | 72 +++++++++++
src/Makefile.global.in | 1 +
src/include/pg_config.h.in | 12 ++
src/include/port/pg_bitutils.h | 10 +-
src/makefiles/meson.build | 1 +
src/port/Makefile | 5 +
src/port/meson.build | 6 +-
src/port/pg_bitutils.c | 9 +-
src/port/pg_popcnt_choose.c | 67 +++++++++-
src/port/pg_popcnt_x86_64_accel.c | 36 +++++-
13 files changed, 494 insertions(+), 11 deletions(-)
diff --git a/config/c-compiler.m4 b/config/c-compiler.m4
index 3268a780bb..54f7415e5a 100644
--- a/config/c-compiler.m4
+++ b/config/c-compiler.m4
@@ -694,3 +694,40 @@ 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>
+#include <stdlib.h>
+#include <string.h>],
+ [const uint64_t *buf = malloc((size_t)64);
+ uint64_t popcnt = 0;
+ __m512i accumulator = _mm512_setzero_si512();
+ const __m512i v = _mm512_loadu_si512((const __m512i *)buf);
+ const __m512i p = _mm512_popcnt_epi64(v);
+ memset(buf, 0, 64);
+ accumulator = _mm512_add_epi64(accumulator, p);
+ popcnt = _mm512_reduce_add_epi64(accumulator);
+ /* return computed value, to prevent the above being optimized away */
+ return popcnt == 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
\ No newline at end of file
diff --git a/configure b/configure
index 36feeafbb2..0fbfc7c78f 100755
--- a/configure
+++ b/configure
@@ -647,6 +647,7 @@ MSGFMT_FLAGS
MSGFMT
PG_CRC32C_OBJS
CFLAGS_CRC
+CFLAGS_AVX512_POPCNT
LIBOBJS
OPENSSL
ZSTD
@@ -17404,6 +17405,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 :
@@ -17438,6 +17474,175 @@ $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>
+#include <stdlib.h>
+#include <string.h>
+int
+main ()
+{
+const uint64_t *buf = malloc((size_t)64);
+ uint64_t popcnt = 0;
+ __m512i accumulator = _mm512_setzero_si512();
+ const __m512i v = _mm512_loadu_si512((const __m512i *)buf);
+ const __m512i p = _mm512_popcnt_epi64(v);
+ memset(buf, 0, 64);
+ accumulator = _mm512_add_epi64(accumulator, p);
+ popcnt = _mm512_reduce_add_epi64(accumulator);
+ /* return computed value, to prevent the above being optimized away */
+ return popcnt == 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" >&5
+$as_echo_n "checking for _mm512_popcnt_epi64 with CFLAGS=-mavx512vpopcntdq... " >&6; }
+if ${pgac_cv_avx512_popcnt_intrinsics__mavx512vpopcntdq+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ pgac_save_CFLAGS=$CFLAGS
+CFLAGS="$pgac_save_CFLAGS -mavx512vpopcntdq"
+cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h. */
+#include <immintrin.h>
+#include <stdint.h>
+#include <stdlib.h>
+#include <string.h>
+int
+main ()
+{
+const uint64_t *buf = malloc((size_t)64);
+ uint64_t popcnt = 0;
+ __m512i accumulator = _mm512_setzero_si512();
+ const __m512i v = _mm512_loadu_si512((const __m512i *)buf);
+ const __m512i p = _mm512_popcnt_epi64(v);
+ memset(buf, 0, 64);
+ accumulator = _mm512_add_epi64(accumulator, p);
+ popcnt = _mm512_reduce_add_epi64(accumulator);
+ /* return computed value, to prevent the above being optimized away */
+ return popcnt == 0;
+ ;
+ return 0;
+}
+_ACEOF
+if ac_fn_c_try_link "$LINENO"; then :
+ pgac_cv_avx512_popcnt_intrinsics__mavx512vpopcntdq=yes
+else
+ pgac_cv_avx512_popcnt_intrinsics__mavx512vpopcntdq=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" >&5
+$as_echo "$pgac_cv_avx512_popcnt_intrinsics__mavx512vpopcntdq" >&6; }
+if test x"$pgac_cv_avx512_popcnt_intrinsics__mavx512vpopcntdq" = x"yes"; then
+ CFLAGS_AVX512_POPCNT="-mavx512vpopcntdq"
+ pgac_avx512_popcnt_intrinsics=yes
+fi
+
+
+$as_echo "#define HAVE__AVX512_POPCNT 1" >>confdefs.h
+
+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 57f734879e..3c741d457d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2052,6 +2052,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};
@@ -2063,6 +2075,38 @@ 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])
+ AC_DEFINE(HAVE__AVX512_POPCNT, 1, [Define to 1 if you have cpu
+ support for AVX512 POPCNT.])
+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 85788f9dd8..39480b4251 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
@@ -2147,6 +2178,47 @@ elif host_cpu == 'ppc' or host_cpu == 'ppc64'
endif
+###############################################################
+# AVX 512 POPCNT Intrinsic check
+###############################################################
+have_avx512_popcnt = false
+cflags_avx512_popcnt = []
+if host_cpu == 'x86_64'
+ test_flags = ['-mavx512vpopcntdq']
+ if host_system == 'windows'
+ test_flags = ['/arch:AVX512']
+ endif
+ prog = '''
+ #include <immintrin.h>
+ #include <stdint.h>
+ #include <stdlib.h>
+ #include <string.h>
+ void main(void)
+ {
+ const uint64_t *buf = malloc((size_t)64);
+ uint64_t popcnt = 0;
+ __m512i accumulator = _mm512_setzero_si512();
+ const __m512i v = _mm512_loadu_si512((const __m512i *)buf);
+ const __m512i p = _mm512_popcnt_epi64(v);
+ memset(buf, 0, 64);
+ accumulator = _mm512_add_epi64(accumulator, p);
+ popcnt = _mm512_reduce_add_epi64(accumulator);
+ /* return computed value, to prevent the above being optimized away */
+ return popcnt == 0;
+ }'''
+ if cc.links(prog, name: '_mm512_* methods with -mavx512vpopcntdq flag.',
+ args: test_c_args + test_flags)
+ have_avx512_popcnt = true
+ cdata.set('USE_AVX512_POPCNT_WITH_RUNTIME_CHECK', 1)
+ cdata.set('HAVE__AVX512_POPCNT', 1)
+ cflags_avx512_popcnt = test_flags
+ 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
+
###############################################################
# Library / OS tests
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 591e1ca3df..e4d56dee79 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -555,6 +555,18 @@
/* Define to 1 if you have __cpuid. */
#undef HAVE__CPUID
+/* Define to 1 if you have __get_cpuid_count. */
+#undef HAVE__GET_CPUID_COUNT
+
+/* Define to 1 if you have __get_cpuidex. */
+#undef HAVE__GET_CPUIDEX
+
+/* Define to 1 if you have immintrin. */
+#undef HAVE__IMMINTRIN
+
+/* Define to 1 if you have AVX512. */
+#undef HAVE__AVX512_POPCNT
+
/* Define to 1 if you have __get_cpuid. */
#undef HAVE__GET_CPUID
diff --git a/src/include/port/pg_bitutils.h b/src/include/port/pg_bitutils.h
index 46bf4f0103..cc42ce49c9 100644
--- a/src/include/port/pg_bitutils.h
+++ b/src/include/port/pg_bitutils.h
@@ -300,19 +300,19 @@ pg_ceil_log2_64(uint64 num)
#ifdef TRY_POPCNT_FAST
/* Attempt to use the POPCNT instruction, but perform a runtime check first */
-extern PGDLLIMPORT int (*pg_popcount32) (uint32 word);
-extern PGDLLIMPORT int (*pg_popcount64) (uint64 word);
-
+extern PGDLLIMPORT int (*pg_popcount32) (uint32 word);
+extern PGDLLIMPORT int (*pg_popcount64) (uint64 word);
+extern PGDLLIMPORT 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 12c56b0ba7..0b76926301 100644
--- a/src/port/Makefile
+++ b/src/port/Makefile
@@ -89,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 ed8828c739..d7930672cb 100644
--- a/src/port/meson.build
+++ b/src/port/meson.build
@@ -8,7 +8,6 @@ pgport_sources = [
'path.c',
'pg_bitutils.c',
'pg_popcnt_choose.c',
- 'pg_popcnt_x86_64_accel.c',
'pg_strong_random.c',
'pgcheckdir.c',
'pgmkdirp.c',
@@ -86,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'],
@@ -100,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 d8b045d0a4..22c51c1679 100644
--- a/src/port/pg_bitutils.c
+++ b/src/port/pg_bitutils.c
@@ -98,6 +98,7 @@ const uint8 pg_number_of_ones[256] = {
int pg_popcount32_slow(uint32 word);
int pg_popcount64_slow(uint64 word);
+uint64 pg_popcount_slow(const char *buf, int bytes);
/*
* pg_popcount32_slow
@@ -170,6 +171,12 @@ 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 */
/*
@@ -177,7 +184,7 @@ pg_popcount64(uint64 word)
* Returns the number of 1-bits in buf
*/
uint64
-pg_popcount(const char *buf, int bytes)
+pg_popcount_slow(const char *buf, int bytes)
{
uint64 popcnt = 0;
diff --git a/src/port/pg_popcnt_choose.c b/src/port/pg_popcnt_choose.c
index 89fcf2609c..ac1344415d 100644
--- a/src/port/pg_popcnt_choose.c
+++ b/src/port/pg_popcnt_choose.c
@@ -26,18 +26,23 @@
/* In pg_bitutils.c file */
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 bool pg_popcount512_available(void);
static int pg_popcount32_choose(uint32 word);
static int pg_popcount64_choose(uint64 word);
+static uint64 pg_popcount_choose(const char *buf, int bytes);
/* In pg_popcnt_*_accel source file. */
int pg_popcount32_fast(uint32 word);
int pg_popcount64_fast(uint64 word);
+uint64 pg_popcount_fast(const char *buf, int bytes);
int (*pg_popcount32) (uint32 word) = pg_popcount32_choose;
int (*pg_popcount64) (uint64 word) = pg_popcount64_choose;
+uint64 (*pg_popcount) (const char *buf, int bytes) = pg_popcount_choose;
#endif /* TRY_POPCNT_FAST */
#ifdef TRY_POPCNT_FAST
@@ -61,6 +66,52 @@ pg_popcount_available(void)
return (exx[2] & (1 << 23)) != 0; /* POPCNT */
}
+/*
+ * Return true if CPUID indicates that the AVX512_POPCNT instruction is
+ * available. This is similar to the method above; see
+ * https://en.wikipedia.org/wiki/CPUID#EAX=7,_ECX=0:_Extended_Features
+ *
+ * Finally, we make sure the xgetbv result is consistent with the CPUID
+ * results.
+ */
+static bool
+pg_popcount512_available(void)
+{
+ unsigned int exx[4] = {0, 0, 0, 0};
+
+ /* Check for AVX512VPOPCNTDQ and AVX512F */
+#if defined(HAVE__GET_CPUID_COUNT)
+ __get_cpuid_count(7, 0, &exx[0], &exx[1], &exx[2], &exx[3]);
+#elif defined(HAVE__CPUIDEX)
+ __cpuidex(exx, 7, 0);
+#endif
+
+ if ((exx[2] & (0x00004000)) != 0 && (exx[1] & (0x00010000)) != 0)
+ {
+ /*
+ * CPUID succeeded, does the current running OS support the
+ * ZMM registers which are required for AVX512? This check is
+ * required to make sure an old OS on a new CPU is correctly
+ * checked or a VM hypervisor is not excluding AVX512 ZMM
+ * support in the VM; see "5.1.9 Detection of AVX Instructions"
+ * https://www.intel.com/content/www/us/en/content-details/671488/intel-64-and-ia-32-architectures-optimization-reference-manual-volume-1.html
+ */
+ uint64 xcr = 0;
+#ifdef _MSC_VER
+ uint64 highlow = _xgetbv(xcr);
+
+ return (highlow & 0xE0) != 0;
+#else
+ uint32 high;
+ uint32 low;
+
+ __asm__ __volatile__("xgetbv\t\n" : "=a"(low), "=d"(high) : "c"(xcr));
+ return (low & 0xE0) != 0;
+#endif
+ } /* POPCNT 512 */
+ return false;
+}
+
/*
* These functions get called on the first call to pg_popcount32 etc.
* They detect whether we can use the asm implementations, and replace
@@ -69,15 +120,23 @@ pg_popcount_available(void)
*/
static void setup_function_pointers()
{
- if (pg_popcount_available())
+ if (pg_popcount512_available())
{
pg_popcount32 = pg_popcount32_fast;
pg_popcount64 = pg_popcount64_fast;
+ pg_popcount = pg_popcount_fast;
+ }
+ else if (pg_popcount_available())
+ {
+ pg_popcount32 = pg_popcount32_fast;
+ pg_popcount64 = pg_popcount64_fast;
+ pg_popcount = pg_popcount_slow;
}
else
{
pg_popcount32 = pg_popcount32_slow;
pg_popcount64 = pg_popcount64_slow;
+ pg_popcount = pg_popcount_slow;
}
}
@@ -94,4 +153,10 @@ pg_popcount64_choose(uint64 word)
setup_function_pointers();
return pg_popcount64(word);
}
+
+static uint64
+pg_popcount_choose(const char* buf, int bytes) {
+ setup_function_pointers();
+ return pg_popcount(buf, bytes);
+}
#endif /* TRY_POPCNT_FAST */
diff --git a/src/port/pg_popcnt_x86_64_accel.c b/src/port/pg_popcnt_x86_64_accel.c
index 2e9b2ee774..ecc07afd37 100644
--- a/src/port/pg_popcnt_x86_64_accel.c
+++ b/src/port/pg_popcnt_x86_64_accel.c
@@ -14,8 +14,14 @@
#include "port/pg_bitutils.h"
+#if defined(HAVE__IMMINTRIN)
+#include <immintrin.h>
+#endif
+
int pg_popcount32_fast(uint32 word);
int pg_popcount64_fast(uint64 word);
+uint64 pg_popcount_fast(const char *buf, int bytes);
+uint64 pg_popcount_slow(const char *buf, int bytes);
#ifdef TRY_POPCNT_FAST
/*
@@ -52,4 +58,32 @@ __asm__ __volatile__(" popcntq %1,%0\n":"=q"(res):"rm"(word):"cc");
#endif
}
-#endif /* TRY_POPCNT_FAST */
+/*
+ * Use AVX-512 Intrinsics for supported Intel CPUs or fall back the the software
+ * loop in pg_bunutils.c and use the best 32 or 64 bit fast methods. If no fast
+ * methods are used this will fall back to __builtin_* or pure software.
+ */
+uint64
+pg_popcount_fast(const char *buf, int bytes)
+{
+ uint64 popcnt = 0;
+ #if defined(HAVE__IMMINTRIN) && HAVE__AVX512_POPCNT == 1
+ __m512i accumulator = _mm512_setzero_si512();
+
+ while (bytes >= 64)
+ {
+ const __m512i v = _mm512_loadu_si512((const __m512i *)buf);
+ const __m512i p = _mm512_popcnt_epi64(v);
+
+ accumulator = _mm512_add_epi64(accumulator, p);
+ bytes -= 64;
+ buf += 64;
+ }
+
+ popcnt = _mm512_reduce_add_epi64(accumulator);
+#endif /* defined(HAVE__IMMINTRIN) && HAVE__AVX512_POPCNT == 1 */
+
+ /* Process any remaining bytes */
+ return popcnt + pg_popcount_slow(buf, bytes);
+}
+#endif /* TRY_POPCNT_FAST */
--
2.34.1
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: Popcount optimization using AVX512
@ 2024-03-15 15:06 Nathan Bossart <[email protected]>
parent: Amonson, Paul D <[email protected]>
0 siblings, 2 replies; 26+ messages in thread
From: Nathan Bossart @ 2024-03-15 15:06 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]>
On Thu, Mar 14, 2024 at 07:50:46PM +0000, Amonson, Paul D wrote:
> As for new performance numbers: I just ran a full suite like I did
> earlier in the process. My latest results an equivalent to a pgbench
> scale factor 10 DB with the target column having varying column widths
> and appropriate random data are 1.2% improvement with a 2.2% Margin of
> Error at a 98% confidence level. Still seeing improvement and no
> regressions.
Which test suite did you run? Those numbers seem potentially
indistinguishable from noise, which probably isn't great for such a large
patch set.
I ran John Naylor's test_popcount module [0] with the following command on
an i7-1195G7:
time psql postgres -c 'select drive_popcount(10000000, 1024)'
Without your patches, this seems to take somewhere around 8.8 seconds.
With your patches, it takes 0.6 seconds. (I re-compiled and re-ran the
tests a couple of times because I had a difficult time believing the amount
of improvement.)
[0] https://postgr.es/m/CAFBsxsE7otwnfA36Ly44zZO%2Bb7AEWHRFANxR1h1kxveEV%3DghLQ%40mail.gmail.com
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 26+ messages in thread
* RE: Popcount optimization using AVX512
@ 2024-03-15 15:31 Amonson, Paul D <[email protected]>
parent: Nathan Bossart <[email protected]>
1 sibling, 1 reply; 26+ messages in thread
From: Amonson, Paul D @ 2024-03-15 15:31 UTC (permalink / raw)
To: Nathan Bossart <[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]>
> -----Original Message-----
> From: Nathan Bossart <[email protected]>
> Sent: Friday, March 15, 2024 8:06 AM
> To: Amonson, Paul D <[email protected]>
> Cc: Andres Freund <[email protected]>; Alvaro Herrera <[email protected]
> ip.org>; Shankaran, Akash <[email protected]>; Noah Misch
> <[email protected]>; Tom Lane <[email protected]>; Matthias van de
> Meent <[email protected]>; pgsql-
> [email protected]
> Subject: Re: Popcount optimization using AVX512
>
> Which test suite did you run? Those numbers seem potentially
> indistinguishable from noise, which probably isn't great for such a large patch
> set.
I ran...
psql -c "select bitcount(column) from table;"
...in a loop with "column" widths of 84, 4096, 8192, and 16384 containing random data. There DB has 1 million rows. In the loop before calling the select I have code to clear all system caches. If I omit the code to clear system caches the margin of error remains the same but the improvement percent changes from 1.2% to 14.6% (much less I/O when cached data is available).
> I ran John Naylor's test_popcount module [0] with the following command on
> an i7-1195G7:
>
> time psql postgres -c 'select drive_popcount(10000000, 1024)'
>
> Without your patches, this seems to take somewhere around 8.8 seconds.
> With your patches, it takes 0.6 seconds. (I re-compiled and re-ran the tests a
> couple of times because I had a difficult time believing the amount of
> improvement.)
When I tested the code outside postgres in a micro benchmark I got 200-300% improvements. Your results are interesting, as it implies more than 300% improvement. Let me do some research on the benchmark you referenced. However, in all cases it seems that there is no regression so should we move forward on merging while I run some more local tests?
Thanks,
Paul
^ permalink raw reply [nested|flat] 26+ messages in thread
* RE: Popcount optimization using AVX512
@ 2024-03-15 17:43 Amonson, Paul D <[email protected]>
parent: Amonson, Paul D <[email protected]>
0 siblings, 0 replies; 26+ messages in thread
From: Amonson, Paul D @ 2024-03-15 17:43 UTC (permalink / raw)
To: Amonson, Paul D <[email protected]>; Nathan Bossart <[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]>
> -----Original Message-----
> From: Amonson, Paul D <[email protected]>
> Sent: Friday, March 15, 2024 8:31 AM
> To: Nathan Bossart <[email protected]>
...
> When I tested the code outside postgres in a micro benchmark I got 200-
> 300% improvements. Your results are interesting, as it implies more than
> 300% improvement. Let me do some research on the benchmark you
> referenced. However, in all cases it seems that there is no regression so should
> we move forward on merging while I run some more local tests?
When running quick test with small buffers (1 to 32K) I see up to about a 740% improvement. This was using my stand-alone micro benchmark outside of PG. My original 200-300% numbers were averaged including sizes up to 512MB which seems to not run as well on large buffers. I will try the referenced micro benchmark on Monday. None of my benchmark testing used the command line "time" command. For Postgres is set "\timing" before the run and for the stand-alone benchmark is took timestamps in the code. In all cases I used -O2 for optimization.
Thanks,
Paul
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: Popcount optimization using AVX512
@ 2024-03-17 20:56 David Rowley <[email protected]>
parent: Nathan Bossart <[email protected]>
1 sibling, 1 reply; 26+ messages in thread
From: David Rowley @ 2024-03-17 20:56 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: Amonson, Paul D <[email protected]>; 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]>
On Sat, 16 Mar 2024 at 04:06, Nathan Bossart <[email protected]> wrote:
> I ran John Naylor's test_popcount module [0] with the following command on
> an i7-1195G7:
>
> time psql postgres -c 'select drive_popcount(10000000, 1024)'
>
> Without your patches, this seems to take somewhere around 8.8 seconds.
> With your patches, it takes 0.6 seconds. (I re-compiled and re-ran the
> tests a couple of times because I had a difficult time believing the amount
> of improvement.)
>
> [0] https://postgr.es/m/CAFBsxsE7otwnfA36Ly44zZO%2Bb7AEWHRFANxR1h1kxveEV%3DghLQ%40mail.gmail.com
I think most of that will come from getting rid of the indirect
function that currently exists in pg_popcount().
Using the attached quick hack, the performance using John's test
module goes from:
-- master
postgres=# select drive_popcount(10000000, 1024);
Time: 9832.845 ms (00:09.833)
Time: 9844.460 ms (00:09.844)
Time: 9858.608 ms (00:09.859)
-- with attached hacky and untested patch
postgres=# select drive_popcount(10000000, 1024);
Time: 2539.029 ms (00:02.539)
Time: 2598.223 ms (00:02.598)
Time: 2611.435 ms (00:02.611)
--- and with the avx512 patch on an AMD 7945HX CPU:
postgres=# select drive_popcount(10000000, 1024);
Time: 564.982 ms
Time: 556.540 ms
Time: 554.032 ms
The following comment seems like it could do with some improvements.
* Use AVX-512 Intrinsics for supported Intel CPUs or fall back the the software
* loop in pg_bunutils.c and use the best 32 or 64 bit fast methods. If no fast
* methods are used this will fall back to __builtin_* or pure software.
There's nothing much specific to Intel here. AMD Zen4 has AVX512.
Plus "pg_bunutils.c" should be "pg_bitutils.c" and "the the"
How about just:
* Use AVX-512 Intrinsics on supported CPUs. Fall back the software loop in
* pg_popcount_slow() when AVX-512 is unavailable.
Maybe it's worth exploring something along the lines of the attached
before doing the AVX512 stuff. It seems like a pretty good speed-up
and will apply for CPUs without AVX512 support.
David
diff --git a/src/port/pg_bitutils.c b/src/port/pg_bitutils.c
index 640a89561a..85e45cee9b 100644
--- a/src/port/pg_bitutils.c
+++ b/src/port/pg_bitutils.c
@@ -305,7 +305,18 @@ pg_popcount(const char *buf, int bytes)
while (bytes >= 8)
{
- popcnt += pg_popcount64(*words++);
+#ifdef _MSC_VER
+ popcnt += __popcnt64(*words++);
+#else
+ uint64 res;
+
+ __asm__ __volatile__(" popcntq %1,%0\n"
+ : "=q"(res)
+ : "rm"(word)
+ : "cc");
+ popcnt += (int) res;
+ words++;
+#endif
bytes -= 8;
}
diff --git a/src/test/modules/meson.build b/src/test/modules/meson.build
index f1d18a1b29..ae880db64c 100644
--- a/src/test/modules/meson.build
+++ b/src/test/modules/meson.build
@@ -26,6 +26,7 @@ subdir('test_misc')
subdir('test_oat_hooks')
subdir('test_parser')
subdir('test_pg_dump')
+subdir('test_popcount')
subdir('test_predtest')
subdir('test_radixtree')
subdir('test_rbtree')
Attachments:
[text/plain] remove_indirect_func_call_in_pg_popcount.patch.txt (928B, ../../CAApHDvrb7MJRB6JuKLDEY2x_LKdFHwVbogpjZBCX547i5+rXOQ@mail.gmail.com/2-remove_indirect_func_call_in_pg_popcount.patch.txt)
download | inline diff:
diff --git a/src/port/pg_bitutils.c b/src/port/pg_bitutils.c
index 640a89561a..85e45cee9b 100644
--- a/src/port/pg_bitutils.c
+++ b/src/port/pg_bitutils.c
@@ -305,7 +305,18 @@ pg_popcount(const char *buf, int bytes)
while (bytes >= 8)
{
- popcnt += pg_popcount64(*words++);
+#ifdef _MSC_VER
+ popcnt += __popcnt64(*words++);
+#else
+ uint64 res;
+
+ __asm__ __volatile__(" popcntq %1,%0\n"
+ : "=q"(res)
+ : "rm"(word)
+ : "cc");
+ popcnt += (int) res;
+ words++;
+#endif
bytes -= 8;
}
diff --git a/src/test/modules/meson.build b/src/test/modules/meson.build
index f1d18a1b29..ae880db64c 100644
--- a/src/test/modules/meson.build
+++ b/src/test/modules/meson.build
@@ -26,6 +26,7 @@ subdir('test_misc')
subdir('test_oat_hooks')
subdir('test_parser')
subdir('test_pg_dump')
+subdir('test_popcount')
subdir('test_predtest')
subdir('test_radixtree')
subdir('test_rbtree')
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: Popcount optimization using AVX512
@ 2024-03-18 15:29 Nathan Bossart <[email protected]>
parent: David Rowley <[email protected]>
0 siblings, 1 reply; 26+ messages in thread
From: Nathan Bossart @ 2024-03-18 15:29 UTC (permalink / raw)
To: David Rowley <[email protected]>; +Cc: Amonson, Paul D <[email protected]>; 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]>
On Mon, Mar 18, 2024 at 09:56:32AM +1300, David Rowley wrote:
> Maybe it's worth exploring something along the lines of the attached
> before doing the AVX512 stuff. It seems like a pretty good speed-up
> and will apply for CPUs without AVX512 support.
+1
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 26+ messages in thread
* RE: Popcount optimization using AVX512
@ 2024-03-18 16:07 Amonson, Paul D <[email protected]>
parent: Nathan Bossart <[email protected]>
0 siblings, 1 reply; 26+ messages in thread
From: Amonson, Paul D @ 2024-03-18 16:07 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; David Rowley <[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]>
Won't I still need the runtime checks? If I compile with a compiler supporting the HW "feature" but run on HW without that feature, I will want to avoid faults due to illegal operations. Won't that also affect performance?
Paul
> -----Original Message-----
> From: Nathan Bossart <[email protected]>
> Sent: Monday, March 18, 2024 8:29 AM
> To: David Rowley <[email protected]>
> Cc: Amonson, Paul D <[email protected]>; 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]
> Subject: Re: Popcount optimization using AVX512
>
> On Mon, Mar 18, 2024 at 09:56:32AM +1300, David Rowley wrote:
> > Maybe it's worth exploring something along the lines of the attached
> > before doing the AVX512 stuff. It seems like a pretty good speed-up
> > and will apply for CPUs without AVX512 support.
>
> +1
>
> --
> Nathan Bossart
> Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: Popcount optimization using AVX512
@ 2024-03-18 16:20 Nathan Bossart <[email protected]>
parent: Amonson, Paul D <[email protected]>
0 siblings, 2 replies; 26+ messages in thread
From: Nathan Bossart @ 2024-03-18 16:20 UTC (permalink / raw)
To: Amonson, Paul D <[email protected]>; +Cc: David Rowley <[email protected]>; 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]>
On Mon, Mar 18, 2024 at 04:07:40PM +0000, Amonson, Paul D wrote:
> Won't I still need the runtime checks? If I compile with a compiler
> supporting the HW "feature" but run on HW without that feature, I will
> want to avoid faults due to illegal operations. Won't that also affect
> performance?
I don't think David was suggesting that we need to remove the runtime
checks for AVX512. IIUC he was pointing out that most of the performance
gain is from removing the function call overhead, which your v8-0002 patch
already does for the proposed AVX512 code. We can apply a similar
optimization for systems without AVX512 by inlining the code for
pg_popcount64() and pg_popcount32().
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 26+ messages in thread
* RE: Popcount optimization using AVX512
@ 2024-03-18 17:28 Amonson, Paul D <[email protected]>
parent: Nathan Bossart <[email protected]>
1 sibling, 0 replies; 26+ messages in thread
From: Amonson, Paul D @ 2024-03-18 17:28 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: David Rowley <[email protected]>; 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]>
> -----Original Message-----
> From: Nathan Bossart <[email protected]>
> Sent: Monday, March 18, 2024 9:20 AM
> ...
> I don't think David was suggesting that we need to remove the runtime checks
> for AVX512. IIUC he was pointing out that most of the performance gain is
> from removing the function call overhead, which your v8-0002 patch already
> does for the proposed AVX512 code. We can apply a similar optimization for
> systems without AVX512 by inlining the code for
> pg_popcount64() and pg_popcount32().
Ok, got you.
Question: I applied the patch for the drive_popcount* functions and rebuilt. The resultant server complains that the function is missing. What is the trick to make this work?
Another Question: Is there a reason "time psql" is used over the Postgres "\timing" command?
Thanks,
Paul
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: Popcount optimization using AVX512
@ 2024-03-18 17:30 Nathan Bossart <[email protected]>
parent: Nathan Bossart <[email protected]>
1 sibling, 0 replies; 26+ messages in thread
From: Nathan Bossart @ 2024-03-18 17:30 UTC (permalink / raw)
To: Amonson, Paul D <[email protected]>; +Cc: David Rowley <[email protected]>; 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]>
On Mon, Mar 18, 2024 at 11:20:18AM -0500, Nathan Bossart wrote:
> I don't think David was suggesting that we need to remove the runtime
> checks for AVX512. IIUC he was pointing out that most of the performance
> gain is from removing the function call overhead, which your v8-0002 patch
> already does for the proposed AVX512 code. We can apply a similar
> optimization for systems without AVX512 by inlining the code for
> pg_popcount64() and pg_popcount32().
Here is a more fleshed-out version of what I believe David is proposing.
On my machine, the gains aren't quite as impressive (~8.8s to ~5.2s for the
test_popcount benchmark). I assume this is because this patch turns
pg_popcount() into a function pointer, which is what the AVX512 patches do,
too. I left out the 32-bit section from pg_popcount_fast(), but I'll admit
that I'm not yet 100% sure that we can assume we're on a 64-bit system
there.
IMHO this work is arguably a prerequisite for the AVX512 work, as turning
pg_popcount() into a function pointer will likely regress performance for
folks on systems without AVX512 otherwise.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
Attachments:
[text/x-diff] 0001-inline-function-calls-in-pg_popcount-when-possible.patch (6.0K, ../../20240318173004.GA361138@nathanxps13/2-0001-inline-function-calls-in-pg_popcount-when-possible.patch)
download | inline diff:
From 1d33c803feb7428f798b13fd643a16c73628f8a9 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Mon, 18 Mar 2024 12:18:15 -0500
Subject: [PATCH 1/1] inline function calls in pg_popcount() when possible
---
src/include/port/pg_bitutils.h | 5 +-
src/port/pg_bitutils.c | 123 +++++++++++++++++++++++++--------
2 files changed, 97 insertions(+), 31 deletions(-)
diff --git a/src/include/port/pg_bitutils.h b/src/include/port/pg_bitutils.h
index 46bf4f0103..53e5239717 100644
--- a/src/include/port/pg_bitutils.h
+++ b/src/include/port/pg_bitutils.h
@@ -302,17 +302,16 @@ pg_ceil_log2_64(uint64 num)
/* Attempt to use the POPCNT instruction, but perform a runtime check first */
extern PGDLLIMPORT int (*pg_popcount32) (uint32 word);
extern PGDLLIMPORT int (*pg_popcount64) (uint64 word);
+extern PGDLLIMPORT 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);
+extern uint64 pg_popcount(const char *buf, int bytes);
#endif /* TRY_POPCNT_FAST */
-/* Count the number of one-bits in a byte array */
-extern uint64 pg_popcount(const char *buf, int bytes);
-
/*
* Rotate the bits of "word" to the right/left by n bits.
*/
diff --git a/src/port/pg_bitutils.c b/src/port/pg_bitutils.c
index 640a89561a..e374e753d7 100644
--- a/src/port/pg_bitutils.c
+++ b/src/port/pg_bitutils.c
@@ -105,16 +105,20 @@ const uint8 pg_number_of_ones[256] = {
static int pg_popcount32_slow(uint32 word);
static int pg_popcount64_slow(uint64 word);
+static 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 uint64 pg_popcount_choose(const char *buf, int bytes);
static int pg_popcount32_fast(uint32 word);
-static int pg_popcount64_fast(uint64 word);
+static inline int pg_popcount64_fast(uint64 word);
+static uint64 pg_popcount_fast(const char *buf, int bytes);
int (*pg_popcount32) (uint32 word) = pg_popcount32_choose;
int (*pg_popcount64) (uint64 word) = pg_popcount64_choose;
+uint64 (*pg_popcount) (const char *buf, int bytes) = pg_popcount_choose;
#endif /* TRY_POPCNT_FAST */
#ifdef TRY_POPCNT_FAST
@@ -151,11 +155,13 @@ pg_popcount32_choose(uint32 word)
{
pg_popcount32 = pg_popcount32_fast;
pg_popcount64 = pg_popcount64_fast;
+ pg_popcount = pg_popcount_fast;
}
else
{
pg_popcount32 = pg_popcount32_slow;
pg_popcount64 = pg_popcount64_slow;
+ pg_popcount = pg_popcount_slow;
}
return pg_popcount32(word);
@@ -168,16 +174,37 @@ pg_popcount64_choose(uint64 word)
{
pg_popcount32 = pg_popcount32_fast;
pg_popcount64 = pg_popcount64_fast;
+ pg_popcount = pg_popcount_fast;
}
else
{
pg_popcount32 = pg_popcount32_slow;
pg_popcount64 = pg_popcount64_slow;
+ pg_popcount = pg_popcount_slow;
}
return pg_popcount64(word);
}
+static uint64
+pg_popcount_choose(const char *buf, int bytes)
+{
+ if (pg_popcount_available())
+ {
+ pg_popcount32 = pg_popcount32_fast;
+ pg_popcount64 = pg_popcount64_fast;
+ pg_popcount = pg_popcount_fast;
+ }
+ else
+ {
+ pg_popcount32 = pg_popcount32_slow;
+ pg_popcount64 = pg_popcount64_slow;
+ pg_popcount = pg_popcount_slow;
+ }
+
+ return pg_popcount(buf, bytes);
+}
+
/*
* pg_popcount32_fast
* Return the number of 1 bits set in word
@@ -199,7 +226,7 @@ __asm__ __volatile__(" popcntl %1,%0\n":"=q"(res):"rm"(word):"cc");
* pg_popcount64_fast
* Return the number of 1 bits set in word
*/
-static int
+static inline int
pg_popcount64_fast(uint64 word)
{
#ifdef _MSC_VER
@@ -212,6 +239,36 @@ __asm__ __volatile__(" popcntq %1,%0\n":"=q"(res):"rm"(word):"cc");
#endif
}
+/*
+ * pg_popcount_fast
+ * Returns the number of 1-bits in buf
+ */
+static uint64
+pg_popcount_fast(const char *buf, int bytes)
+{
+ uint64 popcnt = 0;
+
+ /* Process in 64-bit chunks if the buffer is aligned. */
+ if (buf == (const char *) TYPEALIGN(8, buf))
+ {
+ const uint64 *words = (const uint64 *) buf;
+
+ while (bytes >= 8)
+ {
+ popcnt += pg_popcount64_fast(*words++);
+ bytes -= 8;
+ }
+
+ buf = (const char *) words;
+ }
+
+ /* Process any remaining bytes */
+ while (bytes--)
+ popcnt += pg_number_of_ones[(unsigned char) *buf++];
+
+ return popcnt;
+}
+
#endif /* TRY_POPCNT_FAST */
@@ -265,35 +322,12 @@ pg_popcount64_slow(uint64 word)
#endif /* HAVE__BUILTIN_POPCOUNT */
}
-#ifndef TRY_POPCNT_FAST
-
/*
- * When the POPCNT instruction is not available, there's no point in using
- * function pointers to vary the implementation between the fast and slow
- * method. We instead just make these actual external functions when
- * TRY_POPCNT_FAST is not defined. The compiler should be able to inline
- * the slow versions here.
- */
-int
-pg_popcount32(uint32 word)
-{
- return pg_popcount32_slow(word);
-}
-
-int
-pg_popcount64(uint64 word)
-{
- return pg_popcount64_slow(word);
-}
-
-#endif /* !TRY_POPCNT_FAST */
-
-/*
- * pg_popcount
+ * pg_popcount_slow
* Returns the number of 1-bits in buf
*/
-uint64
-pg_popcount(const char *buf, int bytes)
+static uint64
+pg_popcount_slow(const char *buf, int bytes)
{
uint64 popcnt = 0;
@@ -333,3 +367,36 @@ pg_popcount(const char *buf, int bytes)
return popcnt;
}
+
+#ifndef TRY_POPCNT_FAST
+
+/*
+ * When the POPCNT instruction is not available, there's no point in using
+ * function pointers to vary the implementation between the fast and slow
+ * method. We instead just make these actual external functions when
+ * TRY_POPCNT_FAST is not defined. The compiler should be able to inline
+ * the slow versions here.
+ */
+int
+pg_popcount32(uint32 word)
+{
+ return pg_popcount32_slow(word);
+}
+
+int
+pg_popcount64(uint64 word)
+{
+ return pg_popcount64_slow(word);
+}
+
+/*
+ * pg_popcount
+ * Returns the number of 1-bits in buf
+ */
+uint64
+pg_popcount(const char *buf, int bytes)
+{
+ return pg_popcount_slow(buf, bytes);
+}
+
+#endif /* !TRY_POPCNT_FAST */
--
2.25.1
^ permalink raw reply [nested|flat] 26+ messages in thread
end of thread, other threads:[~2024-03-18 17:30 UTC | newest]
Thread overview: 26+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-08-09 07:56 [PATCH v4 5/7] Row pattern recognition patch (docs). Tatsuo Ishii <[email protected]>
2024-03-04 21:39 RE: Popcount optimization using AVX512 Amonson, Paul D <[email protected]>
2024-03-04 22:21 ` Re: Popcount optimization using AVX512 Nathan Bossart <[email protected]>
2024-03-05 16:31 ` RE: Popcount optimization using AVX512 Amonson, Paul D <[email protected]>
2024-03-05 16:37 ` Re: Popcount optimization using AVX512 Nathan Bossart <[email protected]>
2024-03-05 16:52 ` RE: Popcount optimization using AVX512 Amonson, Paul D <[email protected]>
2024-03-05 22:18 ` Re: Popcount optimization using AVX512 Bruce Momjian <[email protected]>
2024-03-07 17:33 ` Re: Popcount optimization using AVX512 Nathan Bossart <[email protected]>
2024-03-07 21:36 ` Re: Popcount optimization using AVX512 Nathan Bossart <[email protected]>
2024-03-11 21:59 ` RE: Popcount optimization using AVX512 Amonson, Paul D <[email protected]>
2024-03-12 01:34 ` Re: Popcount optimization using AVX512 Nathan Bossart <[email protected]>
2024-03-13 16:39 ` Re: Popcount optimization using AVX512 Nathan Bossart <[email protected]>
2024-03-13 17:52 ` RE: Popcount optimization using AVX512 Amonson, Paul D <[email protected]>
2024-03-13 18:38 ` Re: Popcount optimization using AVX512 Nathan Bossart <[email protected]>
2024-03-14 19:50 ` RE: Popcount optimization using AVX512 Amonson, Paul D <[email protected]>
2024-03-15 15:06 ` Re: Popcount optimization using AVX512 Nathan Bossart <[email protected]>
2024-03-15 15:31 ` RE: Popcount optimization using AVX512 Amonson, Paul D <[email protected]>
2024-03-15 17:43 ` RE: Popcount optimization using AVX512 Amonson, Paul D <[email protected]>
2024-03-17 20:56 ` Re: Popcount optimization using AVX512 David Rowley <[email protected]>
2024-03-18 15:29 ` Re: Popcount optimization using AVX512 Nathan Bossart <[email protected]>
2024-03-18 16:07 ` RE: Popcount optimization using AVX512 Amonson, Paul D <[email protected]>
2024-03-18 16:20 ` Re: Popcount optimization using AVX512 Nathan Bossart <[email protected]>
2024-03-18 17:28 ` RE: Popcount optimization using AVX512 Amonson, Paul D <[email protected]>
2024-03-18 17:30 ` Re: Popcount optimization using AVX512 Nathan Bossart <[email protected]>
2024-03-07 17:53 ` Re: Popcount optimization using AVX512 Alvaro Herrera <[email protected]>
2024-03-07 17:59 ` 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