jermp / pthash

Fast and compact minimal perfect hash functions in C++.
MIT License
182 stars 26 forks source link

Fix compilation and SIGILL on Intel Ivy Bridge #2

Closed ByteHamster closed 2 years ago

jermp commented 2 years ago

Thanks @ByteHamster, what's "SIGILL" by the way? -Giulio

ByteHamster commented 2 years ago

Illegal instruction (because pdep does not exist yet)

jermp commented 2 years ago

Oh ok, thanks @ByteHamster -- I did not know it was not available on Ivy Bridge. Using pdep is much cleaner and faster than other approaches based on broadword programming though. One should use it if available. Here https://github.com/jermp/mutable_rank_select/blob/master/include/rank_select_algorithms/select.hpp me maintain a collection of select algorithms.

ByteHamster commented 2 years ago

I did not know it was not available on Ivy Bridge.

Looking at it again, I should have written #ifndef __BMI2__ instead of #ifndef __haswell__. The pdep instruction was added in BMI2 (which was first added in Haswell) but explicitly checking for haswell probably does not use it on AMD or something. PR incoming.

By the way, this change also makes PTHash compatible with ARMv9.0 processors (hence the #if defined(__x86_64__))

jermp commented 2 years ago

Yep, I've also used the __BMI2__ macro. Thanks! Also good for the ARM processor.