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 support #1119

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

keith-packard
Copy link

This adds picolibc as an optional C library for embedded systems.

There are patches included here for gcc which adds a command line convenience argument, '--oslib=', which would typically be processed by picolibc.specs when that is specified on the command line. As this toolchain configuration installs picolibc as the system C library, it is handy to not require '--specs=picolibc.specs' just to allow the --oslib option to work. However, that is a separable part of the series, so if custom gcc patches aren't interesting, it can be simply dropped.

This builds both C and C++ applications, with full support for libstdc++ as the picolibc patches required for that were merged into gcc quite a while ago.

@kito-cheng
Copy link
Collaborator

Hi Keith, do you mind send those GCC changes to GCC mailing list? we would like move most changes to upstream if possible :)

@keith-packard
Copy link
Author

Done! I rebased to current GCC development head. If it would help, those patches are strictly optional -- they enable use of picolibc-specific support libraries without needing to include --specsfile=picolibc.specs on the GCC command line.

@kito-cheng
Copy link
Collaborator

I am thinking that maybe we should add an -mlibc=[newlib|newlib-nano|picolibc|unknown] to bare-matel toolchain, one reason is having an unify interface to select libc implementation between clang/LLVM, you might know spec is a GCC specific stuff, that cause very bad compatibility between GCC and clang/LLVM, and using having option to control that would be better since clang/LLVM don't have those configure time option.

@kito-cheng
Copy link
Collaborator

I just reply that on GCC mailing list too, -m option is kind of target specific option so we can decide that, but that would be great to gather boarder feedback from whole GCC community.

Copy link
Collaborator

@cmuellner cmuellner left a comment

Choose a reason for hiding this comment

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

Do the check--picolib and the report--picolib targets work?

Makefile.in Outdated
@@ -903,6 +1062,17 @@ stamps/check-dhrystone-newlib-nano-%: \
$(eval $@_XLEN := $(patsubst rv32%,32,$(patsubst rv64%,64,$($@_ARCH))))
$(SIM_PREPARE) $(srcdir)/test/benchmarks/dhrystone/check -march=$($@_ARCH) -mabi=$($@_ABI) -specs=nano.specs -cc=riscv$(XLEN)-unknown-elf-gcc -objdump=riscv$(XLEN)-unknown-elf-objdump -sim=riscv$($@_XLEN)-unknown-elf-run -out=$@ $(filter %.c,$^) || true

.PHONY: check-dhrystone-picolibc
check-dhrystone-picolibc: $(patsubst %,stamps/check-dhrystone-picolibc-%,$(NEWLIB_MULTILIB_NAMES))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy'n'paste error?

Makefile.in Outdated
@@ -876,6 +1030,11 @@ stamps/check-gcc-newlib-nano: stamps/build-gcc-newlib-stage2 $(SIM_STAMP) stamps
mkdir -p $(dir $@)
date > $@

stamps/check-gcc-picolibc: stamps/build-gcc-picolibc-stage2 $(SIM_STAMP) stamps/build-dejagnu
$(SIM_PREPARE) $(MAKE) -C build-gcc-picolibc-stage2 check-gcc "RUNTESTFLAGS=--target_board='$(NEWLIB_TARGET_BOARDS)'"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy'n'paste error?

Makefile.in Outdated
@@ -956,6 +1138,10 @@ report-dhrystone-newlib: $(patsubst %,stamps/check-dhrystone-newlib-%,$(NEWLIB_M
report-dhrystone-newlib-nano: $(patsubst %,stamps/check-dhrystone-newlib-nano-%,$(NEWLIB_MULTILIB_NAMES))
if cat $^ | grep -v '^PASS'; then false; else true; fi

.PHONY: report-dhrystone-picolibc
report-dhrystone-picolibc: $(patsubst %,stamps/check-dhrystone-picolibc-%,$(NEWLIB_MULTILIB_NAMES))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy'n'paste error?

Makefile.in Outdated
@@ -993,6 +1185,13 @@ report-gdb-newlib-nano: stamps/check-gdb-newlib-nano
if grep '^$$' $(patsubst %,$(srcdir)/test/gdb-newlib/%.log,$(NEWLIB_MULTILIB_NAMES)); then exit 1; fi
if find build-gdb-newlib -iname '*.sum' | xargs grep ^FAIL | sort | grep -F -v $(patsubst %,--file=$(srcdir)/test/gdb-newlib/%.log,$(NEWLIB_MULTILIB_NAMES)); then false; else true; fi

.PHONY: report-gdb-picolibc
report-gdb-picolibc: stamps/check-gdb-picolibc
stat $(patsubst %,$(srcdir)/test/gdb-picolibc/%.log,$(NEWLIB_MULTILIB_NAMES)) || exit 1
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy'n'paste error?

Makefile.in Outdated
report-gdb-picolibc: stamps/check-gdb-picolibc
stat $(patsubst %,$(srcdir)/test/gdb-picolibc/%.log,$(NEWLIB_MULTILIB_NAMES)) || exit 1
# Fail if there are blank lines in the log file used as input for grep below.
if grep '^$$' $(patsubst %,$(srcdir)/test/gdb-picolibc/%.log,$(NEWLIB_MULTILIB_NAMES)); then exit 1; fi
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy'n'paste error?

Makefile.in Outdated
stat $(patsubst %,$(srcdir)/test/gdb-picolibc/%.log,$(NEWLIB_MULTILIB_NAMES)) || exit 1
# Fail if there are blank lines in the log file used as input for grep below.
if grep '^$$' $(patsubst %,$(srcdir)/test/gdb-picolibc/%.log,$(NEWLIB_MULTILIB_NAMES)); then exit 1; fi
if find build-gdb-picolibc -iname '*.sum' | xargs grep ^FAIL | sort | grep -F -v $(patsubst %,--file=$(srcdir)/test/gdb-picolibc/%.log,$(NEWLIB_MULTILIB_NAMES)); then false; else true; fi
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy'n'paste error?

@cmuellner
Copy link
Collaborator

Done! I rebased to current GCC development head. If it would help, those patches are strictly optional -- they enable use of picolibc-specific support libraries without needing to include --specsfile=picolibc.specs on the GCC command line.

If the patches are optional, then please remove the GIT URL change in .gitmodules.

@keith-packard
Copy link
Author

Done! I rebased to current GCC development head. If it would help, those patches are strictly optional -- they enable use of picolibc-specific support libraries without needing to include --specsfile=picolibc.specs on the GCC command line.

If the patches are optional, then please remove the GIT URL change in .gitmodules.

will do. I'll work with gcc upstream to see what they want to include (if anything).

@keith-packard
Copy link
Author

Do the check--picolib and the report--picolib targets work?

hanks for catching my copy-n-paste errors; I've fixed all of those. These targets now execute, but they don't come close to working. I suspect it would take significant effort in upstream gcc to support picolibc there, but I haven't looked.

I'm thinking that the least I could do is run the picolibc test suite; that exercises the compiler, picolibc and qemu. I could also run the glibc test suite, but that's a bit more involved. What kind of testing would you like to see here?

@keith-packard
Copy link
Author

I've pushed an update to this branch which doesn't change the GCC version. That works fine, it just requires using --specs=picolibc.specs on the link command line to avoid trying to find libgloss.

@fanghuaqi
Copy link
Contributor

fanghuaqi commented Nov 21, 2022

Will this PR be merged? I see picolibc a good replacement for newlib-nano with good code size performance, Zephyr project is already integrated in its project, see zephyrproject-rtos/zephyr#26545.

It provide really impressive code size reduction compared to newlib-nano:

text data bss dec hex version
16908 428 4917 22253 56ed minimal
15460 464 4924 20848 5170 picolibc integer-only
20272 464 4924 25660 643c picolibc full version
20862 548 4936 26346 66ea newlib-nano integer-only
50542 916 4936 56394 dc4a newlib-nano full version
48170 2924 4984 56078 db0e newlib

@cmuellner
Copy link
Collaborator

We discussed this today and decided to include picolibc.

Kito or I will do some testing and final review in the next few days and either report if issues are left or merge the PR.
Thanks!

@keith-packard
Copy link
Author

We discussed this today and decided to include picolibc.

Kito or I will do some testing and final review in the next few days and either report if issues are left or merge the PR. Thanks!

If you're planning on providing both newlib and picolibc along with libstdc++, you might want to consider a hack that crosstool-ng does to replace instances of #include_next with plain #include in the libstdc++ headers. That lets both C libraries peacefully coexist in one build tree.

@keith-packard
Copy link
Author

I've rebased this. The only 'check' which works is the binutils one; the others fail because the toolchain needs additional arguments to link applications and I can't figure out how to get those past to the various tests. The toolchain is working though; I've tested it by running the full picolibc test suite.

@cmuellner
Copy link
Collaborator

I tested this a couple of days as well and noticed that the GCC tests don't work.
Are the failing GCC tests only those which need to link? If so, can these additional link arguments be provided via env variables?

@keith-packard
Copy link
Author

I tested this a couple of days as well and noticed that the GCC tests don't work. Are the failing GCC tests only those which need to link? If so, can these additional link arguments be provided via env variables?

I think they probably need to link and run. I experimented with this for several hours and eventually gave up attempting to figure out how to pass the correct arguments through the system, including correct arguments to qemu to run the resulting programs. I did manage to compile, link, and run the dhrystone benchmark once I added 'times' support to picolibc in the semihosting code (that patch is moving upstream). If someone with knowledge of the build system wants to take a stab at making this work, you need the following options passed to the linker:

-specs=picolibc.specs --oslib=semihost -Wl,--defsym=__flash=0x80000000 -Wl,--defsym=__ram=0x80200000

With that, the examples should all work under qemu with semihosting enabled. The run-riscv script in picolibc is what I used for that as it sets all of the necessary qemu options.

https://github.com/picolibc/picolibc/blob/main/scripts/run-riscv

I'm happy to help with the picolibc bits when I can, but this build system has defeated me.

@TommyMurphyTM1234
Copy link
Collaborator

I'm just wondering what the current state of play is with this and what, if anything, remains to be done to finally make Picolibc a workable option for the RISC-V bare metal toolchain here (and/or upstream)?

With /usr merge, debian makes /bin/gawk a symlink to /usr/bin/gawk.

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

I'm just wondering what the current state of play is with this and what, if anything, remains to be done to finally make Picolibc a workable option for the RISC-V bare metal toolchain here (and/or upstream)?

Thanks for the query -- I'd not applied the requested changes to the GCC patches and resubmitted them. I've done that and now we can see if the GCC team is interested in applying them. If this project would be willing to carry those patches out-of-tree in the meantime, we could get this finished up.

The GCC patches are in my gcc repo on the riscv-picolibc-oslib branch

https://github.com/keith-packard/gcc/tree/picolibc-oslib

I've also rebased and updated this series to separate out the GCC update from the rest of the series.

@TommyMurphyTM1234
Copy link
Collaborator

Thanks for the update @keith-packard.
Would you be able to summarise how one can try to build the Picolib bare metal toolchain from this repo now?
Is it a case of following the usual instructions but using upstream GCC sources (plus the patches in your repo?) out of tree?
Or does it require something else?
Thanks again.

@keith-packard
Copy link
Author

Thanks for the update @keith-packard. Would you be able to summarise how one can try to build the Picolib bare metal toolchain from this repo now? Is it a case of following the usual instructions but using upstream GCC sources (plus the patches in your repo?) out of tree? Or does it require something else? Thanks again.

I've fixed my picolibc branch so that it (temporarily) moves the gcc bits to my mirror that includes the necessary changes. So, it should be sufficient to just checkout https://github.com/keith-packard/riscv-gnu-toolchain/tree/picolibc and follow the picolibc multilib instructions in the README.md file there.

I've got some more picolibc patches in motion that will make using the result even cleaner; once those have landed, you can actually do:

$ cat << EOF > hello-world.c
#include <stdio.h>

int
main(void)
{
	printf("hello, world\n");
	return 0;
}
EOF
$ riscv64-unknown-elf-gcc -march=rv32imac -mabi=ilp32 hello-world.c --oslib=semihost --crt0=semihost
$ qemu-system-riscv32 -semihosting-config enable=on -monitor none -serial none -nographic -machine virt,accel=tcg -cpu rv32 -bios none -kernel a.out
hello, world
$

When building the system versions of qemu, there are some warnings
which terminate the build if this flag is used. Disable it until qemu
upstream fixes the problem.

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

Do the check-_-picolib and the report-_-picolib targets work?

I've removed all of the broken test bits. The only one remaining is check-binutils-picolibc. Next, I added in the ability to run the Picolibc test suite and use that as the target for check-gcc-picolibc. That performs higher level set of tests, checking the various standard C functions including the math library for correct operation.

Fixing the gcc tests would involve significant hacking to dejagnu, which appears to have C library knowledge spread throughout. If someone feels those tests would be valuable to run against picolibc, I wish them luck with that code base.

Signed-off-by: Keith Packard <[email protected]>
This provides access to the additional commits present in the
riscv-picolibc-oslib branch.

Signed-off-by: Keith Packard <[email protected]>
For picolibc to work as the default C library, a few minor changes are
required in gcc.

Signed-off-by: Keith Packard <[email protected]>
This allows the creation of an embedded toolchain which uses picolibc
as the C library instead of newlib.

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

mrv96 commented Sep 13, 2023

upvote

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

Successfully merging this pull request may close these issues.

6 participants