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

Use UnsafeCell for fast constant thread locals #122583

Merged
merged 1 commit into from Mar 16, 2024
Merged

Conversation

Zoxc
Copy link
Contributor

@Zoxc Zoxc commented Mar 16, 2024

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
Copy link
Collaborator

rustbot commented Mar 16, 2024

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

@joboet joboet left a comment

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

@joboet joboet left a comment

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..

@joboet
Copy link
Contributor

joboet commented Mar 16, 2024

Thank you!
@bors r+

@bors
Copy link
Contributor

bors commented Mar 16, 2024

📌 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
11 checks passed
@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.

None yet

5 participants