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

[MS] Crush when calling intrinsic __cpuid #49477

Closed
phoebewang opened this issue Apr 27, 2021 · 3 comments
Closed

[MS] Crush when calling intrinsic __cpuid #49477

phoebewang opened this issue Apr 27, 2021 · 3 comments
Labels
backend:X86 bugzilla Issues migrated from bugzilla

Comments

@phoebewang
Copy link
Contributor

Bugzilla Link 50133
Resolution FIXED
Resolved on Apr 29, 2021 19:25
Version trunk
OS Windows NT
CC @topperc,@RKSimon,@phoebewang,@rnk,@rotateright
Fixed by commit(s) https://reviews.llvm.org/rGe0c7db7d8ce780df5129b4d0f5bbf145271ef14f

Extended Description

Small reproducer:

#include
#include
#include <intrin.h>

struct A {
std::string S;
std::vector<std::array<int, 4>> V;
A() {
std::array<int, 4> B;
__cpuid(B.data(), 0);
V.push_back(B);
V.push_back(B);
char C[64];
memset(C, 0, sizeof(C));
S = C;
}
} T;

Commands:

clang-cl -mavx2 /EHs repro.cpp -S
cat repro.asm | grep cpuid -A7
cpuid
#NO_APP
mov r11d, eax
mov rax, qword ptr [rbx + 72] # 8-byte Reload
mov r10d, ebx
mov r9d, ecx
mov rcx, qword ptr [rbx + 80] # 8-byte Reload
mov r8d, edx

@phoebewang
Copy link
Contributor Author

As far as my observation, we didn't consider if the frame pointer RBX will be clobbered by inline asm.
I have no idea if we can fix this problem in frame lowering completely.
Given usage of inline asm is rare on Windows, especially MSVC doesn't support for 64 bits. I think a workaround should be good for a quick fix.
Candidate patch: https://reviews.llvm.org/D101338

@rnk
Copy link
Collaborator

rnk commented Apr 29, 2021

This is basically an instance of the more general problem, llvm.org/pr16830, which has been known for a while. In this case, I think a workaround makes sense.

@phoebewang
Copy link
Contributor Author

Thanks for the information.

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 11, 2021
facebook-github-bot pushed a commit to facebook/folly that referenced this issue Apr 8, 2022
Summary:
We need to preserve `rbx` across the call to `cpuid` on Windows. This becomes obvious when compiling under ASan where presumably due to high register pressure, we end up in a situation where `rbx` is clobbered by `cpuid` but the compiler did not expect it to be clobbered.

This is fixed in clang upstream here:
llvm/llvm-project#49477
https://reviews.llvm.org/D101338

However we use an older compiler (LLVM12.0.1) which does not have the patch yet.

Reviewed By: yfeldblum

Differential Revision: D35229538

fbshipit-source-id: 265835fb0e79e3a209dbce4fe82e8baa43e3d6ba
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:X86 bugzilla Issues migrated from bugzilla
Projects
None yet
Development

No branches or pull requests

2 participants