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

Add picolibc to existing newlib/newlib-nano configurations #287

Merged

Conversation

keith-packard
Copy link
Collaborator

This takes advantage of a PR I posted to the zephyr crosstool-ng project which changes picolibc into a companion library, allowing it to be added to any existing build without changing anything else.

@d3zd3z
Copy link

d3zd3z commented Sep 10, 2021

Not exactly a direct comment on this PR, but I wanted to bring up another question. Zephyr depends on newlib (or possibly picolibc). Right now we build newlib as part of our SDK. This makes finding and fixing issues caused by the C library fairly challenging, even though we depend on quite a bit of our posix behavior coming from this library.

What do people think of the idea of putting the C library as a module that gets built as part of Zephyr builds.

@keith-packard
Copy link
Collaborator Author

Does zephyr support submodules? Picolibc currently builds using meson; would it be useful to add cmake support to build for a single target be useful for this?

@stephanosio
Copy link
Member

stephanosio commented Sep 10, 2021

Does zephyr support submodules? Picolibc currently builds using meson; would it be useful to add cmake support to build for a single target be useful for this?

Not "submodules" as in the git sense, but we support external modules through something called "west".

See https://github.com/zephyrproject-rtos/zephyr/blob/main/west.yml
An example module: https://github.com/zephyrproject-rtos/cmsis

What do people think of the idea of putting the C library as a module that gets built as part of Zephyr builds.

As briefly noted in the chat, for newlib which is the default C library for the Zephyr SDK, it is not really feasible for multiple reasons described there.

As for the picolibc though, I think it might be of use to add it as a module so we can have a more fully featured toolchain independent C library besides the "minimal libc" we currently have in Zephyr (of course, the "toolchain independent" part would need to be further investigated).

@keith-packard
Copy link
Collaborator Author

picolibc builds with three compilers at present: gcc, clang and compcert's compiler, and is tested with gcc and clang. Adding cmake build infra to work as a git submodule within Zephyr should be straightforward, and could be upstreamed and tested in picolibc's usual testing framework.

@keith-packard
Copy link
Collaborator Author

Not "submodules" as in the git sense, but we support external modules through something called "west".

I didn't know west could do that! Let me know how I could help make this work.

@d3zd3z
Copy link

d3zd3z commented Sep 13, 2021

How we bring in various modules depends a bit on the build system of that module. In some cases, we use their CMake files. In other cases (such as Mbed TLS, currently) we have our own CMake files that describe that build.

One concern about doing this that was raised on discord was concerning the increase in build time due to having to build the C library. It is an argument for continuing to provide the C library through the SDK.

@keith-packard
Copy link
Collaborator Author

How we bring in various modules depends a bit on the build system of that module. In some cases, we use their CMake files. In other cases (such as Mbed TLS, currently) we have our own CMake files that describe that build.

I'm happy to integrate CMake files into picolibc, although just using meson would be easiest for me.

One concern about doing this that was raised on discord was concerning the increase in build time due to having to build the C library. It is an argument for continuing to provide the C library through the SDK.

Picolibc builds for a single architecture take only a few seconds, including configuration. Prob ably worth it just to reduce the complexity of the SDK.

@keith-packard
Copy link
Collaborator Author

This change is still useful as crosstool-ng has the infrastructure necessary to build libstdc++ as well as libc, so I think it's reasonable to include picolibc in the SDK, especially now that crosstool-ng has picolibc support available.

@keith-packard
Copy link
Collaborator Author

What do people think of the idea of putting the C library as a module that gets built as part of Zephyr builds.

Zephyr also supports C++ development, and libstdc++ must be built against the matching libc. That's quite a trick, which crosstool-ng handles by rebuilding (parts of) gcc against each of the libc versions. As long as Zephyr supports applications using C++ including libstdc++, we're going to probably want to support crosstool-ng toolchains.

Copy link
Member

@stephanosio stephanosio left a comment

Choose a reason for hiding this comment

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

I am in favour of including the picolibc as a Zephyr module with the goal of ultimately replacing the Zephyr's current minimal libc.

See picolibc/picolibc#177 (comment).

@keith-packard
Copy link
Collaborator Author

As discussed in discord, this PR is still useful in providing C++ support using picolibc as that requires the toolchain to provide libstdc++ that works with a specific build of the C library.

@keith-packard keith-packard force-pushed the add-picolibc branch 2 times, most recently from f4648ad to 8bfa479 Compare August 16, 2022 20:05
@evgeniy-paltsev
Copy link
Collaborator

Hi @keith-packard ,

just noted that you add this improvement for configs/arc-zephyr-elf.config but not for configs/arc64-zephyr-elf.config. I'm wondering if there is any specific reason for that?

Or it's just WIP and I'm checking it too soon? :)

@keith-packard
Copy link
Collaborator Author

Hi @keith-packard ,

just noted that you add this improvement for configs/arc-zephyr-elf.config but not for configs/arc64-zephyr-elf.config. I'm wondering if there is any specific reason for that?

Or it's just WIP and I'm checking it too soon? :)

I haven't tried arc64 yet; have you?

@keith-packard keith-packard force-pushed the add-picolibc branch 2 times, most recently from bd5bb9a to 69a3c35 Compare August 17, 2022 05:37
@stephanosio
Copy link
Member

stephanosio commented Aug 17, 2022

The picolibc configs are being (silently) disabled because the build machine is missing ninja and meson:

https://github.com/crosstool-ng/crosstool-ng/blob/29b6e00368b4f74c4d4a1cd22da7fdabdcd1d6ea/config/comp_libs/picolibc.in#L3

checking for meson... no
checking for ninja... no

ct-ng should really warn about selected, but set to n configs ....

UPDATE: see #538

@stephanosio
Copy link
Member

re https://github.com/zephyrproject-rtos/sdk-ng/actions/runs/3175967978/jobs/5182793630

d:/a/sdk-ng/sdk-ng/tools/zephyr-sdk-0.15.1-7-g5b27f40/xtensa-sample_controller_zephyr-elf/bin/../lib/gcc/xtensa-sample_controller_zephyr-elf/12.1.0/../../../../xtensa-sample_controller_zephyr-elf/bin/ld.exe: final link failed: No space left on device

So the Windows test failure is indeed due to insufficient disk space on the GitHub default WIndows runners.

#568 should fix that issue.

@keith-packard keith-packard force-pushed the add-picolibc branch 2 times, most recently from e0de5f0 to 81a093a Compare October 4, 2022 15:21
@keith-packard
Copy link
Collaborator Author

Even though the build failed when it ran out of space doing the Windows testing, I was able to extract the canadian cross built aarch64 linux toolchain for arm targets and test that on a debian aarch64 VM here. The picolibc bits worked correctly, which means I think this is ready to merge whenever the SDK is also ready.

Copy link
Member

@stephanosio stephanosio left a comment

Choose a reason for hiding this comment

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

It looks like this PR is not building picolibc for the following targets:

  • arc64-zephyr-elf
  • nios2-zephyr-elf
  • x86_64-zephyr-elf
  • xtensa-*_zephyr-elf

I recall that it was a deliberate decision to drop nios2-zephyr-elf because there were some issues and the arch is effectively dead so it was not worth the effort.

What about the rest?

@keith-packard
Copy link
Collaborator Author

* arc64-zephyr-elf

Requires setjmp/longjmp bits which picolibc doesn't have yet.

* nios2-zephyr-elf

Skipped as not interesting anymore.

* x86_64-zephyr-elf

Needs some fixes in picocrt code (or just skipping it, maybe?)

* xtensa-*_zephyr-elf

I'm not sure what the state of these architectures are in picolibc, will need to go investigate.

@keith-packard
Copy link
Collaborator Author

  • arc64-zephyr-elf

Requires setjmp/longjmp bits which picolibc doesn't have yet.

I found the ARC ABI specs and it looks like the arc32 setjmp/longjmp bits present in picolibc will
work fine -- they save one extra register (R13), which will be harmless.

@keith-packard
Copy link
Collaborator Author

  • x86_64-zephyr-elf

Needs some fixes in picocrt code (or just skipping it, maybe?)

Nope -- needed to fix stuff so that multilib x86 builds worked at all. Upstream PR is pending, when that works, I'll update picolibc bits and enable x86 here.

@keith-packard
Copy link
Collaborator Author

* xtensa-*_zephyr-elf

For xtensa to work, we need setjmp/longjmp so we can have libstdc++ built. Does anyone have the ABI spec for these chips who can make that happen? Alternatively, we could disable libstdc++ in the SDK.

@stephanosio stephanosio removed ci-macos-x86_64 Run CI for x86-64 macOS host (for PR) ci-macos-aarch64 Run CI for AArch64 macOS host (for PR) ci-windows-x86_64 Run CI for x86-64 Windows host (for PR) labels Oct 6, 2022
@stephanosio
Copy link
Member

For xtensa to work, we need setjmp/longjmp so we can have libstdc++ built. Does anyone have the ABI spec for these chips who can make that happen? Alternatively, we could disable libstdc++ in the SDK.

Not exactly a spec, but the information on the "windowed" and "CALL0" ABIs are in the "Xtensa Instruction Set Architecture (ISA) Reference Manual.".

Also there is an existing implementation in the Zephyr newlib fork:
https://github.com/zephyrproject-rtos/newlib-cygwin/blob/zephyr-newlib-3.3.0/newlib/libc/machine/xtensa/setjmp.S

@keith-packard
Copy link
Collaborator Author

Also there is an existing implementation in the Zephyr newlib fork: https://github.com/zephyrproject-rtos/newlib-cygwin/blob/zephyr-newlib-3.3.0/newlib/libc/machine/xtensa/setjmp.S

I had it stuck in my brain that the older xtensa 8266 architecture was completely different from esp32, but they're actually very similar and the same code mostly works on both. I'll pull the newer xtensa machine-specific bits into picolibc and see about building them into a toolchain for all of the xtensa targets.

@stephanosio stephanosio changed the base branch from main to topic-picolibc October 7, 2022 07:26
@keith-packard
Copy link
Collaborator Author

keith-packard commented Oct 10, 2022

For xtensa to work, we need setjmp/longjmp so we can have libstdc++ built. Does anyone have the ABI spec for these chips who can make that happen? Alternatively, we could disable libstdc++ in the SDK.

Bits merged from the newlib-esp32 project into picolibc. Xtensa libraries are now being built with the rest of the toolchains. Btw, picolibc now uses zephyr SDK to build xtensa libraries during CI, which should at least detect compile issues.

@keith-packard
Copy link
Collaborator Author

* nios2-zephyr-elf

Picolibc already had nios2 support, it just needed fixing for multilib configurations to work around the gcc flags for FPU configuration which forcibly set -fsingle-precision-constants, breaking the double precision math functions. I've hacked around that and pushed changes to picolibc upstream for merging there. Once merged, I'll add that to his PR.

Signed-off-by: Keith Packard <[email protected]>
This builds both picolibc and libstdc++ with picolibc support using
the crosstool-ng support for those configurations.

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

Nios2 added; "should" build cleanly.

@stephanosio stephanosio merged commit e909320 into zephyrproject-rtos:topic-picolibc Oct 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: crosstool-ng Issues related to crosstool-ng area: GCC Issues related to GCC (GNU Compiler Collection) ci-linux-aarch64 Run CI for AArch64 Linux host (for PR) ci-linux-x86_64 Run CI for x86-64 Linux host (for PR)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants