-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Check target definitions to see if more should have cfg(target_thread_local)
enabled.
#91659
Comments
@rustbot modify labels: +A-thread-locals +F-thread_local |
Hm, yeah. I'm pretty sure there's no reason for any Windows targets not to support it. Or at least it's very inconsistent that Btw, the name of the spec configuration option confused me. I initially overlooked that |
Porting from the issue just above this comment… We have a test |
Enable `#[thread_local]` for all windows-msvc targets As it stands, `#[thread_local]` is enabled haphazardly for msvc. It seems all 64-bit targets have it enabled, but not 32-bit targets unless they're also UWP targets (perhaps because UWP was added more recently?). So this PR simply enables it for 32-bit targets as well. I can't think of a reason not to and I've confirmed by running tests locally which pass. See also rust-lang#91659
Fixing
Contents of CGU (
For this to work Rust would have to not try to export symbols without |
This is enabled for all windows-msvc and windows-gnullvm targets, I believe it's just windows-gnu that's left as far as Windows is concerned. |
Out of curiosity, I ran a script to print out the list of targets where
cfg(target_thread_local)
(indicating support for#[thread_local]
) is not true.The results (and script) are available here: https://gist.github.com/thomcc/766d76e4da35ce1a73b93ccd548fd143
This looks somewhat wrong to me — I believe several of these should have support for static/linker thread local (or at least
__thread
/__declspec(thread)
has worked there for me). Here are some items in that list which are... fairly suspicious to me.Playing around with1
clang
,gcc
, andmsvc
a bit indicates that (the ones I can easily test of these) should have it, and probably others.My suspicion is:
If the issue is about old versions, I guess a question would be what should the cfg do in situations where the support is version-dependent...
This sounds like a tricky thing to decide! Worth noting: support for TLS is what caused us to choose 10.7 as the minimum supported version for macOS: #11927, but I guess back then2 we might not have had a
#[cfg(target_thread_local)]
?.But if nothing else, libstd probably has more concrete ideas about what it supports, and this causes it to be far more pessimistic in
std::thread_local!
.And more concretely: I'm pretty sure if I used
#[thread_local]
in a malloc, I'd be pretty annoyed if it couldn't support, say, FreeBSD, since I know that at least GCC/Clang support__thread
on that target.Footnotes
I think the correct thing to compare against would be
_Thread_local
/__thread
/__declspec(thread)
rather than C++'sthread_local
— these don't have ctor/dtor support, and thus map more directly to Rust's#[thread_local]
. That said, I don't really see a difference in what I've checked. ↩I don't feel doing that much spelunking, seems likely that given the numeric gap between the macOS issue and the
thread_local
issue, that things probably worked very differently! ↩The text was updated successfully, but these errors were encountered: