Add pico_thread_local library providing thread local variable support #2910

Merged
kilograham merged 16 commits from tls-support into develop 2026-05-14 14:55:11 -05:00
kilograham commented 2026-04-24 20:37:52 -05:00 (Migrated from github.com)
  • Works on GCC/llvm, newlib/picolibc, Arm/RISC-V
  • Provides picolibc style functions for FreeRTOS (and perhaps others to use)
  • Attempts to minimize runtime overhead when not used:
    • note linker scripts are not competent enough to allow us to elide code based on the emptiness of sections
    • on Arm we have a signal of whether __aeabi_read_tp or __emutls_get_address are called (we dont use this to change the link, we just do lazy initializtion from these methods which removes most oof the code if there are no thread local variables (since the methods aren't ever called and we use gc-sections)
    • on RISC-V theres not much we can do since TP reg is used
    • support for resetting thread locals on core 1 re-init is optional
    • thread local support can be dialed back if not needed (see below)
  • Three types of TLS support are provided:
    • per_thread (default) - proper per core (and potentially RTOS task) values for __thread variables
    • global - __thread is supported but only a single shared value is used
    • none - programs using __thread may fail to link and runtime results are undefined
  • Note that tdata/tbss source data sections can be used if present
  • Note that emutls source data sections can be used if present
  • Code will still compile with older linker scripts missing the tls sections
  • In either case two linear RAM sections .tls0 and .tls1 are used, and _init_tls is used to copy the relevant source data from there
  • Note .tdata/.tbss sit between .data/.bss so that they are copied/cleared correctly by crt0. This was the case before, but I've recombined the separate linker script fragments to make this more concrete
  • The CMake build has code to detect emutls vs __aeabi_read_tp on Arm which allows it to pass defines indicating support is only needed for one type. Of course, other build systems or pico_clib_interfaces are free to define these variables too, but they are not required for correct behavior (they may just save space)
  • Note that the code was basde somewhat on earlier separate PRs. Despite being somewhat related to the c library being used, it is orthogonal in the pico-sdk code since it really is a compiler decision. If a particular pico_clib_interface wants to influence the behavior, it can do so via #defines

PER_THREAD tls mode

  • picolibc style tls funcs (_tls_size(), _init_tls()) work based on either tdata/tbss or emutls source data for the latter the emutls_objects are walked to figure out a linear size
  • two sections .tls0 and .tls1 are added to RAM to hold tdata and tbss for the two cores
  • .tls1 is re-initialized (or rather re-initialization is marked but deferred via per core runtime_init by default so values are reset on core 1 reset). There is a define to turn this off to save a tiny amount of space if not needed
  • note that .tbss actually currently wasted space in RAM in this scenario which is annoying but I think common to most runtimes
  • note similarly .tdata need not be in RAM here either as it is not directly
  • note w.r.t. to the last two points I don't want to rock the boar in terms of other code that may use these sections (e.g. other RTOS)
  • on RISC-V TP is initialed per core to .tls0 and .tls1

GLOBAL tls mode

  • the __aeabi_read_tp() value returned always points at .tdata/.tbss
  • RISC-V TP reg is initialized to .tdata./.tbss on core init
  • emutls falls back to the c library
  • the .tls0 and .tls1 sections are elided from the output via some sorcery
  • _tls_size() returns 0 to prevent anyone else allocating space

NONE tls mode

  • the .tls0 and .tls1 sections are elided from the output via some sorcery
  • no other code is included, so you are left with whatever c library support there is, but without crt having initialized it correctly. in other words, it ain't gonna work right!
* Works on GCC/llvm, newlib/picolibc, Arm/RISC-V * Provides picolibc style functions for FreeRTOS (and perhaps others to use) * Attempts to minimize runtime overhead when not used: - note linker scripts are not competent enough to allow us to elide code based on the emptiness of sections - on Arm we have a signal of whether __aeabi_read_tp or __emutls_get_address are called (we dont use this to change the link, we just do lazy initializtion from these methods which removes most oof the code if there are no thread local variables (since the methods aren't ever called and we use gc-sections) - on RISC-V theres not much we can do since TP reg is used - support for resetting thread locals on core 1 re-init is optional - thread local support can be dialed back if not needed (see below) * Three types of TLS support are provided: - per_thread (default) - proper per core (and potentially RTOS task) values for __thread variables - global - __thread is supported but only a single shared value is used - none - programs using __thread may fail to link and runtime results are undefined * Note that tdata/tbss source data sections can be used if present * Note that emutls source data sections can be used if present * Code will still compile with older linker scripts missing the tls sections * In either case two linear RAM sections .tls0 and .tls1 are used, and _init_tls is used to copy the relevant source data from there * Note .tdata/.tbss sit between .data/.bss so that they are copied/cleared correctly by crt0. This was the case before, but I've recombined the separate linker script fragments to make this more concrete * The CMake build has code to detect emutls vs __aeabi_read_tp on Arm which allows it to pass defines indicating support is only needed for one type. Of course, other build systems or pico_clib_interfaces are free to define these variables too, but they are not required for correct behavior (they may just save space) * Note that the code was basde somewhat on earlier separate PRs. Despite being somewhat related to the c library being used, it is orthogonal in the pico-sdk code since it really is a compiler decision. If a particular pico_clib_interface wants to influence the behavior, it can do so via #defines PER_THREAD tls mode - picolibc style tls funcs (_tls_size(), _init_tls()) work based on either tdata/tbss or emutls source data for the latter the emutls_objects are walked to figure out a linear size - two sections .tls0 and .tls1 are added to RAM to hold tdata and tbss for the two cores - .tls1 is re-initialized (or rather re-initialization is marked but deferred via per core runtime_init by default so values are reset on core 1 reset). There is a define to turn this off to save a tiny amount of space if not needed - note that .tbss actually currently wasted space in RAM in this scenario which is annoying but I think common to most runtimes - note similarly .tdata need not be in RAM here either as it is not directly - note w.r.t. to the last two points I don't want to rock the boar in terms of other code that may use these sections (e.g. other RTOS) - on RISC-V TP is initialed per core to .tls0 and .tls1 GLOBAL tls mode - the __aeabi_read_tp() value returned always points at .tdata/.tbss - RISC-V TP reg is initialized to .tdata./.tbss on core init - emutls falls back to the c library - the .tls0 and .tls1 sections are elided from the output via some sorcery - _tls_size() returns 0 to prevent anyone else allocating space NONE tls mode - the .tls0 and .tls1 sections are elided from the output via some sorcery - no other code is included, so you are left with whatever c library support there is, but without crt having initialized it correctly. in other words, it ain't gonna work right!
Wren6991 (Migrated from github.com) reviewed 2026-04-24 20:37:52 -05:00
kilograham commented 2026-04-24 20:39:09 -05:00 (Migrated from github.com)

cc @alastairpatrick @keith-packard

cc @alastairpatrick @keith-packard
keith-packard (Migrated from github.com) reviewed 2026-04-27 11:02:19 -05:00
kilograham (Migrated from github.com) reviewed 2026-04-27 14:36:20 -05:00
will-v-pi (Migrated from github.com) reviewed 2026-05-08 09:12:42 -05:00
@ -0,0 +1,93 @@
will-v-pi (Migrated from github.com) commented 2026-05-08 08:24:42 -05:00

I don't think TLS_TYPE needs to be a CACHE variable, it can just be a normal one

            set(TLS_TYPE "emutls")
        elseif(NM_OUT MATCHES "__aeabi_read_tp")
            message(STATUS "Detected native TLS (__aeabi_read_tp)")
            set(TLS_TYPE "arm-aeabi")
        else()
            if (NOT PICO_RISCV)
                message(WARNING "Could not identify TLS mechanism")
            endif()
            set(TLS_TYPE "unknown")
        endif()
    else()
        message(STATUS "Compiler does not support __thread keyword.")
        set(TLS_TYPE "unsupported")
I don't think `TLS_TYPE` needs to be a `CACHE` variable, it can just be a normal one ```suggestion set(TLS_TYPE "emutls") elseif(NM_OUT MATCHES "__aeabi_read_tp") message(STATUS "Detected native TLS (__aeabi_read_tp)") set(TLS_TYPE "arm-aeabi") else() if (NOT PICO_RISCV) message(WARNING "Could not identify TLS mechanism") endif() set(TLS_TYPE "unknown") endif() else() message(STATUS "Compiler does not support __thread keyword.") set(TLS_TYPE "unsupported") ```
will-v-pi (Migrated from github.com) commented 2026-05-08 08:26:23 -05:00

It might be better to write this in CMAKE_CURRENT_BINARY_DIR to avoid polluting the root of the build directory

    file(WRITE "${CMAKE_CURRENT_BINARY_DIR}/tls_check.c" "${TLS_CHECK_SOURCE}")

    set(TLS_CHECK_LIBRARY "${CMAKE_CURRENT_BINARY_DIR}/tls_check.a")
    set(CMAKE_TRY_COMPILE_TARGET_TYPE STATIC_LIBRARY)
    # Attempt to compile the source file
    try_compile(CAN_COMPILE_TLS
            "${CMAKE_CURRENT_BINARY_DIR}"                     # Directory for temporary build files
            "${CMAKE_CURRENT_BINARY_DIR}/tls_check.c"        # Source file to compile
It might be better to write this in `CMAKE_CURRENT_BINARY_DIR` to avoid polluting the root of the build directory ```suggestion file(WRITE "${CMAKE_CURRENT_BINARY_DIR}/tls_check.c" "${TLS_CHECK_SOURCE}") set(TLS_CHECK_LIBRARY "${CMAKE_CURRENT_BINARY_DIR}/tls_check.a") set(CMAKE_TRY_COMPILE_TARGET_TYPE STATIC_LIBRARY) # Attempt to compile the source file try_compile(CAN_COMPILE_TLS "${CMAKE_CURRENT_BINARY_DIR}" # Directory for temporary build files "${CMAKE_CURRENT_BINARY_DIR}/tls_check.c" # Source file to compile ```
@ -0,0 +76,4 @@
message(STATUS "Compiler does not support __thread keyword.")
set(TLS_TYPE "unsupported")
endif()
set(${OUTPUT_VAR} ${TLS_TYPE} PARENT_SCOPE)
will-v-pi (Migrated from github.com) commented 2026-05-08 08:22:22 -05:00

Maybe delete the files before returning too, to clean up

    set(${OUTPUT_VAR} ${TLS_TYPE} PARENT_SCOPE)
    file(REMOVE ${CMAKE_CURRENT_BINARY_DIR}/tls_check.c ${CMAKE_CURRENT_BINARY_DIR}/tls_check.a)
Maybe delete the files before returning too, to clean up ```suggestion set(${OUTPUT_VAR} ${TLS_TYPE} PARENT_SCOPE) file(REMOVE ${CMAKE_CURRENT_BINARY_DIR}/tls_check.c ${CMAKE_CURRENT_BINARY_DIR}/tls_check.a) ```
@ -0,0 +1,122 @@
/*
will-v-pi (Migrated from github.com) commented 2026-05-08 08:50:37 -05:00

It looks like there are 2 names used for this - PICO_THREAD_LOCAL_THREAD_POINTER_ARM_CONFIRMED here which has the PICO_CONFIG but doesn't affect the build, or PICO_THREAD_LOCAL_THREAD_POINTER_ARM_AEABI_CONFIRMED below which doesn't have a PICO_CONFIG but does affect the build. I assume these are supposed to be the same name?

It looks like there are 2 names used for this - `PICO_THREAD_LOCAL_THREAD_POINTER_ARM_CONFIRMED` here which has the `PICO_CONFIG` but doesn't affect the build, or `PICO_THREAD_LOCAL_THREAD_POINTER_ARM_AEABI_CONFIRMED` below which doesn't have a `PICO_CONFIG` but does affect the build. I assume these are supposed to be the same name?
will-v-pi (Migrated from github.com) commented 2026-05-08 09:02:34 -05:00

CMake set doesn't use =

 * The TLS provided by the `pico_thread_local` library may be set from CMake via `pico_set_tls_implementation(<TARGET> per_thread|global|none)` and the default for all targets may be set via CMake variable (e.g. `set(PICO_DEFAULT_TLS_IMPL pico_thread_local_none`).
CMake `set` doesn't use `=` ```suggestion * The TLS provided by the `pico_thread_local` library may be set from CMake via `pico_set_tls_implementation(<TARGET> per_thread|global|none)` and the default for all targets may be set via CMake variable (e.g. `set(PICO_DEFAULT_TLS_IMPL pico_thread_local_none`). ```
will-v-pi (Migrated from github.com) commented 2026-05-08 09:08:14 -05:00
// we assume here and in thread_local.c that if PICOLIBC_TLS is set, or we're on RISC-V
```suggestion // we assume here and in thread_local.c that if PICOLIBC_TLS is set, or we're on RISC-V ```
kilograham (Migrated from github.com) reviewed 2026-05-08 09:18:25 -05:00
@ -0,0 +1,122 @@
/*
kilograham (Migrated from github.com) commented 2026-05-08 09:18:25 -05:00

i will review - actually i think i'm gonna back out the part of @lurch config checker which requires initializers for defaults of 0 too because these defines just clutter things

i will review - actually i think i'm gonna back out the part of @lurch config checker which requires initializers for defaults of 0 too because these defines just clutter things
lurch (Migrated from github.com) reviewed 2026-05-09 15:47:36 -05:00
@ -151,6 +151,17 @@ string_flag(
],
lurch (Migrated from github.com) commented 2026-05-09 15:47:35 -05:00
# PICO_BAZEL_CONFIG: PICO_DEFAULT_THREAD_LOCAL_IMPL, The default implementation for pico_thread_local to link; per_thread provides per thread locals while global provides shared global values, type=string, default=per_thread, group=build
```suggestion # PICO_BAZEL_CONFIG: PICO_DEFAULT_THREAD_LOCAL_IMPL, The default implementation for pico_thread_local to link; per_thread provides per thread locals while global provides shared global values, type=string, default=per_thread, group=build ```
lurch (Migrated from github.com) reviewed 2026-05-09 16:03:28 -05:00
@ -0,0 +1,63 @@
/* Sections for proper TLS
lurch (Migrated from github.com) commented 2026-05-09 16:03:27 -05:00
    /* this marker is present if the code knows we don't need __tls0 and __tls1... we decide
```suggestion /* this marker is present if the code knows we don't need __tls0 and __tls1... we decide ```
lurch (Migrated from github.com) reviewed 2026-05-09 16:12:51 -05:00
@ -0,0 +42,4 @@
}
")
# Create a temporary file for the source
lurch (Migrated from github.com) commented 2026-05-09 16:12:50 -05:00

Perhaps it's worth doing

set(TLS_CHECK_SOURCEFILE "${CMAKE_CURRENT_BINARY_DIR}/tls_check.c")

and then e.g. line 79 might "simplify" to:

file(REMOVE ${TLS_CHECK_SOURCEFILE} ${TLS_CHECK_LIBRARY})
_Perhaps_ it's worth doing ```cmake set(TLS_CHECK_SOURCEFILE "${CMAKE_CURRENT_BINARY_DIR}/tls_check.c") ``` and then e.g. line 79 might "simplify" to: ```cmake file(REMOVE ${TLS_CHECK_SOURCEFILE} ${TLS_CHECK_LIBRARY}) ```
lurch (Migrated from github.com) reviewed 2026-05-09 16:18:37 -05:00
@ -0,0 +7,4 @@
#ifndef _PICO_THREAD_LOCAL_PICOTLS_H
#define _PICO_THREAD_LOCAL_PICOTLS_H
// Note the name of this file is slightly confusing - this is actually overriding <picotls.h> from picolibc (if present)!
lurch (Migrated from github.com) commented 2026-05-09 16:18:36 -05:00

...and presumably nothing to do with mbedtls? 🤣

...and presumably nothing to do with mbedtls? :rofl:
lurch (Migrated from github.com) reviewed 2026-05-09 16:21:14 -05:00
@ -0,0 +1,122 @@
/*
lurch (Migrated from github.com) commented 2026-05-09 16:21:13 -05:00
 * 1. `pico_thread_local_per_thread` (default) - Full support for thread local variables. Each thread/core gets its own copy of the variable initialized when it starts executing. The library tries to minimize the overhead if there are no thread locals used, however particularly on RISC-V there is always a small overhead.
```suggestion * 1. `pico_thread_local_per_thread` (default) - Full support for thread local variables. Each thread/core gets its own copy of the variable initialized when it starts executing. The library tries to minimize the overhead if there are no thread locals used, however particularly on RISC-V there is always a small overhead. ```
lurch (Migrated from github.com) reviewed 2026-05-09 16:23:17 -05:00
@ -0,0 +1,122 @@
/*
lurch (Migrated from github.com) commented 2026-05-09 16:23:17 -05:00
 * The support provided by the `pico_thread_local` library may be set from CMake via `pico_set_thread_local_implementation(<TARGET> per_thread|global|none)` and the default for all targets may be set via CMake variable (e.g. `set(PICO_DEFAULT_THREAD_LOCAL_IMPL pico_thread_local_none)`).
```suggestion * The support provided by the `pico_thread_local` library may be set from CMake via `pico_set_thread_local_implementation(<TARGET> per_thread|global|none)` and the default for all targets may be set via CMake variable (e.g. `set(PICO_DEFAULT_THREAD_LOCAL_IMPL pico_thread_local_none)`). ```
lurch (Migrated from github.com) reviewed 2026-05-09 16:39:19 -05:00
@ -0,0 +55,4 @@
#if PICO_THREAD_LOCAL_EMUTLS_CONFIRMED + PICO_THREAD_LOCAL_THREAD_POINTER_ARM_AEABI_CONFIRMED + PICO_THREAD_LOCAL_THREAD_POINTER_RISCV_REG_CONFIRMED > 1
#error expected at most one of PICO_THREAD_LOCAL_EMUTLS_CONFIRMED, PICO_THREAD_LOCAL_THREAD_POINTER_ARM_AEABI_CONFIRMED and PICO_THREAD_LOCAL_THREAD_POINTER_RISCV_REG_CONFIRMED to be set
#endif
lurch (Migrated from github.com) commented 2026-05-09 16:39:18 -05:00

Cunning. Maybe it's worth doing something similar to check that only one of PICO_THREAD_LOCAL_MODE_PER_THREAD, PICO_THREAD_LOCAL_MODE_GLOBAL or PICO_THREAD_LOCAL_MODE_NONE is set, rather than the the multiple nested #ifs that are being used currently?

Cunning. Maybe it's worth doing something similar to check that only one of `PICO_THREAD_LOCAL_MODE_PER_THREAD`, `PICO_THREAD_LOCAL_MODE_GLOBAL` or `PICO_THREAD_LOCAL_MODE_NONE` is set, rather than the the multiple nested `#if`s that are being used currently?
lurch (Migrated from github.com) reviewed 2026-05-10 04:47:28 -05:00
@ -0,0 +1,335 @@
/*
lurch (Migrated from github.com) commented 2026-05-10 04:47:28 -05:00
// we don't have TP register so need to track one tls region per core
```suggestion // we don't have TP register so need to track one tls region per core ```
lurch (Migrated from github.com) reviewed 2026-05-10 04:50:35 -05:00
@ -0,0 +1,335 @@
/*
lurch (Migrated from github.com) commented 2026-05-10 04:50:35 -05:00
// This array is available to other TLS implementations too, such as a TLS implementation for an RTOS.

?

```suggestion // This array is available to other TLS implementations too, such as a TLS implementation for an RTOS. ``` ?
lurch (Migrated from github.com) reviewed 2026-05-10 05:03:07 -05:00
@ -0,0 +1,335 @@
/*
lurch (Migrated from github.com) commented 2026-05-10 05:03:07 -05:00

This seems to missing code-comments for passes 2 and 3? 🤨

This seems to missing code-comments for passes 2 and 3? :raised_eyebrow:
lurch (Migrated from github.com) reviewed 2026-05-10 05:05:02 -05:00
@ -0,0 +1,335 @@
/*
lurch (Migrated from github.com) commented 2026-05-10 05:05:02 -05:00

Was this supposed to say replacement <picotls.h> ? 🤷

Was this supposed to say `replacement <picotls.h>` ? :shrug:
lurch (Migrated from github.com) reviewed 2026-05-10 05:11:26 -05:00
@ -0,0 +1,335 @@
/*
lurch (Migrated from github.com) commented 2026-05-10 05:11:26 -05:00
        // aligned_alloc is not available in all libraries we support, and it isn't thread safe anyway,
```suggestion // aligned_alloc is not available in all libraries we support, and it isn't thread safe anyway, ```
lurch (Migrated from github.com) reviewed 2026-05-10 05:17:23 -05:00
@ -0,0 +237,4 @@
// tls = aligned_alloc(size, _emutls_align);
void *tls = malloc(size + _emutls_align - 1);
// note we never free the memory, so bumping the pointer is fine
tls = (void *)((((uintptr_t)tls) + (_emutls_align - 1)) & ~(_emutls_align - 1));
lurch (Migrated from github.com) commented 2026-05-10 05:17:23 -05:00

Perhaps?

        uint32_t align_minus_1 = _emutls_align - 1;
        void *tls = malloc(size + align_minus_1);
        // note we never free the memory, so bumping the pointer is fine
        tls = (void *)((((uintptr_t)tls) + align_minus_1) & ~align_minus_1);

Or is that the kind of optimization that the compiler will do automatically?

Perhaps? ```suggestion uint32_t align_minus_1 = _emutls_align - 1; void *tls = malloc(size + align_minus_1); // note we never free the memory, so bumping the pointer is fine tls = (void *)((((uintptr_t)tls) + align_minus_1) & ~align_minus_1); ``` Or is that the kind of optimization that the compiler will do automatically?
lurch (Migrated from github.com) reviewed 2026-05-10 05:18:58 -05:00
@ -0,0 +1,335 @@
/*
lurch (Migrated from github.com) commented 2026-05-10 05:18:58 -05:00
// on RISC-V as it is an optimization flag not a behavioral flag (i.e. it is intended to be set to 0
```suggestion // on RISC-V as it is an optimization flag not a behavioral flag (i.e. it is intended to be set to 0 ```
lurch (Migrated from github.com) reviewed 2026-05-10 05:25:29 -05:00
@ -0,0 +314,4 @@
// we expect an impl in this case
_INIT_TLS_IMPL(tls);
}
#endif
lurch (Migrated from github.com) commented 2026-05-10 05:25:29 -05:00

Should this #error if _INIT_TLS_IMPL isn't defined?
(and obviously same question for _SET_TLS_IMPL below)

Should this `#error` if `_INIT_TLS_IMPL` isn't defined? (and obviously same question for `_SET_TLS_IMPL` below)
lurch (Migrated from github.com) reviewed 2026-05-10 05:29:47 -05:00
@ -0,0 +7,4 @@
add_executable(pico_thread_local_test_global pico_thread_local_test.c)
target_link_libraries(pico_thread_local_test_global PRIVATE pico_stdlib pico_multicore)
pico_add_extra_outputs(pico_thread_local_test_global)
pico_set_thread_local_implementation(pico_thread_local_test_global global)
lurch (Migrated from github.com) commented 2026-05-10 05:29:47 -05:00

Should this also be testing the "none" implementation?
EDIT: Doh, forget I said that 😉 "Code using them may not compile or link"

Should this also be testing the "none" implementation? EDIT: Doh, forget I said that :wink: "Code using them may not compile or link"
lurch (Migrated from github.com) reviewed 2026-05-10 05:32:14 -05:00
@ -0,0 +1,96 @@
/**
lurch (Migrated from github.com) commented 2026-05-10 05:32:14 -05:00

2026?

2026?
lurch (Migrated from github.com) reviewed 2026-05-10 05:37:40 -05:00
@ -0,0 +1,96 @@
/**
lurch (Migrated from github.com) commented 2026-05-10 05:37:40 -05:00

Maybe it's worth doing

#define COUNTER_START 7

and then all the 7s in this file could be replaced with COUNTER_START?

Maybe it's worth doing ```c #define COUNTER_START 7 ``` and then all the `7`s in this file could be replaced with `COUNTER_START`?
lurch (Migrated from github.com) reviewed 2026-05-10 05:38:53 -05:00
@ -0,0 +1,96 @@
/**
lurch (Migrated from github.com) commented 2026-05-10 05:38:53 -05:00

Huh?

Huh?
lurch (Migrated from github.com) reviewed 2026-05-10 05:55:18 -05:00
@ -303,8 +303,11 @@ for applicable, all_configs in chips_all_configs.items():
first_define_value = get_first_dict_key(defines_obj)
lurch (Migrated from github.com) commented 2026-05-10 05:55:18 -05:00

I think I'd be happier with this "a missing default #define is allowed if the default is 0" if it was restricted to only bool types? i.e.

            elif config_obj['attrs']['type'] == "bool" and config_default == "0":
                # a bool with a missing #define defaults to 0 (false) anyway
                logger.info('Found {} (bool) at {}:{} with a default of {}, but no matching #define found'.format(config_name, file_path, linenum, config_default))
            else:
                errors.append(Exception('Found {} at {}:{} with a default of {}, but no matching #define found'.format(config_name, file_path, linenum, config_default)))

(I've also swapped things around so that the Exception is in the final else: clause, which is how it was structured before)

I think I'd be happier with this "a missing default #define is allowed if the default is 0" if it was restricted to only `bool` types? i.e. ```suggestion elif config_obj['attrs']['type'] == "bool" and config_default == "0": # a bool with a missing #define defaults to 0 (false) anyway logger.info('Found {} (bool) at {}:{} with a default of {}, but no matching #define found'.format(config_name, file_path, linenum, config_default)) else: errors.append(Exception('Found {} at {}:{} with a default of {}, but no matching #define found'.format(config_name, file_path, linenum, config_default))) ``` (I've also swapped things around so that the Exception is in the final `else:` clause, which is how it was structured before)
kilograham (Migrated from github.com) reviewed 2026-05-11 12:05:40 -05:00
@ -303,8 +303,11 @@ for applicable, all_configs in chips_all_configs.items():
first_define_value = get_first_dict_key(defines_obj)
kilograham (Migrated from github.com) commented 2026-05-11 12:05:40 -05:00

yes, i had already though to restrict it to bools

yes, i had already though to restrict it to bools
kilograham (Migrated from github.com) reviewed 2026-05-11 12:06:50 -05:00
@ -0,0 +7,4 @@
#ifndef _PICO_THREAD_LOCAL_PICOTLS_H
#define _PICO_THREAD_LOCAL_PICOTLS_H
// Note the name of this file is slightly confusing - this is actually overriding <picotls.h> from picolibc (if present)!
kilograham (Migrated from github.com) commented 2026-05-11 12:06:50 -05:00

correct - hence tls -> thread_local_storage where we have control

correct - hence tls -> thread_local_storage where we have control
kilograham (Migrated from github.com) reviewed 2026-05-11 12:13:46 -05:00
@ -0,0 +42,4 @@
}
")
# Create a temporary file for the source
kilograham (Migrated from github.com) commented 2026-05-11 12:13:46 -05:00

possibly, but i dont care enough ;-)

possibly, but i dont care enough ;-)
kilograham (Migrated from github.com) reviewed 2026-05-11 12:14:43 -05:00
@ -0,0 +1,335 @@
/*
kilograham (Migrated from github.com) commented 2026-05-11 12:14:43 -05:00

yes

yes
kilograham (Migrated from github.com) reviewed 2026-05-11 12:15:17 -05:00
@ -0,0 +237,4 @@
// tls = aligned_alloc(size, _emutls_align);
void *tls = malloc(size + _emutls_align - 1);
// note we never free the memory, so bumping the pointer is fine
tls = (void *)((((uintptr_t)tls) + (_emutls_align - 1)) & ~(_emutls_align - 1));
kilograham (Migrated from github.com) commented 2026-05-11 12:15:16 -05:00

yes the compiler is quite capable

yes the compiler is quite capable
kilograham (Migrated from github.com) reviewed 2026-05-11 12:15:42 -05:00
@ -0,0 +314,4 @@
// we expect an impl in this case
_INIT_TLS_IMPL(tls);
}
#endif
kilograham (Migrated from github.com) commented 2026-05-11 12:15:42 -05:00

it will error anyway, i dont think there is a need for a nicer error

it will error anyway, i dont think there is a need for a nicer error
kilograham (Migrated from github.com) reviewed 2026-05-11 12:28:19 -05:00
@ -0,0 +1,335 @@
/*
kilograham (Migrated from github.com) commented 2026-05-11 12:28:18 -05:00

oops - comment is wrong

oops - comment is wrong
lurch (Migrated from github.com) reviewed 2026-05-11 15:17:23 -05:00
@ -0,0 +42,4 @@
}
")
# Create a temporary file for the source
lurch (Migrated from github.com) commented 2026-05-11 15:17:22 -05:00

Haha, fair enough. In that case, do you need to ensure that line 79 is properly quoting the paths?

Haha, fair enough. In that case, do you need to ensure that line 79 is properly quoting the paths?
lurch (Migrated from github.com) reviewed 2026-05-11 15:24:54 -05:00
@ -0,0 +1,96 @@
/**
lurch (Migrated from github.com) commented 2026-05-11 15:24:54 -05:00
    if (core0_count != COUNT + COUNTER_INIT_VALUE || core1_count != COUNT*2 + COUNTER_INIT_VALUE) {
```suggestion if (core0_count != COUNT + COUNTER_INIT_VALUE || core1_count != COUNT*2 + COUNTER_INIT_VALUE) { ```
kilograham (Migrated from github.com) reviewed 2026-05-12 16:42:19 -05:00
@ -0,0 +42,4 @@
}
")
# Create a temporary file for the source
kilograham (Migrated from github.com) commented 2026-05-12 16:42:19 -05:00

ok, on reflection, i do feel like it!

ok, on reflection, i do feel like it!
will-v-pi commented 2026-05-15 11:03:28 -05:00 (Migrated from github.com)

This has broken the picotool test build of pico-examples using Clang (https://github.com/raspberrypi/picotool/actions/runs/25926790767/job/76210082546?pr=314) with the error:

src/rp2_common/pico_thread_local/thread_local.c:84:20: fatal error: unused function '_tls_init_from_emutls_or_tdata' [-Wunused-function]

when building the tinyusb_dev_audio_4_channel_mic example. This is using:

LLVM 19.1.1 from https://github.com/ARM-software/LLVM-embedded-toolchain-for-Arm/releases/download/release-19.1.1/LLVM-ET-Arm-19.1.1-Linux-x86_64.tar.xz
This has broken the `picotool` test build of pico-examples using Clang (https://github.com/raspberrypi/picotool/actions/runs/25926790767/job/76210082546?pr=314) with the error: ``` src/rp2_common/pico_thread_local/thread_local.c:84:20: fatal error: unused function '_tls_init_from_emutls_or_tdata' [-Wunused-function] ``` when building the `tinyusb_dev_audio_4_channel_mic` example. This is using: ``` LLVM 19.1.1 from https://github.com/ARM-software/LLVM-embedded-toolchain-for-Arm/releases/download/release-19.1.1/LLVM-ET-Arm-19.1.1-Linux-x86_64.tar.xz ```
Sign in to join this conversation.
No reviewers
No milestone
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
gh-raspberrypi/pico-sdk!2910
No description provided.