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.

Sign up for GitHub

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

Jump to bottom

Remove GCX_PTR. #74969

Merged
merged 2 commits into from
Aug 3, 2020
Merged

Remove GCX_PTR. #74969

merged 2 commits into from
Aug 3, 2020

Conversation

nnethercote
Copy link
Contributor

We store an ImplicitCtxt pointer in a thread-local value (TLV). This allows
implicit access to a GlobalCtxt and some other things.

We also store a GlobalCtxt pointer in GCX_PTR. This is always the same
GlobalCtxt as the one within the ImplicitCtxt pointer in TLV. GCX_PTR
is only used in the parallel compiler's handle_deadlock() function.

This commit does the following.

  • It removes GCX_PTR.
  • It also adds ImplicitCtxt::new(), which constructs an ImplicitCtxt from a
    GlobalCtxt. ImplicitCtxt::new() + tls::enter_context() is now
    equivalent to the old tls::enter_global().
  • Makes tls::get_tlv() public for the parallel compiler, because it's
    now used in handle_deadlock().

r? @petrochenkov

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 31, 2020
@nnethercote
Copy link
Contributor Author

I'm not totally confident about the new code in deadlock_handler(). I don't know if that code is tested. The parallel compiler's behaviour is unchanged -- it bootstraps and then fails on the same five tests as normal.

@bors
Copy link
Contributor

bors commented Aug 1, 2020

☔ The latest upstream changes (presumably #74726) made this pull request unmergeable. Please resolve the merge conflicts.

@petrochenkov
Copy link
Contributor

cc #50699 ("Blocking Rayon queries") where GCX_PTR was introduced.

r? @nikomatsakis
This should ideally be reviewed by Zoxc or michaelwoerister, but neither is here.

@nnethercote
Copy link
Contributor Author

Unfortunately #50699 has zero additional explanation beyond what is already in the code :(

@Mark-Simulacrum
Copy link
Member

r=me with a rebase.

@nnethercote
Remove GCX_PTR.
d93277b 
We store an `ImplicitCtxt` pointer in a thread-local value (TLV). This allows
implicit access to a `GlobalCtxt` and some other things.

We also store a `GlobalCtxt` pointer in `GCX_PTR`. This is always the same
`GlobalCtxt` as the one within the `ImplicitCtxt` pointer in TLV. `GCX_PTR`
is only used in the parallel compiler's `handle_deadlock()` function.

This commit does the following.
- It removes `GCX_PTR`.
- It also adds `ImplicitCtxt::new()`, which constructs an `ImplicitCtxt` from a
  `GlobalCtxt`. `ImplicitCtxt::new()` + `tls::enter_context()` is now
  equivalent to the old `tls::enter_global()`.
- Makes `tls::get_tlv()` public for the parallel compiler, because it's
  now used in `handle_deadlock()`.
@nnethercote
Use more appropriate tls::with_* methods in some places.
8c78fd2
@nnethercote
Copy link
Contributor Author

I have rebased.

@bors r=Mark-Simulacrum

@bors
Copy link
Contributor

bors commented Aug 2, 2020

📌 Commit 8c78fd2 has been approved by Mark-Simulacrum

@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 Aug 2, 2020
@bors
Copy link
Contributor

bors commented Aug 3, 2020

⌛ Testing commit 8c78fd2 with merge 8244b1b...

@bors
Copy link
Contributor

bors commented Aug 3, 2020

☀️ Test successful - checks-actions, checks-azure
Approved by: Mark-Simulacrum
Pushing 8244b1b to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 3, 2020
@bors bors merged commit 8244b1b into rust-lang:master Aug 3, 2020
@nnethercote nnethercote deleted the rm-GCX_PTR branch August 3, 2020 04:25
@Zoxc Zoxc mentioned this pull request Apr 17, 2022
Closed
@Zoxc Zoxc mentioned this pull request Aug 25, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 28, 2024
@bors
Auto merge of rust-lang#115220 - Zoxc:revive-gcx-ptr, r=oli-obk
c5e7f45 
Add a `CurrentGcx` type to let the deadlock handler access `TyCtxt`

This brings back `GCX_PTR` (previously removed in rust-lang#74969) allowing the deadlock handler access to `GlobalCtxt`. This fixes rust-lang#111522.

r? `@cjgillot`
@cuviper cuviper added this to the 1.47.0 milestone May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@nikomatsakis nikomatsakis

Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.47.0
Development

Successfully merging this pull request may close these issues.
7 participants
@nnethercote @bors @petrochenkov @Mark-Simulacrum @cuviper @nikomatsakis @rust-highfive