CMake project relies on available headers and compiler options for instruction set availability detection

[davidebeatrici]

https://github.com/xiph/opus/blob/034c1b61a250457649d788bbf983b3f0fb63f02e/cmake/OpusFunctions.cmake#L44-L132 https://github.com/xiph/opus/blob/b83dd52868326a401c8578041e3dbea439d53f11/CMakeLists.txt#L127-L206 https://github.com/xiph/opus/blob/b83dd52868326a401c8578041e3dbea439d53f11/CMakeLists.txt#L324-L450

I believe that, instead, it should check whether the instruction set is supported by the CPU.

[xnorpx]

There is for sure improvements that can be done here no doubt.

But it would be good to understand how it happened by providing the CMake options set and what OS?

[xnorpx]

Also what version of Opus (what is the latest commit)

[ghost]

Using default CMake configure options with Ninja generator (i.e. cmake -G "Ninja" "-DBUILD_SHARED_LIBS=ON") Then cmake --build . Windows 10 x64 2004 We have opus version 1.3.1 as a submodule.

[xnorpx]

@ZeroAbility

There was bugs in regarding to avx flags in the first iteration of cmake, there is fixes here: https://github.com/xiph/opus/commit/927de8453c502586c03e25c169ec08f2a93ebc02

Can you upgrade to latest Opus master and see if you can reproduce?

[ghost]

I just did a clone and build of master and the dll appears to work.

[davidebeatrici]

I just realized /arch:AVX is only specified when both AVX_SUPPORTED and OPUS_X86_PRESUME_AVX are true, sorry about that.

In 1.3.1 only AVX_SUPPORTED is taken into account: https://github.com/xiph/opus/blob/e85ed7726db5d677c9c0677298ea0cb9c65bdd23/opus_functions.cmake#L104-L185

I updated my initial message.

[xnorpx]

So the PRESUME was added here mostly for when the target machine was known to support the intrinsic version but is not the same as the buildmachine. Think separate buildsystem and deployed on Servers with known hardware. Or compiling for your own machine that you know support AVX.

MAY_HAVE define will use runtime check to ensure that CPU supports intrinsics.

[xnorpx]

@davidebeatrici just to clarify what do want added to CMake here?

[davidebeatrici]

I would use a COMPILER_ prefix for those definitions, to make it clear that the project only checks whether the compiler supports the flag/option.

[xnorpx]

opened https://gitlab.xiph.org/xiph/opus/-/issues/2340 to track this

[davidebeatrici]

Thank you very much!