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

fix microbit1 compiler flags to clear bss section properly #2411

Merged
merged 1 commit into from
Sep 15, 2023

Conversation

fanoush
Copy link
Contributor

@fanoush fanoush commented Sep 8, 2023

CFLAGS are not actually passed to .S files, we need ASFLAGS for that

also there is easier way to use main() as _start() via -D__START=main - this also verifies flags are passed properly otherwise it fails in linker not finding _start

This can be merged as is for microbit1 however there are unclear issues with makefiles.

1.

nrf51 adds .o file to PRECOMPILED_OBJS here https://github.com/espruino/Espruino/blob/master/make/family/NRF51.make#L15 and this works fine, however nrf52 has .S files there in many places like here https://github.com/espruino/Espruino/blob/master/make/family/NRF52.make#L25 except here https://github.com/espruino/Espruino/blob/master/make/family/NRF52.make#L63

When I tried to put .S to nrf51 makefile instead of .o it did not pass ASFLAGS properly (nor CFLAGS) so the .o variant is a good one here.

However when I tried to add same lines

     'ASFLAGS += -D__STARTUP_CLEAR_BSS -D__START=main',
     'LDFLAGS += -nostartfiles',

to BANGLEJS board file it worked fine despite .S! Or maybe it uses the .o makefile variant here too (?)

Why there are .S files in PRECOMPILED_OBJS ?

And btw it saves about 300 bytes in BANGLE build - 1688b free vs 1380b free

2.

targetlibs/nrf5x_12/components/toolchain/gcc/gcc_startup_nrf5(2|1).o files are not cleared on make clean so if you try to build locally for two boards with different flags old .o file with different flags gets reused. Won't affect pipeline builds so can be merged for microbit1 despite possible other nrf51 builds not having the flag.

- also replace LD_NOSTARTFILES by __START=main
fanoush referenced this pull request Sep 14, 2023
@fanoush
Copy link
Contributor Author

fanoush commented Sep 14, 2023

targetlibs/nrf5x_12/components/toolchain/gcc/gcc_startup_nrf5(2|1).o files are not cleared on make clean

also reported here #2336 (comment)

@gfwilliams
Copy link
Member

Thanks! Sorry I missed this earlier. Looks great - merging now.

Why there are .S files in PRECOMPILED_OBJS ?

I'm not sure!

And btw it saves about 300 bytes in BANGLE build - 1688b free vs 1380b free

Doin gnostartfiles? Have you tested it? Part of me wonders whether as Tensorflow uses C++, it might need some of the stuff that's in there?

And I'll make sure those .o files get cleaned - thanks! I wonder if that's been affecting my firmware builds - luckily I guess most of the flags are the same usually

@gfwilliams gfwilliams merged commit 350b5c3 into espruino:master Sep 15, 2023
15 checks passed
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.

2 participants