> +    __attribute__((target("sse4.2")))
> 
> These need to be surrounded with
> 
>       #if defined(__has_attribute) && __has_attribute (target)
> 
> so that we still attempt the check on compilers that don't support it (e.g., 
> MSVC).

I assumed configure was only used on linux and ignored this check. Doesn't hurt 
to add this I suppose. 

> 
>  # Check for Intel SSE 4.2 intrinsics to do CRC calculations.
>  #
> -# First check if the _mm_crc32_u8 and _mm_crc32_u64 intrinsics can be used -#
> with the default compiler flags. If not, check if adding the -msse4.2 -# flag 
> helps.
> CFLAGS_CRC is set to -msse4.2 if that's required.
> +# Check if the _mm_crc32_u8 and _mm_crc32_u64 intrinsics can be used #
> +with the __attribute__((target("sse4.2"))).
>  PGAC_SSE42_CRC32_INTRINSICS([])
> -if test x"$pgac_sse42_crc32_intrinsics" != x"yes"; then
> -  PGAC_SSE42_CRC32_INTRINSICS([-msse4.2])
> -fi
> 
> IIUC this means we will never set USE_SSE42_CRC32C_WITH_RUNTIME_CHECK.

I don't think so. USE_SSE42_CRC32C additionally requires SSE4_2_TARGETED to be 
true which will only happen when explicitly built with -msse4.2. When this 
explicit compiler flag is missing, we set USE_SSE42_CRC32C_WITH_RUNTIME_CHECK 
to true. This logic is further down in  the configure.ac file:

if test x"$pgac_sse42_crc32_intrinsics" = x"yes" && test x"$SSE4_2_TARGETED" = 
x"1" ; then
      USE_SSE42_CRC32C=1
    else

> To maintain the existing behavior, I think we need to still perform two 
> configure
> tests (one with __attribute__((target(...))) and another without), and then 
> we can
> choose whether to set USE_SSE42_CRC32C or
> USE_SSE42_CRC32C_WITH_RUNTIME_CHECK based on the results.
> 
> +  'pg_crc32c_sse42_choose.c',
> +  'pg_crc32c_sse42.c',
> 
> Can we combine these?

Knowing the AVX-512 will come next, I think it makes sense to keep the runtime 
choose function separate. Otherwise this gets polluted with runtime choose 
function, sse42 algorithm and the avx512 algorithm in the next patch. Does that 
make sense? 

Raghuveer


Reply via email to