lib/libc: Add picolibc support (aarch32, aarch64 and RISC-V) [v14] #39563

keith-packard · 2021-10-20T06:21:39Z

Picolibc is a fork of newlib designed and tested on embedded systems. It
offers a smaller memory footprint (both ROM and RAM), and native TLS
support, which uses the Zephyr TLS support.

By default, the full printf version is included in the executable, which
includes exact floating point and long long input and output. A
configuration option has been added to switch to the integer-only
version (which also omits long long support).

Here are some size comparisons using qemu-cortex-m3 and this application
(parameters passed to printf to avoid GCC optimizing it into puts):

void main(void)
{
	printf("Hello World! %s %d\n", CONFIG_BOARD, 12);
}

Size data generated during build:

		       FLASH    SRAM
    minimal	        8696    3952
    picolibc int        7600    3960
    picolibc float     12304    3960
    newlib-nano int    11696    4128
    newlib-nano float  30516    4496
    newlib             34800    6112

Signed-off-by: Keith Packard keithp@keithp.com

v2:
Include picolibc-tls.ld

v3:
Document usage in guides/c_library.rst and
getting_started/toolchain_other_x_compilers.rst

v4:
Lost the lib/libc/picolibc directory somehow!

v5:
Add PICOLIBC_ALIGNED_HEAP_SIZE configuration option.
Delete PICOLIBC_SEMIHOST option support code

v6:
Don't allocate static RAM for TLS values; TLS
values only need to be allocated for each thread.

v7:
Use arm coprocessor for TLS pointer storage where supported for
compatibility with the -mtp=cp15 compiler option (or when the
target cpu type selects this option)

Add a bunch of tests

Round TLS segment up to stack alignment so that overall stack
remains correctly aligned

Add aarch64 support

Rebase to upstream head

v8:
Share NEWLIB, NEWLIB_NANO and PICOLIBC library configuration
variables in a single LIBC_PARTITIONS variable instead of
having separate PICOLIBC_PART and NEWLIB_PART variables.

v9:
Update docs to reference pending sdk-ng support for picolibc

v10:
Support memory protection by creating a partition for
picolibc shared data and any pre-defined picolibc heap.

v11:
Fix formatting in arch/arm/core/aarch64/switch.S

v12:
Remove TLS support from this patch now that TLS is upstream
Require THREAD_LOCAL_STORAGE when using PICOLIBC for architectures
that support it.

v13:
Merge errno changes as they're only needed for picolibc.
Adapt cmake changes suggested by Torsten Tejlmand Rasmussen

v14:
Update to picolibc 1.7 and newer (new stdin/stdout/stderr ABI)

Signed-off-by: Keith Packard keithp@keithp.com

tejlmand · 2021-10-20T08:42:11Z

@keith-packard Good to see you again.

Could you give a brief overview of the differences between this PR and the old stale PR #26545 are ?

keith-packard · 2021-10-20T17:01:34Z

Could you give a brief overview of the differences between this PR and the old stale PR #26545 are ?

Rely on picolibc built using sdk-ng (PR Add picolibc to existing newlib/newlib-nano configurations sdk-ng#287)
Rebase on current Zephyr main
Adapt to picolibc changes, which now includes C++ support (upstream gcc version with changes is in ct-ng).
TLS support is now part of Zephyr

There's a second PR (#39564) which demonstrates how to take advantage of Picolibc's stdio architecture to replace printk and cbprintf.

carlescufi · 2021-12-20T17:36:16Z

@stephanosio @tejlmand @cfriedt could you please review this?

carlescufi · 2021-12-20T17:37:08Z

@keith-packard would you please rebase this on top of the latest tip of main?

V7-A also supports TPIDRURO, so go ahead and use that for TLS, enabling thread local storage for the other ARM architectures. Add __aeabi_read_tp function in case code was compiled to use that. Signed-off-by: Keith Packard <keithp@keithp.com>

keith-packard · 2021-12-20T23:09:48Z

Happy to! Note that this doesn't pass the tests yet; there's a minor picolibc bug and a bunch of tests make assumptions about the C library implementation that aren't true (like rand() uses a specific PRNG implementation).

carlescufi · 2021-12-21T17:03:35Z

Happy to! Note that this doesn't pass the tests yet; there's a minor picolibc bug and a bunch of tests make assumptions about the C library implementation that aren't true (like rand() uses a specific PRNG implementation).

That's OK for now, I just wanted to keep this updated while we wait for reviews. Thanks for rebasing.

keith-packard · 2021-12-21T22:09:51Z

That's OK for now, I just wanted to keep this updated while we wait for reviews. Thanks for rebasing.

Would be great if we could get zephyrproject-rtos/sdk-ng#287 merged so that picolibc support was available in sdk-ng. That would make reviewing and testing this PR far easier.

bbolen · 2021-12-21T22:05:37Z

arch/arm/core/aarch32/cortex_a_r/__aeabi_read_tp.S

+
+GDATA(z_arm_tls_ptr)
+
+SECTION_FUNC(text, __aeabi_read_tp)


Is there any chance that you could instead make this as a cmsis function, probably putting it in include/arch/arm/aarch32/cortex_a_r/cmsis.h until it could be moved to the true arm cmsis module? I assume you probably copied this structure from Cortex-M, but since this uses a mrc, it makes sense to abstract this access.

The other side of this operation, storing the TLS value into the register, is done in swap_helper.S without any abstraction, so I'm not sure it makes sense to create an abstraction here?

I also don't think we should use this function anywhere; it's a part of EABI, but should only be used in code compiled with -mtp=soft. However, as Zephyr is the code base storing the TLS base address, it makes sense for Zephyr to offer the standard ABI that also fetches it, so I went ahead and added it.

bbolen · 2021-12-30T20:43:39Z

Should all of the reviews happen on #39564 instead? Looks like people were commenting on these changes there as well.

keith-packard · 2021-12-30T21:32:27Z

Should all of the reviews happen on #39564 instead? Looks like people were commenting on these changes there as well.

#39564 is supposed to reference only the printf-related optimizations possible with picolibc and not the basic library integration, but you're correct in noting that most of the review for the library integration happened there. I think I'd like guidance on how to proceed:

Ask that people add their basic library review comments to this PR
Plan on referencing the review comments in Picolibc optimizations #39564 when considering this PR
Remove this PR and re-title Picolibc optimizations #39564 as a single picolibc PR, planning on merging both the basic integration and optimizations at the same time.
Further discussion

kernel/errno.c

dcpleung · 2022-01-04T19:34:21Z

lib/libc/Kconfig

+config PICOLIBC
+	bool "Picolibc library"
+	depends on !NATIVE_APPLICATION
+	select THREAD_LOCAL_STORAGE if ARCH_HAS_THREAD_LOCAL_STORAGE


Just wondering why TLS is always enabled. Can picolib work without TLS if choose to do so?

TLS must be enabled in zephyr if picolibc is built with TLS support. Otherwise, any TLS variables in picolibc will not work. I'm using 'ARCH_HAS_THREAD_LOCAL_STORAGE' as a proxy for whether picolibc was built with TLS support as I don't know how to check for that at configure time (picolibc has settings in picolibc.h which could be used, is there some way to look at those?

If picolibc TLS is also always enabled when building for those architectures, this is fine.

Well, that depends on the toolchain. ARM ships their GCC embedded toolchain with TLS disabled. As there is no impact on code not using TLS in either case, I assume the ARM people just don't realize they've disabled it. Crosstool-ng and debian both ship with toolchains that enable TLS support in the compiler and picolibc. Once the sdk-ng PR for picolibc is merged, then zephyr can rely on that toolchain for picolibc, where TLS is enabled.

In this case, might want to add "&& !(arm toolchain)" for the if condition, since TLS is not required.

I think it would be a lot better to figure out if picolibc has TLS support automatically; anyone know enough cmake to do that?

tests/lib/sprintf/src/main.c

Picolibc is a fork of newlib designed and tested on embedded systems. It offers a smaller memory footprint (both ROM and RAM), and native TLS support, which uses the Zephyr TLS support. By default, the full printf version is included in the executable, which includes exact floating point and long long input and output. A configuration option has been added to switch to the integer-only version (which also omits long long support). Here are some size comparisons using qemu-cortex-m3 and this application (parameters passed to printf to avoid GCC optimizing it into puts): void main(void) { printf("Hello World! %s %d\n", CONFIG_BOARD, 12); } FLASH SRAM minimal 8696 3952 picolibc int 7600 3960 picolibc float 12304 3960 newlib-nano int 11696 4128 newlib-nano float 30516 4496 newlib 34800 6112 --- v2: Include picolibc-tls.ld v3: Document usage in guides/c_library.rst and getting_started/toolchain_other_x_compilers.rst v4: Lost the lib/libc/picolibc directory somehow! v5: Add PICOLIBC_ALIGNED_HEAP_SIZE configuration option. Delete PICOLIBC_SEMIHOST option support code v6: Don't allocate static RAM for TLS values; TLS values only need to be allocated for each thread. v7: Use arm coprocessor for TLS pointer storage where supported for compatibility with the -mtp=cp15 compiler option (or when the target cpu type selects this option) Add a bunch of tests Round TLS segment up to stack alignment so that overall stack remains correctly aligned Add aarch64 support Rebase to upstream head v8: Share NEWLIB, NEWLIB_NANO and PICOLIBC library configuration variables in a single LIBC_PARTITIONS variable instead of having separate PICOLIBC_PART and NEWLIB_PART variables. v9: Update docs to reference pending sdk-ng support for picolibc v10: Support memory protection by creating a partition for picolibc shared data and any pre-defined picolibc heap. v11: Fix formatting in arch/arm/core/aarch64/switch.S v12: Remove TLS support from this patch now that TLS is upstream Require THREAD_LOCAL_STORAGE when using PICOLIBC for architectures that support it. v13: Merge errno changes as they're only needed for picolibc. Adapt cmake changes suggested by Torsten Tejlmand Rasmussen v14: Update to picolibc 1.7 and newer (new stdin/stdout/stderr ABI) v15: Respond to comments from dcpleung: * switch kernel/errno to use CONFIG_LIBC_ERRNO instead of CONFIG_PICOLIBC * Add comment to test/lib/sprintf as to why the %n test was disabled for picolibc. Signed-off-by: Keith Packard <keithp@keithp.com>

github-actions · 2022-03-07T00:29:28Z

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

keith-packard requested review from andyross, carlescufi, dcpleung, galak, MaureenHelm, nashif, pfalcon and tejlmand as code owners October 20, 2021 06:21

github-actions bot added area: API Changes to public APIs area: Build System area: C Library C Standard Library area: Kernel area: Tests Issues related to a particular existing or missing test labels Oct 20, 2021

keith-packard force-pushed the picolibc branch from 6a2b97a to be60960 Compare October 20, 2021 07:41

tejlmand requested review from stephanosio, carlocaione, mbolivar-nordic and cfriedt October 20, 2021 08:42

zephyrbot added area: Base OS Base OS Library (lib/os) area: POSIX POSIX API Library labels Oct 20, 2021

zephyrbot requested review from ceolin, enjiamai, KangJianX and peter-mitsis and removed request for mbolivar-nordic October 20, 2021 11:55

zephyrbot assigned tejlmand Oct 20, 2021

keith-packard mentioned this pull request Oct 21, 2021

Picolibc optimizations #39564

Closed

keith-packard force-pushed the picolibc branch from be60960 to 289dd30 Compare October 21, 2021 15:51

arch/arm: Use TPIDRURO on cortex-a too

8e9000c

V7-A also supports TPIDRURO, so go ahead and use that for TLS, enabling thread local storage for the other ARM architectures. Add __aeabi_read_tp function in case code was compiled to use that. Signed-off-by: Keith Packard <keithp@keithp.com>

keith-packard force-pushed the picolibc branch from 289dd30 to 4811f64 Compare December 20, 2021 23:07

keith-packard requested review from bbolen and ioannisg as code owners December 20, 2021 23:07

github-actions bot added the area: ARM ARM (32-bit) Architecture label Dec 20, 2021

keith-packard force-pushed the picolibc branch from 4811f64 to 95ef04c Compare December 21, 2021 18:26

bbolen requested changes Dec 21, 2021

View reviewed changes

dcpleung reviewed Jan 4, 2022

View reviewed changes

keith-packard force-pushed the picolibc branch from 95ef04c to fe48e9c Compare January 4, 2022 20:54

dcpleung approved these changes Jan 4, 2022

View reviewed changes

github-actions bot added the Stale label Mar 7, 2022

github-actions bot closed this Mar 21, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

lib/libc: Add picolibc support (aarch32, aarch64 and RISC-V) [v14] #39563

lib/libc: Add picolibc support (aarch32, aarch64 and RISC-V) [v14] #39563

keith-packard commented Oct 20, 2021

tejlmand commented Oct 20, 2021

keith-packard commented Oct 20, 2021

carlescufi commented Dec 20, 2021 •

edited

Loading

carlescufi commented Dec 20, 2021

keith-packard commented Dec 20, 2021

carlescufi commented Dec 21, 2021

keith-packard commented Dec 21, 2021

bbolen Dec 21, 2021

keith-packard Dec 22, 2021

bbolen commented Dec 30, 2021

keith-packard commented Dec 30, 2021

dcpleung Jan 4, 2022

keith-packard Jan 4, 2022

dcpleung Jan 4, 2022

keith-packard Jan 4, 2022

dcpleung Jan 4, 2022

keith-packard Jan 5, 2022

github-actions bot commented Mar 7, 2022


		GDATA(z_arm_tls_ptr)

		SECTION_FUNC(text, __aeabi_read_tp)

lib/libc: Add picolibc support (aarch32, aarch64 and RISC-V) [v14] #39563

lib/libc: Add picolibc support (aarch32, aarch64 and RISC-V) [v14] #39563

Conversation

keith-packard commented Oct 20, 2021

tejlmand commented Oct 20, 2021

keith-packard commented Oct 20, 2021

carlescufi commented Dec 20, 2021 • edited Loading

carlescufi commented Dec 20, 2021

keith-packard commented Dec 20, 2021

carlescufi commented Dec 21, 2021

keith-packard commented Dec 21, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bbolen commented Dec 30, 2021

keith-packard commented Dec 30, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Mar 7, 2022

carlescufi commented Dec 20, 2021 •

edited

Loading