RE: Use __attribute__((target(sse4.2))) for SSE42 CRC32C
От | Devulapalli, Raghuveer |
---|---|
Тема | RE: Use __attribute__((target(sse4.2))) for SSE42 CRC32C |
Дата | |
Msg-id | PH8PR11MB8286A700203AAE91C5AFCB18FB5D2@PH8PR11MB8286.namprd11.prod.outlook.com обсуждение исходный текст |
Ответ на | Re: Use __attribute__((target(sse4.2))) for SSE42 CRC32C (Nathan Bossart <nathandbossart@gmail.com>) |
Ответы |
Re: Use __attribute__((target(sse4.2))) for SSE42 CRC32C
|
Список | pgsql-hackers |
> - prog = ''' > + sse42_crc_prog = ''' > > nitpick: Why does this need to be renamed? Doesn't need to be renamed, but just a better name to describe that the code is meant for. It will be followed up with avx512_crc_progin the next patch. > > +#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. I just realized, isn't this a problem on MSVC? When building with MSVC, USE_SSE42_CRC32C is always set to true (because MSVCdoesn't require a specific SSE42 flag to build with): see https://gcc.godbolt.org/z/eoc1Ec33j. This means it is alwaysrunning the SSE42 without a runtime check which can technically SEGILL on a really old CPU (SSE42 is nearly 18 yearsold though). This problem exists in the master branch too. > > +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. Sure. > +#if defined(USE_SSE42_CRC32C) || > +defined(USE_SSE42_CRC32C_WITH_RUNTIME_CHECK) > > Can we move this to just below #include "c.h"? Sounds good. Raghuveer
В списке pgsql-hackers по дате отправления: