> - 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
