Re: Use __attribute__((target(sse4.2))) for SSE42 CRC32C - Mailing list pgsql-hackers

From Nathan Bossart
Subject Re: Use __attribute__((target(sse4.2))) for SSE42 CRC32C
Date
Msg-id Zy5K5Qmlb3Z4dsd4@nathan
Whole thread Raw
In response to Re: Use __attribute__((target(sse4.2))) for SSE42 CRC32C  (Nathan Bossart <nathandbossart@gmail.com>)
Responses RE: Use __attribute__((target(sse4.2))) for SSE42 CRC32C
List pgsql-hackers
-    prog = '''
+    sse42_crc_prog = '''

nitpick: Why does this need to be renamed?

+#ifdef TEST_SSE42_WITH_ATTRIBUTE
+#if defined(__has_attribute) && __has_attribute (target)
+__attribute__((target("sse4.2")))
+#endif
+#endif

So, for meson builds, we do test with and without
__attribute___((target(..."))), but for autoconf builds, we check for
__SSE4_2__ to determine whether we need a runtime check.  This difference
isn't the fault of your patch, but it's a little odd.  That being said, I'm
not sure there's a problem with either approach.

+if host_cpu == 'x86' or host_cpu == 'x86_64'
+  pgport_sources += files(
+  'pg_crc32c_sse42_choose.c',
+  'pg_crc32c_sse42.c',
+    )
+endi

We could probably just build these unconditionally (i.e., put them in the
first list of pgport_sources in this file) instead of keeping this
complexity in the build scripts.

+#if defined(USE_SSE42_CRC32C) || defined(USE_SSE42_CRC32C_WITH_RUNTIME_CHECK)

Can we move this to just below #include "c.h"?  The reason I didn't do this
for the AVX-512 stuff is because TRY_POPCNT_FAST is defined in
pg_bitutils.h, but looking again, I may be able to at least move the check
for USE_AVX512_POPCNT_WITH_RUNTIME_CHECK up similarly.

-- 
nathan



pgsql-hackers by date:

Previous
From: Dmitry Dolgov
Date:
Subject: Re: Changing shared_buffers without restart
Next
From: Nathan Bossart
Date:
Subject: Re: New GUC autovacuum_max_threshold ?