> - 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_prog in 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 MSVC doesn't require a specific
SSE42 flag to build with): see https://gcc.godbolt.org/z/eoc1Ec33j. This means
it is always running the SSE42 without a runtime check which can technically
SEGILL on a really old CPU (SSE42 is nearly 18 years old 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
