cpu/msp430: update to modern gcc/ELF/newlib toolchain

[kaspar030]

Contribution description

This PR updates msp430 to a current gcc/newlib.

I used a self-compiled toolchain while developing this, but it turns out that the current TI gcc 8.3.0 toolchain works just fine. *Thus that's PR'ed at RIOT-OS/riotdocker#91.
Update: now using toolchains from https://github.com/RIOT-OS/toolchains/releases, which is also in our Docker container.

Until this is in riotdocker, the toolchain can be downloaded here. Only the gcc binary tarball is needed.

As expected, code size increases a lot. E.g., hello-world almost doubles (4k -> 8k ROM). That's probably mostly because newlibs stdio is much larger than the old TI one. Let's hope that picolibc will mitigate this...

The upside is that now msp430 can use C11, is re-using unmodified newlib-syscalls-default, and generally gets some support from upstream. Generally, many msp430 specific hacks are made obsolete.

The PR reverts some of the recent malloc updates to msp430, as they're not needed anymore.
This also includes a script to download the msp430-support-files (including headers and linkerscripts for every msp430 variant) that then removes all but the actually currently used files. That script should make adding more msp430 boards, or updating the support files, quite easy.

The PR does not yet remove all the blacklists that might not be necessary anymore, and it does not yet update insufficient memory lists that will probably get longer due to increased code size. I'm waiting for CI support to tackle those. done, all blacklists are updated.

Testing procedure

Test the shit out of any msp430 board.

Issues/PRs references

RIOT-OS/riotdocker#91
Partly fixes #3355
Fixes #8408.
Fixes all compilation errors and most tests found in #13267.

[aabadie]

Test the shit out of any msp430 board.

Can you give more details ?

[gschorcht]

The compilation of following tests failed with new gcc version in RIOT-OS/riotdocker#91 and this PR:

tests/gnrc_sock_udp
tests/heap_cmd
tests/libc_newlib
tests/nanocoap_cli
tests/pkg_fatfs
tests/pkg_microcoap

I tried it for wsn430-v1_3b.

[gschorcht]

tests/gnrc_sock_udp, tests/nanocoap_cli and tests/pkg_microcoap seem no to fit into the memory. MSP430 boards would have to be blacklisted for these tests.

[kaspar030]

tests/gnrc_sock_udp, tests/nanocoap_cli and tests/pkg_microcoap seem no to fit into the memory. MSP430 boards would have to be blacklisted for these tests.

Done. I've rebased to make use of the new Makefile.ci. @maribu, the flattened BOARD_INSUFFICIENT_MEMORY lists are so much nicer. 😉

[kaspar030]

tests/pkg_fatfs

This failes to compile here because of an sprintf overflow error. Fix in #12503.

[gschorcht]

tests/pkg_fatfs

This failes to compile here because of an sprintf overflow error. Fix in #12503.

Even it compiles now, I get the following error on linking:

/opt/msp430-gcc-8.3.0.16_linux64/bin/../lib/gcc/msp430-elf/8.3.0/../../../../msp430-elf/bin/ld: error: /data/riotbuild/tests/pkg_fatfs/bin/wsn430-v1_3b/cpu/msp430_stdio.o uses unknown instructions but /opt/msp430-gcc-8.3.0.16_linux64/bin/../lib/gcc/msp430-elf/8.3.0/../../../../msp430-elf/lib/430/crt0.o uses MSP430
/opt/msp430-gcc-8.3.0.16_linux64/bin/../lib/gcc/msp430-elf/8.3.0/../../../../msp430-elf/bin/ld: error: /data/riotbuild/tests/pkg_fatfs/bin/wsn430-v1_3b/cpu/msp430_stdio.o uses the unknown code model whereas /opt/msp430-gcc-8.3.0.16_linux64/bin/../lib/gcc/msp430-elf/8.3.0/../../../../msp430-elf/lib/430/crt0.o uses the small code model
/opt/msp430-gcc-8.3.0.16_linux64/bin/../lib/gcc/msp430-elf/8.3.0/../../../../msp430-elf/bin/ld: error: /data/riotbuild/tests/pkg_fatfs/bin/wsn430-v1_3b/cpu/msp430_stdio.o uses the unknown data model whereas /opt/msp430-gcc-8.3.0.16_linux64/bin/../lib/gcc/msp430-elf/8.3.0/../../../../msp430-elf/lib/430/crt0.o uses the small data model

Any idea? I have seen this for other tests too.

[kaspar030]

Even it compiles now, I get the following error on linking:

Now that's weird, I don't get that error with the same toolchain!?

[kaspar030]

Now that's weird, I don't get that error with the same toolchain!?

I tried building using a freshly built RIOT-OS/riotdocker#91, that also builds fine. Something is weird. I've seen that linking error, too, but don't remember where.

[gschorcht]

Now that's weird, I don't get that error with the same toolchain!?

I tried building using a freshly built RIOT-OS/riotdocker#91, that also builds fine. Something is weird. I've seen that linking error, too, but don't remember where.

I have build riotdocker from scratch and try to recompile now completely with your last force-pushed branch.

[kaspar030]

your last force-pushed branch.

sorry, I rebased to get #12503.

[gschorcht]

That's ok, I had to cherr pick PR #12503 before. I started complete recompilation by intention to see in which other tests I got this linking error.

[gschorcht]

Great, the complete rerun of compilation test in a fresh environment worked. There are no compilation or linking errors anymore.

[kaspar030]

I'm observing an issue with stdio.

The unittest "OK (N tests)" output doesn't show the N (number). Weirdly the number shows when adding printf("foo %d", 1234); (did that for testing). So something is wrong inside the syscalls.

[kaspar030]

I'm observing an issue with stdio.

Hm, with my self-compiled toolchain it works fine. Maybe we have to compile ourselves after all. :(

[gschorcht]

I'm observing an issue with stdio.

Hm, with my self-compiled toolchain it works fine. Maybe we have to compile ourselves after all. :(

Sounds depressing 😟

[gschorcht]

I'm observing an issue with stdio.

Hm, with my self-compiled toolchain it works fine. Maybe we have to compile ourselves after all. :(

Sounds depressing 😟

[kaspar030]

Sounds depressing worried

Yup. :)

embunit does this:

        stdimpl_print("\nOK (");                                                                                                   
        stdimpl_itoa(result_.runCount, buf, 10);                                                                                   
        stdimpl_print(buf);                                                                                                        
        stdimpl_print(" tests)\n");                                                                                                

stdimpl_print(x) is defined to printf("%s", x).

The resulting writes arrive re-ordered at newlib's _write_r(), even though they're from the same thread.

[kaspar030]

all dependencies are in.

• rebased, squashed (somewhat)

[maribu]

I don't really have hardware to test. But I honestly would argue to merge this as is, provided that everything compiles, as:

• This fixes a lot of issues and is a big step in the right direction
• It gets rid of many special cases / handling
• It will make our life much easier, as this is the last platform without C11 support. (The AVR compiler installed in the docker is also not yet C11 ready, but I locally use a modern GCC compiler for AVR without any issues...)
• This PR will not get easier to test and review over time. Instead, we should IMO just say this is a great step in the right direction and everything else can be easier addressed, reviewed and tested in follow up PRs.
• The support for MSP430 platform in master is in a pretty bade shape. If there are any active MSP430 users, the are used to suffering anyway ;-P

[fjmolinas]

I don't really have hardware to test. But I honestly would argue to merge this as is, provided that everything compiles, as:

• This fixes a lot of issues and is a big step in the right direction
• It gets rid of many special cases / handling
• It will make our life much easier, as this is the last platform without C11 support. (The AVR compiler installed in the docker is also not yet C11 ready, but I locally use a modern GCC compiler for AVR without any issues...)
• This PR will not get easier to test and review over time. Instead, we should IMO just say this is a great step in the right direction and everything else can be easier addressed, reviewed and tested in follow up PRs.
• The support for MSP430 platform in master is in a pretty bade shape. If there are any active MSP430 users, the are used to suffering anyway ;-P

+1, let me run tests on z1 before merging though that that the issue listing failing tests can be updated (but I think Kaspar took care of most already.)

[fjmolinas]

+1, let me run tests on z1 before merging though that that the issue listing failing tests can be updated (but I think Kaspar took care of most already.)

Results summary, and details

- [tests/driver_grove_ledbar](tests/driver_grove_ledbar/test.failed)
- [tests/driver_hd44780](tests/driver_hd44780/test.failed)
- [tests/driver_my9221](tests/driver_my9221/test.failed)
- [tests/periph_timer_short_relative_set](tests/periph_timer_short_relative_set/test.failed)

[fjmolinas]

Huge improvement compared to #13267, nothing unexpected is failing now.

[fjmolinas]

@maribu you have taken a closer look at the code do you want to tick the other labels and ACK?

[maribu]

ACK

[maribu]

Thanks @kaspar030! This was a big one :-)

[kaspar030]

Thanks a lot for reviewing!

[benpicco]

fyi: Since msp430 has newlib support, adding support to picolibc should be rather straightforward.

That should bring the ROM size down again.