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

Picolibc 1.8.5 #62882

Merged
merged 8 commits into from
Nov 20, 2023
Merged

Picolibc 1.8.5 #62882

merged 8 commits into from
Nov 20, 2023

Conversation

keith-packard
Copy link
Collaborator

@keith-packard keith-packard commented Sep 21, 2023

Merge in picolibc version 1.8.5.

There is a matching SDK PR pending, zephyrproject-rtos/sdk-ng#707

This offers two new printf variants, 'long long', which provides an integer-only mode with long long support included and a 'minimal' variant which cuts out a bunch of format modifier support to make things very small.

hello-world on cortex-m3 FLASH sizes:

float		13378
float inexact	10666
long long	 7950
integer		 7798
minimal		 6834

For comparison, here are the minimal C lib sizes:

float		11082
long long	 9194
integer		 8426
nano		 7386

With this change, all of the cbprintf configuration modes are reflected in the available printf variants, which means the SDK will be able to offer memory savings comparable to the picolibc module.

Closes: #62444
Closes: #64333

@zephyrbot
Copy link
Collaborator

zephyrbot commented Sep 21, 2023

The following west manifest projects have been modified in this Pull Request:

Name Old Revision New Revision Diff
picolibc zephyrproject-rtos/picolibc@d07c38f zephyrproject-rtos/picolibc@1a5c603 (main) zephyrproject-rtos/[email protected]

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@keith-packard
Copy link
Collaborator Author

This should address issue #62444

@jhedberg jhedberg removed this from the v3.5.0 milestone Oct 10, 2023
@jhedberg
Copy link
Member

For this to get merged in 3.5.0, we'll need a new SDK release including picolibc 1.8.5 as well. If that isn't going to happen, then this needs to wait.

I had a short chat with Anas about this. It's inevitably post 3.5 stuff.

@keith-packard
Copy link
Collaborator Author

For this to get merged in 3.5.0, we'll need a new SDK release including picolibc 1.8.5 as well. If that isn't going to happen, then this needs to wait.

I had a short chat with Anas about this. It's inevitably post 3.5 stuff.

That's what I expected. No good reason to make this large a change with only a few days to go.

@zephyrbot zephyrbot added the Release Notes To be mentioned in the release notes label Oct 20, 2023
@keith-packard
Copy link
Collaborator Author

I'm seeing the same failure on main. This test is built with the external C library by default, so I'm not sure what updating picolibc would do anyways...

The compiler cannot tell that path_list_size is never used without being
set, so help it out by providing a reasonable initialization value.

Signed-off-by: Keith Packard <[email protected]>
CBPRINTF_FULL_INTEGRAL doesn't happen to explicitly conflict with
CBPRINTF_NANO, but when CBPRINTF_NANO is enabled, there's no long long I/O
support provided.

Allow picolibc long-long I/O support to also be elided when CBPRINTF_NANO
is enabled to save similar amounts of space.

Signed-off-by: Keith Packard <[email protected]>
Picolibc version 1.8.5 offers additional memory savings and new long-long
and minimal printf variants which can be selected from either the SDK or
module version of the library.

This needed a patch to the Zephyr cmake support bits to enable one of the
new picolibc 1.8.5 features.

Signed-off-by: Keith Packard <[email protected]>
This option in picolibc switches the assert macro between a chatty version
and one which provides no information at all. This latter mode avoids
placing the associated strings in memory.

The Zephyr option is PICOLIBC_ASSERT_VERBOSE and it is disable by default.

Signed-off-by: Keith Packard <[email protected]>
@keith-packard
Copy link
Collaborator Author

With SDK 0.16.4 released, this should be ready to merge. I've rebased to re-test against current bits.

Picolibc's 'minimal' printf mode reduces functionality and size even more
than the 'integer' mode. Use this where memory is at a premium and where
the application knows that it does not require exact printf semantics.

1.8.5 adds two more printf variants, 'long long' and 'minimal'. The 'long
long' variant is the same as the 'integer' variant but with long long
support enabled. The 'minimal' variant reduces functionality and size even
more than the 'integer' mode. Applications can use this where memory is at
a premium and where the application does not require exact printf
semantics.

With these two added variants, the SDK has enough options so that all of
the cbprintf modes can be supported with the pre-compiled bits:

 1. CBPRINTF_NANO - picolibc's 'minimal' variant
 2. CBPRINTF_REDUCED_INTEGRAL - picolibc's 'integer' variant
 3. CBPRINTF_FULL_INTEGRAL - picolibc's 'long long' variant
 4. CBPRINTF_FB_SUPPORT - picolibc's 'double' variant

This patch makes the cbprintf Kconfig values drive the default picolibc
variant, disables picolibc variants not capable of supporting the required
cbprintf level, but allows applications to select more functionality in
picolibc than cbprintf requires.

Note that this depends on the SDK including picolibc 1.8.5. Without that,
selecting the 'minimal' or 'long long' variant in Zephyr will end up with
the default variant from picolibc, which is the full version with floating
point support. When using the module things will work as specified.

Signed-off-by: Keith Packard <[email protected]>
In minimal mode, format modifiers are not supported, leading to a lack
of width and precision support.

long long values are presented correctly in either long long or floating
mode.

Signed-off-by: Keith Packard <[email protected]>
When linking with the 0.16.3 SDK version of picolibc, using long long
cbprintf support pulls in the full floating point printf which is much
larger than the integer-only version. This ends up overflowing the memory
region available for it. Increase the size of that by bumping the start of
the fake region by 8kB.

Signed-off-by: Keith Packard <[email protected]>
Picolibc 1.8.5 includes more control over printf capabilities, document
those in the migration guide.

Signed-off-by: Keith Packard <[email protected]>
Copy link
Member

@mniestroj mniestroj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

I can confirm, that #62444 is solved with this PR.

@cfriedt cfriedt merged commit a4a8120 into zephyrproject-rtos:main Nov 20, 2023
27 checks passed
@henrikbrixandersen
Copy link
Member

This seems to have broken main: #65509

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

Successfully merging this pull request may close these issues.

picolibc: FPU issue on x86 with timer_behavior test 64 bit number formatting broken on Picolib