-
Notifications
You must be signed in to change notification settings - Fork 5k
[NativeAOT] Thin locks #79519
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
[NativeAOT] Thin locks #79519
Changes from all commits
aaee546
2ddb01a
7a49ffc
700d437
cd5fc2a
7beea61
59610de
f809161
e3d4e87
46f28ed
16183e9
236410f
0eebcab
85dc071
77c1bec
867a8ee
b755737
6b557ec
bc54d49
0a78358
367acbb
43a354d
536492b
a96f618
a7256ec
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,12 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
|
||
EXTERN_C DECLSPEC_THREAD ThreadBuffer tls_CurrentThread; | ||
#ifdef _MSC_VER | ||
// a workaround to prevent tls_CurrentThread from becoming dynamically checked/initialized. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we keep on There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just ported the workaround from a similar change in CoreClr. I am not sure I completely understand why it is needed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The fix works fine with extern "C", so I will add it and revert the mangling in asm |
||
EXTERN_C __declspec(selectany) __declspec(thread) ThreadBuffer tls_CurrentThread; | ||
#else | ||
EXTERN_C __thread ThreadBuffer tls_CurrentThread; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does non-Windows pay the penalty for TLS init checks? (ie similar workaround is not possible on non-Windows) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From the discussion of the original fix (#33347), I gathered that even on windows the workaround was not needed until some version of MSVC. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Linux/glibc does not require a similar fix. There are no checks when accessing TLS. Here is the disassembly for reference. Linux/glibc x64 System.Collections.Tests`::RhGetCurrentManagedThreadId():
0x5555554d5db0 <+0>: pushq %rbp
0x5555554d5db1 <+1>: movq %rsp, %rbp
0x5555554d5db4 <+4>: movq %fs:0x0, %rax
0x5555554d5dbd <+13>: leaq -0x100(%rax), %rax
0x5555554d5dc4 <+20>: movl 0xc0(%rax), %eax
0x5555554d5dca <+26>: popq %rbp
0x5555554d5dcb <+27>: retq Linux/glibc arm64 System.Collections.Tests`::RhGetCurrentManagedThreadId():
0xaaaaaab87400 <+0>: stp x29, x30, [sp, #-0x10]!
0xaaaaaab87404 <+4>: mov x29, sp
0xaaaaaab87408 <+8>: adrp x0, 4173
0xaaaaaab8740c <+12>: ldr x0, [x0, #0xa30]
0xaaaaaab87410 <+16>: nop
0xaaaaaab87414 <+20>: nop
0xaaaaaab87418 <+24>: mrs x8, TPIDR_EL0
0xaaaaaab8741c <+28>: add x8, x8, x0
0xaaaaaab87420 <+32>: ldr w0, [x8, #0xc0]
0xaaaaaab87424 <+36>: ldp x29, x30, [sp], #0x10
0xaaaaaab87428 <+40>: ret MSVC (with the fix) return ThreadStore::RawGetCurrentThread()->GetManagedThreadId();
00007FF6A99D7AE0 mov ecx,dword ptr [_tls_index (07FF6A9CB3A28h)]
00007FF6A99D7AE6 mov rax,qword ptr gs:[58h]
00007FF6A99D7AEF mov edx,10h
00007FF6A99D7AF4 mov rax,qword ptr [rax+rcx*8]
00007FF6A99D7AF8 mov eax,dword ptr [rax+rdx+0C0h]
}
00007FF6A99D7AFF ret |
||
#endif | ||
|
||
// static | ||
inline Thread * ThreadStore::RawGetCurrentThread() | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is just because we switched to SpinWait that is calibrated to 35 nsec.
Previous numbers were assumed in terms of pre-skylake "pause", or, simply put, were divided by 8 in the underlying APIs.
I also made it power of 2. It is more convenient.