Skip to content

Use UnsafeCell for fast constant thread locals#122583

Merged
bors merged 1 commit into
rust-lang:masterfrom
Zoxc:tls-non-mut
Mar 16, 2024
Merged

Use UnsafeCell for fast constant thread locals#122583
bors merged 1 commit into
rust-lang:masterfrom
Zoxc:tls-non-mut

Conversation

@Zoxc

@Zoxc Zoxc commented Mar 16, 2024

Copy link
Copy Markdown
Contributor

This uses UnsafeCell instead of static mut for fast constant thread locals. This changes the type of the TLS shims to return &UnsafeCell<T> instead of *mut T which means they are always non-null so LLVM can optimize away the check for Some in LocalKey::with if T has no destructor.

LLVM is currently unable to do this optimization as we lose the fact that __getit always returns Some as it gets optimized to just returning the value of the TLS shim.

@rustbot

rustbot commented Mar 16, 2024

Copy link
Copy Markdown
Collaborator

r? @Amanieu

rustbot has assigned @Amanieu.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Mar 16, 2024

@joboet joboet left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good to me! Could you also remove the FIXME and the #[allow(static_mut_ref)] in line 16-18? They are no longer needed thanks to this change.

@joboet joboet assigned joboet and unassigned Amanieu Mar 16, 2024

@joboet joboet left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm very confused by that one comment..

Comment thread library/std/src/sys/thread_local/fast_local.rs
@joboet

joboet commented Mar 16, 2024

Copy link
Copy Markdown
Member

Thank you!
@bors r+

@bors

bors commented Mar 16, 2024

Copy link
Copy Markdown
Collaborator

📌 Commit b0b2493 has been approved by joboet

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 16, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 16, 2024
…enton

Rollup of 7 pull requests

Successful merges:

 - rust-lang#122323 (configure.py: add flag for loongarch64 musl-root)
 - rust-lang#122372 (prevent notifying the same changes more than once)
 - rust-lang#122390 (Bump windows-bindgen to 0.55.0)
 - rust-lang#122401 (Check library crates for all tier 1 targets in PR CI)
 - rust-lang#122489 (Bump `cargo update` PR more often)
 - rust-lang#122583 (Use `UnsafeCell` for fast constant thread locals)
 - rust-lang#122590 (bootstrap: Don't name things copy that are not copies)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit a9f8f8b into rust-lang:master Mar 16, 2024
@rustbot rustbot added this to the 1.78.0 milestone Mar 16, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 16, 2024
Rollup merge of rust-lang#122583 - Zoxc:tls-non-mut, r=joboet

Use `UnsafeCell` for fast constant thread locals

This uses `UnsafeCell` instead of `static mut` for fast constant thread locals. This changes the type of the TLS shims to return `&UnsafeCell<T>` instead of `*mut T` which means they are always non-null so LLVM can optimize away the check for `Some` in `LocalKey::with` if `T` has no destructor.

LLVM is currently unable to do this optimization as we lose the fact that `__getit` always returns `Some` as it gets optimized to just returning the value of the TLS shim.
@Zoxc Zoxc deleted the tls-non-mut branch March 17, 2024 03:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants