Skip to content

Commit

Permalink
std: Attempt again to inline thread-local-init across crates
Browse files Browse the repository at this point in the history
Issue rust-lang#25088 has been part of `thread_local!` for quite some time now.
Historical attempts have been made to add `#[inline]` to `__getit`
in rust-lang#43931, rust-lang#50252, and rust-lang#59720, but these attempts ended up not landing
at the time due to segfaults on Windows.

In the interim though with `const`-initialized thread locals AFAIK this
is the only remaining bug which is why you might want to use
`#[thread_local]` over `thread_local!`. As a result I figured it was
time to resubmit this and see how it fares on CI and if I can help
debugging any issues that crop up.

Closes rust-lang#25088
  • Loading branch information
alexcrichton committed May 18, 2021
1 parent 716394d commit 641d3b0
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 0 deletions.
24 changes: 24 additions & 0 deletions library/std/src/thread/local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ macro_rules! thread_local {
macro_rules! __thread_local_inner {
// used to generate the `LocalKey` value for const-initialized thread locals
(@key $t:ty, const $init:expr) => {{
#[cfg_attr(not(target_env = "msvc"), inline)] // see comments below
unsafe fn __getit() -> $crate::option::Option<&'static $t> {
const _REQUIRE_UNSTABLE: () = $crate::thread::require_unstable_const_init_thread_local();

Expand Down Expand Up @@ -260,6 +261,29 @@ macro_rules! __thread_local_inner {
#[inline]
fn __init() -> $t { $init }

// When reading this function you might ask "why is this inlined
// everywhere other than MSVC?", and that's a very reasonable
// question to ask. The short story is that it segfaults rustc if
// this function is inlined. The longer story is that MSVC looks to
// not support `extern` references to thread locals across DLL
// boundaries. This appears to at least not be supported in the ABI
// that LLVM implements.
//
// Because of this we never inline on MVSC, but we do inline on
// other platforms (where external references to thread locals
// across DLLs are supported). A better fix for this would be to
// inline this function on MSVC, but only for "statically linked"
// components. For example if two separately compiled rlibs end up
// getting linked into a DLL then it's fine to inline this function
// across that boundary. It's only not fine to inline this function
// across a DLL boundary. Unfortunately rustc doesn't currently have
// this sort of logic available in an attribute, and it's not clear
// that rustc is even equipped to answer this (it's more of a Cargo
// question kinda). This means that, unfortunately, MSVC gets the
// pessimistic path for now where it's never inlined.
//
// The issue of "should enable on MSVC sometimes" is #84933
#[cfg_attr(not(target_env = "msvc"), inline)]
unsafe fn __getit() -> $crate::option::Option<&'static $t> {
#[cfg(all(target_arch = "wasm32", not(target_feature = "atomics")))]
static __KEY: $crate::thread::__StaticLocalKeyInner<$t> =
Expand Down
6 changes: 6 additions & 0 deletions src/test/codegen/auxiliary/thread_local_aux.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#![crate_type = "lib"]
#![feature(thread_local_const_init)]

use std::cell::Cell;

thread_local!(pub static A: Cell<u64> = const { Cell::new(0) });
50 changes: 50 additions & 0 deletions src/test/codegen/thread-local.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// compile-flags: -O
// aux-build:thread_local_aux.rs
// ignore-windows FIXME(#84933)
// ignore-wasm globals are used instead of thread locals
// ignore-emscripten globals are used instead of thread locals
// ignore-android does not use #[thread_local]

#![crate_type = "lib"]
#![feature(thread_local_const_init)]

extern crate thread_local_aux as aux;

use std::cell::Cell;

thread_local!(static A: Cell<u32> = const { Cell::new(1) });

// CHECK: [[TLS_AUX:@.+]] = external thread_local local_unnamed_addr global i64
// CHECK: [[TLS:@.+]] = internal thread_local unnamed_addr global

// CHECK-LABEL: @get
#[no_mangle]
fn get() -> u32 {
// CHECK: %0 = load i32, i32* bitcast ({{.*}} [[TLS]] to i32*)
// CHECK-NEXT: ret i32 %0
A.with(|a| a.get())
}

// CHECK-LABEL: @set
#[no_mangle]
fn set(v: u32) {
// CHECK: store i32 %0, i32* bitcast ({{.*}} [[TLS]] to i32*)
// CHECK-NEXT: ret void
A.with(|a| a.set(v))
}

// CHECK-LABEL: @get_aux
#[no_mangle]
fn get_aux() -> u64 {
// CHECK: %0 = load i64, i64* [[TLS_AUX]]
// CHECK-NEXT: ret i64 %0
aux::A.with(|a| a.get())
}

// CHECK-LABEL: @set_aux
#[no_mangle]
fn set_aux(v: u64) {
// CHECK: store i64 %0, i64* [[TLS_AUX]]
// CHECK-NEXT: ret void
aux::A.with(|a| a.set(v))
}

0 comments on commit 641d3b0

Please sign in to comment.