| 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: | Whole Thread | Raw Message | 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 |