acoustid / chromaprint

C library for generating audio fingerprints used by AcoustID
Other
971 stars 133 forks source link

Compile chromaprint 1.4 with Visual C++ 2015 #28

Closed CyberSinh closed 7 years ago

CyberSinh commented 7 years ago

Hi Lukas,

I tried to compile chromaprint 1.4 with MSVC 2015 and the compilation failed because __builtin_popcount, a GCC specific function, is missing.

I had to add this code to allow MSVC to compile the library:

#ifdef _MSC_VER
#include <intrin.h>
#define __builtin_popcount __popcnt
#define __builtin_popcountll __popcnt64
#endif

I don't know if adding that is sufficient to get robust code. According to the MSDN documentation, the hardware support should be checked before using _popcount.

To determine hardware support for the popcnt instruction, call the __cpuid intrinsic with InfoType=0x00000001 and check bit 23 of CPUInfo[2] (ECX). This bit is 1 if the instruction is supported, and 0 otherwise. If you run code that uses this intrinsic on hardware that does not support the popcnt instruction, the results are unpredictable. To determine hardware support for the popcnt instruction, call the __cpuid intrinsic with InfoType=0x00000001 and check bit 23 of CPUInfo[2] (ECX). This bit is 1 if the instruction is supported, and 0 otherwise. If you run code that uses this intrinsic on hardware that does not support the popcnt instruction, the results are unpredictable.

It seems GCC use a fallback implementation if __builtin_popcount is called on a system without popcount CPU support.

lalinsky commented 7 years ago

This should be fixed in https://github.com/acoustid/chromaprint/commit/6802c8f9dc93de0a40ff0b6fb9af60a13a8b1deb

CyberSinh commented 7 years ago

Thanks Lukas. But cannot we expect more performance by using the intrinsic __popcnt function available in MSVC?

lalinsky commented 7 years ago

This function is currently not used anywhere except for the matcher (which is not publicly exposed). Calculating the bit counts is not a very big part of the matching process. I'm using code like this in production and it has never been a problem. It's not worth dealing with the CPU detection.