Fix CMakeLists.txt

[neurolabusc]

This patch restores the functionality of CMake. In order to compile, the compiler requires the -mpclmul and you must source all the files. If you can add this, we can add this to other projects, for example this fork of pigz allows the user to select System, CloudFlare, zlib-ng or (if you accept this pull request) Intel. You can test with this with:

mkdir build
cd build
cmake ..
make

[jtkukunas]

Thanks for this.

It looks like this change would compile all of zlib w/ -mpclmul. That would let the compiler generate those instructions outside of the cpuid checks, and potentially break older platforms that don't support pclmul.

If you look at the current Makefile, you'll see I compile only specific files with these cflags to avoid that issue.

[neurolabusc]

Yes, the make file is fine. This patch is for cmake.

I did the minimal modification to allow the project to compile. Without my modifications the CMake file will always fail (e.g. with default x86.h settings: deflate.c requires slide_hash_sse, inflate.c and deflate.c require crc_fold_512to32, etc.). So this pull request is very simple, and improves a CMakefile from non-functional to functional. So it seems like a simple decision to apply it.

In future, one could extend this to ensure that the compiler and computer that support clmul (e.g. more recent than 2010):

1. Update the CMakelist to ensure it supports clmul: check_c_compiler_flag(-mpclmul HAS_PCLMUL). Note that the x86.h already includes the nice x86_cpu_has_pclmul() function to determine if the cpu supports this instruction. So you only need to check compiler support and have this toggle the USE_PCLMUL_CRC definition currently in x86.h.

[jtkukunas]

No. The runtime cpuid check I'm talking about above (x86_cpu_has_pclmul()) only works correctly if the compiler doesn't generate pclmul instructions outside those checked areas. With your patch applied, zlib built with cmake could crash on chips that do not support pclmul.

The fix to your patch is to follow the approach in the Makefile, i.e., only crc_folding.c should be compiled w/ -mpclmul

[neurolabusc]

Hmm, I guess your fear is that including the -mpclmul might allow the compiler to optimize other code to use clmul instructions even though they are not explicitly used. This has not been my experience. However, I concede it is possible that some future compiler might be clever enough to do this. How about changing one line in my patch from:

set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -mpclmul")
to read:
set_source_files_properties(crc_folding.c PROPERTIES COMPILE_FLAGS -mpclmul)

That would make the CMake work similar to your makefile.

Without adding a -mpclmul compile flag to the CMake, the CMake will fail:
crc_folding.c:431:14: error: '__builtin_ia32_pclmulqdq128' needs target feature pclmul