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

Make thread local accesses an Rvalue in MIR #70685

Closed
oli-obk opened this issue Apr 2, 2020 · 4 comments · Fixed by #71192
Closed

Make thread local accesses an Rvalue in MIR #70685

oli-obk opened this issue Apr 2, 2020 · 4 comments · Fixed by #71192
Labels
A-mir Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html A-thread-locals Area: Thread local storage (TLS) C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@oli-obk
Copy link
Contributor

oli-obk commented Apr 2, 2020

#70598 (comment)

Thread local accesses are a runtime operation, even if LLVM shims them to appear like normal static accesses. We should encode this runtime-ness in MIR and translate them back to the llvm static accesses in the llvm backend. The other backends (miri, cranelift) then have an easier life at doing the right thing ^TM.

cc @bjorn3 @eddyb @vakaras

@eddyb
Copy link
Member

eddyb commented Apr 2, 2020

Copying my #70684 (comment) here
(note "the address of a #[thread_local] static", not "access", the address itself is runtime):


As I suggested elsewhere, I believe #[thread_local] statics should have their own separate access mechanism, probably their own Rvalue (or we could start extending what Rvalue::{Ref,RawPtr} can borrow).

That's because getting the address of a #[thread_local] static is a fundamentally runtime operation, and LLVM reusing the same syntax as for regular statics, is in the wrong here.

@jonas-schievink jonas-schievink added A-mir Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html A-thread-locals Area: Thread local storage (TLS) C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 2, 2020
@vakaras
Copy link
Contributor

vakaras commented Apr 2, 2020

@eddyb @oli-obk
Do you have an estimate how much effort this would require? If it is doable in a day or two and you could give me the mentoring instructions, I could try implementing this.

@eddyb
Copy link
Member

eddyb commented Apr 2, 2020

No idea, cc @matthewjasper @wesleywiser @spastorino (I forget who did the move to constant pointers for statics) thanks @bjorn3!

@bjorn3
Copy link
Member

bjorn3 commented Apr 2, 2020

I forget who did the move to constant pointers for statics

That was done in #67000.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-mir Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html A-thread-locals Area: Thread local storage (TLS) C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants