Move the thread alloc context off of Thread in CoreCLR #103055
Conversation
jkoritzinsky
added
NO-MERGE
|
Tagging subscribers to this area: @mangod9 |
…es to enable MSVC to optimize dynamic TLS initialization away. Clean up comments. Fix NativeAOT emulated TLS
…alloc context for the correct thread.
…ave nearly identical assembly as the (raw hcall impl) portable helpers in jithelpers.cpp. Use those instead. Leave JIT_Box's assembly helper as we can't get quite as good of codegen with a portable helper today (and we don't even have one at the moment)
…nsky/runtime into alloc-context-no-thread
…up when a thread dies, as the TLS memory will be released
…minate (which does all the same cleanup as DetachThread, but in a scenario where we can actually clean up the object as well).
jkoritzinsky
removed
NO-MERGE
jkoritzinsky
requested review from
jkotas and
davidwrighton
and removed request for
MichalStrehovsky
jkoritzinsky
changed the title
Co-authored-by: Jan Kotas <[email protected]>
| extern CopyValueClassUnchecked:proc | ||
| extern JIT_Box:proc | ||
|
|
||
| ; HCIMPL2(Object*, JIT_Box, CORINFO_CLASS_HANDLE type, void* unboxedData) |
There was a problem hiding this comment.
It would be nice to get rid of this asm implementation and replace it with the portable C++ one. (It can be done in separate change.)
There was a problem hiding this comment.
I was having trouble getting the exact same assembly out of this one with a C++ implementation, so I didn't want to do it in this PR. I'll move it over in another PR and we can review the assembly differences there.
There was a problem hiding this comment.
The box helper is not super perf critical. The JIT will inline it for all cases that matter. I think it would be fine to have something close enough - anything that does not raise HELPER_METHOD_FRAME on a hot path should be ok.
…nsky/runtime into alloc-context-no-thread
| #ifdef _MSC_VER | ||
| EXTERN_C __declspec(selectany) __declspec(thread) gc_alloc_context t_thread_alloc_context; | ||
| #else | ||
| EXTERN_C __thread gc_alloc_context t_thread_alloc_context; |
There was a problem hiding this comment.
I was about to see what it would take to move m_fPreemptiveGCDisabled and m_pFrame to be on this same plan (inline thread static - to save an indirection in PInvoke transitions). It would help to have all these key inline thread statics together, similar to how it is done in naot. Can we turn this into a structure like ThreadBuffer and make gc_alloc_context to be the first field in it? We can then move more into that structure.
There was a problem hiding this comment.
I can do that. Would you like me to do it in this PR or in a follow-up?
I'd prefer to do it in a follow-up when I mess with the box helper as there won't be any assembly changes I need to make then (as the thread-specific alloc-context won't be referenced in any hand-written CoreCLR assembly helpers at that point)
Fixes #99661
This PR also removes the x64-specific inline-assembly helpers for a few scenarios as we now get identical or near-identical codegen with MSVC for our release builds.