martinus / robin-hood-hashing

Fast & memory efficient hashtable based on robin hood hashing for C++11/14/17/20
https://gitter.im/martinus/robin-hood-hashing
MIT License
1.5k stars 142 forks source link

Fix CRC32 support on Windows ARM64 and use runtime detection #92

Closed wyattoday closed 3 years ago

wyattoday commented 3 years ago

__cpuid isn't present on ARM64 builds in Windows, and CRC32 is always present for ARM64 processors that Windows 10 supports (making runtime detection unnecessary).

Also, remove the hardware support detection at compile time. You've already added runtime detection (the more flexible method), so use that.

martinus commented 3 years ago

Hi, thanks for that! Unfortunately this does not compile when one doesn't compile with -march=native, then I get these compile errors:

/opt/gcc-9.1.0-visibility/lib/gcc/x86_64-linux-gnu/9.1.0/include/smmintrin.h:846:1: error: inlining failed in call to always_inline ‘long long unsigned int _mm_crc32_u64(long long unsigned int, long long unsigned int)’: target specific option mismatch
  846 | _mm_crc32_u64 (unsigned long long __C, unsigned long long __V)
      | ^~~~~~~~~~~~~
wyattoday commented 3 years ago

We hadn't tried the Linux builds yet (we use g++ 9 as well, so I'll see if we run into the same compile errors). But this bit of code was untouched. Meaning all we did was remove the compile-time detection of CRC32 -- but keep the runtime detection.

wyattoday commented 3 years ago

Yes, it's looking like a problem in the macro definition ROBIN_HOOD_CRC32_64 itself. Might be due to the quirkiness of unsigned long long int vs. uint64_t.

martinus commented 3 years ago

I think we need at least the compiler flag -msse4.2 for this to compile. Otherwise the instruction is not available

wyattoday commented 3 years ago

You're absolutely right, I'll test the fix on our Linux / BSD builds, than add a commit that makes this PR work.

martinus commented 3 years ago

thank you!