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

std: Attempt again to inline thread-local-init across crates #84876

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where you say "cargo", I would say "linker", or rather "if you are doing link-time-codegen, then it is knowable/doable" but maybe "cargo" is correct.

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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should have been ignore-msvc because now we enable it for windows-gnu without testing.

// 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))
}