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

Build -Os and -O2 versions of every target library #821

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

Conversation

keith-packard
Copy link
Collaborator

@keith-packard keith-packard commented Oct 14, 2024

This uses a new gcc config option, --enable-multilib-space, which duplicates every multilib configuration, adding
a new version which optimizes for space. With this, every library in the toolchain will get built twice and will be selected
depending on the options applied during the link phase. Add -Os or -Oz and you will get space-optimized versions of every toolchain library.

crosstool-ng PR: zephyrproject-rtos/crosstool-ng#26
GCC PR: zephyrproject-rtos/gcc#39
Picolibc PR: TBD

@abrodkin
Copy link
Collaborator

@keith-packard that feature looks awesome, and as usual only two questions:

  1. Any plans to get it submitted to the upstream Crosstool-NG?
  2. any chance to get that patch zephyrproject-rtos/gcc@34caa70 submitted to the GCC upstream?

@keith-packard
Copy link
Collaborator Author

@keith-packard that feature looks awesome, and as usual only two questions:

1. Any plans to get it submitted to the upstream Crosstool-NG?

Yup, once we know how this will work upstream in gcc, we can figure out how to get it merged to ct-ng.

2. any chance to get that patch [zephyrproject-rtos/gcc@34caa70](https://github.com/zephyrproject-rtos/gcc/commit/34caa704141c7ccde3ed2e0a7512ca636cb29bc5) submitted to the GCC upstream?

Yup, already submitted. https://gcc.gnu.org/pipermail/gcc-patches/2024-October/665446.html

@keith-packard
Copy link
Collaborator Author

keith-packard commented Oct 21, 2024

This is actually working when I disabled newlib-nano to keep the build times in check. The only testing failure occurred when a newlib-nano test slipped through; that should be fixed with zephyrproject-rtos/zephyr#80161.

Here's my best guess as to the space required for an x86_64 linux toolchain:

Only -Os    9GB
-Os+-O2     13GB

This includes -Os and -O2 versions of picolibc, newlib and newlib-nano along with libstdc++ compiled for each one. We could probably hack things up so that newlib was only provided in -O2 version while newlib-nano only in -Os; that would save two thirds of the increase measured above.

There isn't a great alternative here -- we can tell people to use the module when they need -O2, but that rules out using C++ with -O2.

When can we consider reducing the number of C library variants provided in the SDK?

@stephanosio
Copy link
Member

This is actually working when I disabled newlib-nano to keep the build times in check. The only testing failure occurred when a newlib-nano test slipped through; that should be fixed with zephyrproject-rtos/zephyr#80161.

I have cherry-picked that patch into the collab-sdk-dev branch for now so that this can be tested.

When can we consider reducing the number of C library variants provided in the SDK?

I think trying to completely remove newlib from the SDK is going to be met with a lot of backlashes.

Another option would be to remove newlib from the main SDK distribution and create a separate newlib "overlay," which can be downloaded and installed only if one needs it (similar to what LLVM Embedded Toolchain for Arm does).

@keith-packard
Copy link
Collaborator Author

This is actually working when I disabled newlib-nano to keep the build times in check. The only testing failure occurred when a newlib-nano test slipped through; that should be fixed with zephyrproject-rtos/zephyr#80161.

I have cherry-picked that patch into the collab-sdk-dev branch for now so that this can be tested.

Thanks.

I think trying to completely remove newlib from the SDK is going to be met with a lot of backlashes.

Yeah, was only thinking about newlib-nano.

Another option would be to remove newlib from the main SDK distribution and create a separate newlib "overlay," which can be downloaded and installed only if one needs it (similar to what LLVM Embedded Toolchain for Arm does).

That would save a huge amount of space, and should be fairly easy to implement.

configs/common.config Outdated Show resolved Hide resolved
@stephanosio
Copy link
Member

@keith-packard crosstool-ng PR has been merged. Can you please open a PR for GCC and picolibc (or do upstream sync for it)?

@keith-packard
Copy link
Collaborator Author

Will take care of this tomorrow.

Need to use BUILD_DIR instead of WORKSPACE as the latter hasn't been
set at this point.

Signed-off-by: Keith Packard <[email protected]>
Make sure we catch build failures.

Signed-off-by: Keith Packard <[email protected]>
Merge GCC version with multilib-space support which adds
-Os versions of every multilib configuration.

Signed-off-by: Keith Packard <[email protected]>
Make sure arc builds in -O2 mode successfully.

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

I've got the GCC PR queued. zephyrproject-rtos/picolibc main is sync'd with picolibc main to pull in fixes for arc, xtensa and add uchar.h

@stephanosio
Copy link
Member

Merged GCC PR.

It looks like I merged wrong crosstool-ng PR (though, I assume it is still valid?) and crosstool-ng submodule is still pointing to multlib-space branch.

Could you please open a PR for crosstool-ng as well?

This option passes --enable-multilib-space to gcc so that we
get -Os versions of every multilib configuration.

Signed-off-by: Keith Packard <[email protected]>
This provides -Os versions of every toolchain library.

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

Merged GCC PR.

It looks like I merged wrong crosstool-ng PR (though, I assume it is still valid?) and crosstool-ng submodule is still pointing to multlib-space branch.

Could you please open a PR for crosstool-ng as well?

zephyrproject-rtos/crosstool-ng#30

Note that the build tests are currently failing because Zephyr is defining values which are required to be present in limits.h (and which upstream picolibc now includes). zephyrproject-rtos/zephyr#80409

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

Successfully merging this pull request may close these issues.

3 participants