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

Workaround inefficient codegen for thread statics in latest MSVC #33347

Merged
merged 1 commit into from
Mar 9, 2020
Merged

Workaround inefficient codegen for thread statics in latest MSVC #33347

merged 1 commit into from
Mar 9, 2020

Conversation

jkotas
Copy link
Member

@jkotas jkotas commented Mar 8, 2020

Fixes for #33341

@Wraith2
Copy link
Contributor

Wraith2 commented Mar 8, 2020

I don't doubt the quality but could you explain the problem this fixes for those of playing along at home?

@AaronRobinsonMSFT
Copy link
Member

@Wraith2 This was a bit surprising but the general gist here is the GetThread() is no longer getting inlined. It isn't obvious why this was no longer getting inlined, but it was observed that the VC++ compiler was generating code (Win-x64 Release) in GetThread() that included a call to __dyn_tls_on_demand_init for initialization of the thread local gCurrentThreadInfo. Through a lot of trial and error it appeared that removing the extern "C" and applying a __declspec(selectany) would let the compiler know the dynamic initialization isn't needed and thus is now able to inline GetThread(). A side effect of removing the extern "C" required updating all the assembly code to use the managled named instead of the simple C style symbol.

We are working offline with the VC++ team to understand why this dynamic initialization is occurring now since it wasn't occurring using the previous tool chain (i.e. VS2017).

@Wraith2
Copy link
Contributor

Wraith2 commented Mar 8, 2020

Thanks 😁

GetThread() is no longer getting in latest MSVC and includes unnecessary call
to __dyn_tls_on_demand_init. Removing the extern "C" and applying a __declspec(selectany)
makes the compiler to generate same code as before.

Fixes for #33341
@jkotas jkotas merged commit a278840 into dotnet:master Mar 9, 2020
@jkotas jkotas deleted the thread branch March 9, 2020 06:50
@ghost ghost locked as resolved and limited conversation to collaborators Dec 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants