Skip to content

Allow overriding the TLS model #45666

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 4 commits into from
Nov 7, 2017
Merged

Allow overriding the TLS model #45666

merged 4 commits into from
Nov 7, 2017

Conversation

Amanieu
Copy link
Member

@Amanieu Amanieu commented Oct 31, 2017

This PR adds the ability to override the default "global-dynamic" TLS model with a more specific one through a target json option or a command-line option. This allows for better code generation in certain situations.

This is similar to the -ftls-model= option in GCC and Clang.

@rust-highfive
Copy link
Contributor

r? @pnkfelix

(rust_highfive has picked a reviewer for you, use r? to override)

@kennytm kennytm added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 1, 2017
@Amanieu
Copy link
Member Author

Amanieu commented Nov 2, 2017

At the moment get_tls_model is called each time a #[thread_local] static is encountered. I would like to only call this once at the start, that way the TLS model string is validated even if there are no #[thread_local] statics. However I am not sure where I should do this and where I should put the cached value.

@Amanieu
Copy link
Member Author

Amanieu commented Nov 3, 2017

Ok, I'm now caching the parsed TLS model in the crate context. This PR is ready for merging.

@carols10cents
Copy link
Member

r? @michaelwoerister

@michaelwoerister
Copy link
Member

r? @alexcrichton

I think @alexcrichton is more qualified to review this.

@alexcrichton
Copy link
Member

I don't really know what a TLS model is but it seems fine to expose for now. Can this be placed behind feature gates, however, to require nightly and go through the normal stabilization process?

@Amanieu
Copy link
Member Author

Amanieu commented Nov 6, 2017

I don't think we have a mechanism for putting a target json attribute under a feature gate.

@Amanieu
Copy link
Member Author

Amanieu commented Nov 6, 2017

Also, how do I mark a -C option as unstable?

@alexcrichton
Copy link
Member

That's ok to leave the target json as-is, those are "pretty unstable" as-is anyway. For the options you can move it to -Z to make it unstable.

@Amanieu
Copy link
Member Author

Amanieu commented Nov 6, 2017

Done

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Collaborator

bors commented Nov 7, 2017

📌 Commit fdf7ba2 has been approved by alexcrichton

@bors
Copy link
Collaborator

bors commented Nov 7, 2017

⌛ Testing commit fdf7ba2 with merge 7ade24f...

bors added a commit that referenced this pull request Nov 7, 2017
Allow overriding the TLS model

This PR adds the ability to override the default "global-dynamic" TLS model with a more specific one through a target json option or a command-line option. This allows for better code generation in certain situations.

This is similar to the `-ftls-model=` option in GCC and Clang.
@bors
Copy link
Collaborator

bors commented Nov 7, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 7ade24f to master...

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants