Skip to content

cpu: cortexm: make newlib a default module#10767

Closed
kaspar030 wants to merge 3 commits intoRIOT-OS:masterfrom
kaspar030:cortexm_make_newlib_default_module
Closed

cpu: cortexm: make newlib a default module#10767
kaspar030 wants to merge 3 commits intoRIOT-OS:masterfrom
kaspar030:cortexm_make_newlib_default_module

Conversation

@kaspar030
Copy link
Contributor

Contribution description

Currently, newlib is always selected using USEMODULE for all Cortex-M.
This PR turns it into DEFAULT_MODULE. The advantage is that now it can be disabled on-demand using DISABLE_MODULE.

Testing procedure

CI compiling successfully should be fine.

Issues/PRs references

With #10744, this allows deployments without newlib altogether in some cases.

@kaspar030 kaspar030 added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jan 14, 2019
@kaspar030 kaspar030 requested review from cladmi and jnohlgard January 14, 2019 15:26
@jcarrano
Copy link
Contributor

To be more precise, this does really disable newlib, only the bindings to the system calls (which are not "system calls" in RIOT really.)

I'm trying to achieve the same, but in a more granular way, in #9258. Many applications will not be able to remove their dependencies on all syscalls, but they can still benefit from not including certain parts.

@kaspar030
Copy link
Contributor Author

To be more precise, this does really disable newlib

Yup. Useful for chopping off unused fat, or possibly using a slimmer libc, or ...

@cladmi
Copy link
Contributor

cladmi commented Jan 14, 2019

Can you give a testing procedure for the NEW possible usage?

@cladmi
Copy link
Contributor

cladmi commented Jan 14, 2019

If it should be used to select a different libc I would not do it with DEFAULT_MODULE.
libc should be in DEFAULT_MODULE, and newlib_nano would be the default implementation.

So I am interested at seeing the usage.

Also right now, when not setting newlib I think it would make the default C library being used asnewlib.inc.mk would not be included. Which feels different from the meaning of disabling newlib.

@kaspar030
Copy link
Contributor Author

Can you give a testing procedure for the NEW possible usage?

Add DISABLE_MODULE += newlib newlib_nano to tests/minimal/Makefile.
Then try compilation with master and with this branch for e.g., samr21-xpro.

On master, you'll get the usual "EXPECT ERRORS" warning. There even seems to be some effect on code size, but that must be accidental.

Size:

   text    data     bss     dec     hex filename
   3044      36    2604    5684    1634 /home/kaspar/src/riot/tests/minimal/bin/samr21-xpro/tests_minimal.elf

This is the output of BOARD=samr21-xpro make info-modules:

board
core
core_msg
cortexm_common
cortexm_common_periph
cpu
isrpipe
newlib
newlib_nano
newlib_syscalls_default
periph
periph_common
periph_gpio
periph_pm
periph_uart
pm_layered
sam0_common_periph
stdio_uart
sys
tsrb

Note newlib*, stdio_uart and periph_uart.

With this PR (and the same modified Makefile):

Size:

   text    data     bss     dec     hex filename
   2140      16    2524    4680    1248 /home/kaspar/src/riot/tests/minimal/bin/samr21-xpro/tests_minimal.elf

info-modules:

[kaspar@ng minimal (cortexm_make_newlib_default_module)]$ BOARD=samr21-xpro make info-modules
board
core
core_msg
cortexm_common
cortexm_common_periph
cpu
periph
periph_common
periph_gpio
periph_pm
pm_layered
sam0_common_periph
sys
[kaspar@ng minimal (cortexm_make_newlib_default_module)]$

Note the absence of newlib*, stdio_uart and periph_uart.

@kaspar030
Copy link
Contributor Author

Also right now, when not setting newlib I think it would make the default C library being used asnewlib.inc.mk would not be included. Which feels different from the meaning of disabling newlib.

Actually with our toolchains, newlib is the default. By disabling the newlib module, what actually happens is that RIOT's newlib support code (the syscalls) and their dependency don't get built/linked.
Which is useful for bare-metal stuff (e.g., riotboot).

libc should be in DEFAULT_MODULE, and newlib_nano would be the default implementation.

Agreed, but IMO that would be a next step.

@jcarrano
Copy link
Contributor

To be more precise, this does really disable newlib

Oh, OK, good to know. I think we need a test to show that a functioning application can be built without newlib, and that newlib itself is not being included (i.e. that the change is effective). For the second point, maybe verify that newlib init stuff is not the symbols in the elf file.

@kaspar030
Copy link
Contributor Author

I think we need a test to show that a functioning application can be built without newlib, and that newlib itself is not being included (i.e. that the change is effective). For the second point, maybe verify that newlib init stuff is not the symbols in the elf file.

I've added a commit disabling newlib for the bootloader.

This is before (on samr21-xpro):

   text    data     bss     dec     hex filename
   2544      24     672    3240     ca8 /home/kaspar/src/riot/bootloaders/riotboot/bin/samr21-xpro/riotboot.elf

This is after:

   text    data     bss     dec     hex filename
   1124       0     512    1636     664 /home/kaspar/src/riot/bootloaders/riotboot/bin/samr21-xpro/riotboot.elf                              

@basilfx can you give this a try on efm32?

@stale
Copy link

stale bot commented Aug 10, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

@stale stale bot added the State: stale State: The issue / PR has no activity for >185 days label Aug 10, 2019
@kaspar030 kaspar030 force-pushed the cortexm_make_newlib_default_module branch from ae7182a to cf16071 Compare August 19, 2019 21:06
@stale stale bot removed the State: stale State: The issue / PR has no activity for >185 days label Aug 19, 2019
@kaspar030 kaspar030 added CI: run tests If set, CI server will run tests on hardware for the labeled PR Platform: ARM Platform: This PR/issue effects ARM-based platforms labels Aug 19, 2019
@benpicco benpicco added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: run tests If set, CI server will run tests on hardware for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: run tests If set, CI server will run tests on hardware for the labeled PR labels Oct 28, 2019
@fjmolinas
Copy link
Contributor

@basilfx can you give this a try on efm32?

I have 1 efm32, what would you want me to test? just build riotboot? @kaspar030

@stale
Copy link

stale bot commented May 23, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

@stale stale bot added the State: stale State: The issue / PR has no activity for >185 days label May 23, 2020
@fjmolinas
Copy link
Contributor

ping @kaspar030 are you still interested in this one?

@stale stale bot removed the State: stale State: The issue / PR has no activity for >185 days label May 25, 2020
@stale
Copy link

stale bot commented Nov 26, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

@stale stale bot added the State: stale State: The issue / PR has no activity for >185 days label Nov 26, 2020
@stale stale bot closed this Dec 28, 2020
@miri64
Copy link
Member

miri64 commented Dec 28, 2020

ping @kaspar030 are you still interested in this one?

Obviously not...

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

Labels

CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: run tests If set, CI server will run tests on hardware for the labeled PR Platform: ARM Platform: This PR/issue effects ARM-based platforms State: stale State: The issue / PR has no activity for >185 days Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants