New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix clang-cl _tzcnt_u64 not defined issue #1017
Conversation
Thanks for the suggestion @fanzeyi . It also makes me wonder if we should have a test dedicated for this scenario (compiling with Visual Studio using the |
I'm sure this PR improves compatibility. But when I use MSVC 2019 (Version 16.10) with its I also confirmed that recent version of MSVC has // C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\Llvm\lib\clang\11.0.0\include\immintrin.h
#ifndef __IMMINTRIN_H
#define __IMMINTRIN_H
...
/* No feature check desired due to internal checks */
#include <bmiintrin.h>
...
#endif I have no idea when they fixed this error. But at least when |
I think it's great. But I don't know how we can install multiple/specific version of The problem is they don't have major version number. They just call it Anyway, I think many MSVC user starts to use |
Considering the current test situation, I wouldn't go as far as trying to have multiple versions of I get it that it wouldn't necessarily catch the issue reported in this PR. |
We are seeing this issue on Windows with LLVM 9.0.1 and my understanding is that MSVC toolchain does not participate here. The The LLVM bug was fixed in https://reviews.llvm.org/rG282eff38477ebf665f88f52411edd591067af883 and it seems to be included in LLVM 10.0.0 and after. We can have a guard like I've updated the PR to have nested macro guards. I wasn't able to find examples of other nested guards so the code style might be incorrect. Let me know what style you prefer. |
54b10a1
to
1a2e8f4
Compare
When building with Clang-cl on Windows,
_tzcnt_u64
was not properly defined until https://bugs.llvm.org/show_bug.cgi?id=30506 was fixed.This PR works around the issue by directly using the builtin intrinsic when it is building with Clang on Windows.
Test Plan:
To build lz4 with clang-cl on Windows (Before):
With changes in this PR applied:
(Also a
lib/Makefile
change is required to make this actually work: changing.o
to.obj
)Unfortunately I can't get the rest of the build working due to
windres
issues so I can't really run the test suite. :(