Skip to content
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.exe compatibility by using the correct cpuid intrinsics #4738

Merged
merged 1 commit into from
May 19, 2023

Conversation

charlescao460
Copy link
Contributor

clang-cl.exe is designed to be the drop-in replacement of MSVC's cl.exe. Therefore, _MSC_VER preprocessor symbol is also defined by clang-cl.exe.

However, some LLVM Clang's built-in functions (Intrinsics) are not compatible with MSVC. For example, __cpuid() is defined as:

#if __i386__
#define __cpuid(__leaf, __eax, __ebx, __ecx, __edx) \
    __asm("cpuid" : "=a"(__eax), "=b" (__ebx), "=c"(__ecx), "=d"(__edx) \
                  : "0"(__leaf))

(Reference: https://clang.llvm.org/doxygen/cpuid_8h_source.html)

Therefore, we need to use __get_cpuid instead of __cpuid when using clang-cl.exe

@tencent-adm
Copy link
Member

tencent-adm commented May 18, 2023

CLA assistant check
All committers have signed the CLA.

@codecov-commenter
Copy link

codecov-commenter commented May 19, 2023

Codecov Report

Merging #4738 (f4a321f) into master (903ec7c) will decrease coverage by 0.02%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #4738      +/-   ##
==========================================
- Coverage   94.59%   94.57%   -0.02%     
==========================================
  Files         687      687              
  Lines      207674   206717     -957     
==========================================
- Hits       196440   195506     -934     
+ Misses      11234    11211      -23     
Impacted Files Coverage Δ
src/cpu.cpp 58.60% <ø> (ø)

... and 4 files with indirect coverage changes

@nihui nihui merged commit 7646392 into Tencent:master May 19, 2023
@nihui
Copy link
Member

nihui commented May 19, 2023

Thanks for your contribution !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants