From: | Nathan Bossart <nathandbossart(at)gmail(dot)com> |
---|---|
To: | "Devulapalli, Raghuveer" <raghuveer(dot)devulapalli(at)intel(dot)com> |
Cc: | "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Use __attribute__((target(sse4.2))) for SSE42 CRC32C |
Date: | 2024-11-08 17:31:17 |
Message-ID: | Zy5K5Qmlb3Z4dsd4@nathan |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | 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
From | Date | Subject | |
---|---|---|---|
Next Message | Bertrand Drouvot | 2024-11-08 17:33:52 | Re: define pg_structiszero(addr, s, r) |
Previous Message | Dmitry Dolgov | 2024-11-08 16:43:06 | Re: Changing shared_buffers without restart |