Fixing compilation with optimisation: Set optimisation flags when building shuffle.c

[t20100]

This PR fixes a compilation issue introduced in PR #431 (commit c603171) and first raised here: #431 (comment).

The definition of SHUFFLE_USE_* made in shuffle.h relies on hardware optimisation macros (__SSE2__, __AVX2__, __ARM_NEON and __ALTIVEC__), so the hardware optimisation compilation flags must also be set where SHUFFLE_USE_* macros are used, thus in shuffle.c.

blosc2.c also includes shuffle.h, but does not use SHUFFLE_USE_* macros. Should it also be compiled with the optimisations flags? or all blosc source to avoid such issues in the future?

[t20100]

Using COMPILE_OPTIONS here since COMPILE_FLAGS is deprecated (see https://cmake.org/cmake/help/latest/prop_tgt/COMPILE_FLAGS.html).

Also COMPILE_OPTIONS is a ;-separated list which fits set_property as opposed to COMPILE_FLAGS

[FrancescAlted]

After your new commits, is really necessary to add SSE2 / AVX2 support to shuffle.c. This was not necessary before.

[t20100]

Yes, it is needed or it needs other modifications:

shuffle.c is using SHUFFLE_USE_AVX2 which is defined as:

c-blosc2/blosc/shuffle.h

Lines 28 to 30 in e204359

	#if defined(SHUFFLE_AVX2_ENABLED) && defined(__AVX2__)
	#define SHUFFLE_USE_AVX2
	#endif

So the definition needs -mavx2 flag to have __AVX2__ defined. Same applies for SSE2/NEON/ALTIVEC.

To avoid this, it looks to require to change the conditional include of [bit]shuffle-*.h and the way get_shuffle_implementation is implemented.

[FrancescAlted]

I am fine with that. I just want to make sure we don't abuse of SIMD flags.

[FrancescAlted]

I tested this, but it does not work for me:

Do you have access to the perf tool on Linux? It's a nice way to check that AVX2 is being used.

[t20100]

I also reverted commit 89f9335. This breaks the universal2 build, but it seems it is not the way to fix it. I'm looking for a way to make it work.

With this, I get the following for perf record -e cpu-cycles ./bench/b2bench lz4 bitshuffle single:

[FrancescAlted]

Yes, your last fix seems to work for me too. I just have a comment. Thanks!

[t20100]

I haven't checked yet the build for universal2 and I expect it too fail.

[FrancescAlted]

I haven't checked yet the build for universal2 and I expect it too fail.

It would be possible to disable AVX2 just for universal builds? That may provide an easy workaround.

[t20100]

I checked universal2 and it works fine, && !defined(__IMMINTRIN_H) is doing the work also in this case.

I just updated the comment at the end of #if...#end in shuffle.c: https://github.com/Blosc/c-blosc2/pull/436/files#diff-a10ebc109ea502babcc7b8845fc8c74137389e4a46d30667e2bfee427f21e020R163

[FrancescAlted]

Thanks @t20100 !