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

Support thread local storage - Partially Fixes #291 #1019

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

alastairpatrick
Copy link
Contributor

This patch adds a pico_tls module, which adds support for GCC's __thread, C11's _Thread_local, and C++'s thread_local storage class specifiers. For example:

__thread int foo;

Different implementations of pico_tls may be selected by means of the pico_set_tls_implementation cmake macro, similar to some other SDK modules. For example, a different implementation could be selected if using an RTOS.

In this implementation, a thread corresponds to a core. No TLS storage is allocated for exceptions handlers. So there are two threads total.

Issue #291 has two parts: making C++ thread_local work and making C++ exceptions work. This patch adds support for thread_local but not C++ exceptions.

Tested by building SDK and running modified kitchen_sink in debug and release, stepping through tls_init and __wrap___emutls_get_address in GDB.

@kilograham kilograham added this to the 1.5.0 milestone Sep 11, 2022
@kilograham
Copy link
Contributor

Nice; just glanced at this so far.

A couple o-things

  • We should probably not make core_thread the default (as it pulls in new extra code), by which i actually mean we should make it the default, but not unilaterally add pico_tls as a dependency. (i.e. add that if you want TLS)
  • Can you add our copyright (congratulations on perhaps being the first that i recall to add new code!) ... you can include your name as well if you want.

@alastairpatrick
Copy link
Contributor Author

Copyright comments added to tls.c and tls.S but not CMakeLists.txt, which I think is consistent with other sources. I removed pico_tls from pico_runtime's dependencies, which I think is what you want.

newlib/libc/stdlib/aligned_alloc.c:35: undefined reference to `posix_memalign'

Maybe memalign() works.
@kilograham
Copy link
Contributor

moving to 1.7.0 so we can fix this on clang too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants