cpu_x86.h dependency for runtime instruction set checks with MSVC

[alxvth]

Does the runtime check for SSE and AVX with MSVC at

hnswlib/hnswlib/hnswlib.h

Line 18 in 443d667

#include "cpu_x86.h"

currently require to use Mysticial/FeatureDetector?

If that is intentional, it would be good to mention that somewhere but otherwise it'd be nicer to not have that dependency and just go with

//#include "cpu_x86.h"
void cpuid(int32_t out[4], int32_t eax, int32_t ecx) {
    __cpuidex(out, eax, ecx);
}

[yurymalkov]

Hi @alxvth,

I am do not think it requires external dependencies and the windows build tests pass.
Do you have a different experience?

[alxvth]

Yes, for me it does not build on Windows with a MSVC compiler since that compiler does not know any cpu_x86.h

Have a look at this godbolt example. I think this problem does not show in the CI since MSVC does not set USE_AVX but instead defines __AVX__. It also does not set anything equivalent for SSE, see here. As far as I know there is no march=native equivalent for MSVC, but SSE2 compiler intrinsics are active by default. All that's why I guess the problematic #include "cpu_x86.h" is never actually reached in the windows build tests.
Removing that include will have the code run nicely in the example above.

[alxvth]

I just noticed that not all I wrote above made too much sense, I somehow overlooked the USE_SSE definition in the first lines of hnswlib.h. Anyways, the main problem remains in that MCVS does not define __SSE__. To cover both x86 and x64 msvc compilers I'd suggest to use this (a little bit more verbose) check instead of just checking for __SSE__

#if (defined(__SSE__) || _M_IX86_FP > 0 || defined(_M_AMD64) || defined(_M_X64))
#define USE_SSE
#endif

I guess then the build test on windows also run into the #include "cpu_x86.h" problem.

[yurymalkov]

Thank you! Will take a look at the PR!

[alxvth]

Fixed with 1f49ffe