Add newlib-nano as multilib #60
Conversation
This is meant to reflect what ARM GCC provides when compiling with --specs=nano.specs
newlib was not building correctly for ARMv7 and ARMv8 targets due to not being passed the -march, -mfpu and -mfloat-abi flags which are required for the compiler to generate correct assembly (it was outputing incorrect assembly without triggering any warnings)
stevew817
force-pushed
the
feature/newlib-nano
branch
from
4435bff to
c66cd73
Compare
|
Hi Stephen, Thank you for working on this and posting the patch! I was playing with the changes, I can confirm that with this patch I can build the Surprisingly, I am not able to build the normal newlib package anymore: |
|
Thanks for the swift feedback!
I don't think so - according to the readme for newlib, enabling
I'll admit I didn't retry building the regular newlib locally (as each build run takes about four hours for me... I do need a new machine). Since I didn't change any of the newlib config flags, it's probably due to now passing the actual architecture and architectural option flags to the compiler? Before I spend hours rebuilding newlib from scratch, so you have any more information on the failure (such as line number etc)? |
|
Please see the |
|
LLVM libc++ seems to have this option to turn wide characters support on/off: https://github.com/llvm/llvm-project/blob/83433d936195c612a51b54397f82ab0d97369d86/libcxx/CMakeLists.txt#L98 |
Thanks! Is it only on AArch64 or does it also appear on ARM? |
|
I rerun the build, apparently it tries to build the first configuration which is When I try to build armv6m and armv8.m-main separately, the build succeeds, so likely the above is an AArch64 specific issue. |
Building newlibe with --enable-newlib-nano-formatted-io removes all support for wchar_t functions from the C libary, meaning that the CPP library on top also needs to not rely on the wchar variants.
It's incompatible, see https://gitlab.arm.com/tooling/gnu-devtools-for-arm/-/blob/main/build-baremetal-toolchain.sh#L624 Instead, ship a size-optimized build of regular newlib as the 'newlib-nano' multilib for AArch64.
Forces the build system to keep them separate and keeps us free from having to think about reconfiguration issues when switching between library builds.
|
Sorry for the delayed response, I was out on vacation. After digging back into this, I came to the following:
@voltur01 can you check the changes I just pushed? |
|
Hi Steven, Thank you for the updated patch! I can confirm that both Code change look good as well. As you rightly pointed out, the If you take the |
And newlib insists on using them for any AArch64 target. So for now don't build newlib for AArch64 with software-disabled FPU.
stevew817
force-pushed
the
feature/newlib-nano
branch
from
115ff22 to
6212152
Compare
|
Dropped the libc++ change from the PR as requested, and hopefully llvm/llvm-project#126924 will land instead. If it doesn't, we'll have to do a workaround similar to picolibc where we patch newlib with an extra header in |
|
Thank you! I will approve and merge now. |
See ARM-software/LLVM-embedded-toolchain-for-Arm#628 for context. This enables newlib-nano as a multilib. I didn't find any release scripts, so I don't know how to add it to the CI such that it also generates an overlay package for newlib-nano on release. Note: this also fixes an issue with the current newlib builds where they don't get built correctly for anything above ARMv6 as the `-march`, `-mfloat-abi` and `-mfpu` arguments weren't being passed on to the newlib compilation configuration. A little unsure why this hasn't popped up before...
See ARM-software/LLVM-embedded-toolchain-for-Arm#628 for context. This enables newlib-nano as a multilib. I didn't find any release scripts, so I don't know how to add it to the CI such that it also generates an overlay package for newlib-nano on release. Note: this also fixes an issue with the current newlib builds where they don't get built correctly for anything above ARMv6 as the `-march`, `-mfloat-abi` and `-mfpu` arguments weren't being passed on to the newlib compilation configuration. A little unsure why this hasn't popped up before...
#314 should have only enabled aarch64 nofp variants for LLVM-libc builds. However it also enabled the variants for newlib builds, which were disabled since newlib is similarly unable to build those variants (see #60 (comment)). While there is now a fix for the LLVM-libc build, there is yet no fix for newlib so they need to remain disabled for that selection. This patch sets the variants to only build for picolibc and llvmlibc, but not newlib.
PR #60, which introduced it, set the build flags `-g -Oz` for newlib-nano (both libc and libcxx), and `-g -O2` for full newlib libcxx. `-g` was probably left over from development: in our production builds it's taking up a great deal of space, making the newlib-nano overlay archive more like ¾Gb than ¼Gb. I've reverted the entire flags change for full newlib libcxx, putting it back to how it was before that PR. For newlib-nano, I've removed the `-g`, but kept `-Oz`, on the basis that optimizing for small size still seems sensible if someone's deliberately selecting the smallest version of newlib.
… >= 4G (#151460)" This reverts commit 600976f. The test was crashing trying to access any element of the GPR struct. (gdb) disas Dump of assembler code for function _ZN7testing8internal11CmpHelperEQIyyEENS_15AssertionResultEPKcS4_RKT_RKT0_: 0x00450afc <+0>: push {r4, r5, r6, r7, r8, r9, r10, r11, lr} 0x00450b00 <+4>: sub sp, sp, #60 ; 0x3c 0x00450b04 <+8>: ldr r5, [sp, #96] ; 0x60 => 0x00450b08 <+12>: ldm r3, {r4, r7} 0x00450b0c <+16>: ldm r5, {r6, r9} 0x00450b10 <+20>: eor r7, r7, r9 0x00450b14 <+24>: eor r6, r4, r6 0x00450b18 <+28>: orrs r7, r6, r7 (gdb) p/x r3 $3 = 0x3e300f6e "However, load and store multiple instructions (LDM and STM) and load and store double-word (LDRD or STRD) must be aligned to at least a word boundary." https://developer.arm.com/documentation/den0013/d/Porting/Alignment >>> 0x3e300f6e % 4 2 Program received signal SIGBUS, Bus error. 0x00450b08 in testing::AssertionResult testing::internal::CmpHelperEQ<unsigned long long, unsigned long long>(char const*, char const*, unsigned long long const&, unsigned long long const&) () The struct is packed with 1 byte alignment, but it needs to start at an aligned address for us to ldm from it. So I've done that with alignas. Also fixed some compiler warnings in the test itself.
…RM64` if PC >= 4G (#151460)" This reverts commit 600976f. The test was crashing trying to access any element of the GPR struct. (gdb) disas Dump of assembler code for function _ZN7testing8internal11CmpHelperEQIyyEENS_15AssertionResultEPKcS4_RKT_RKT0_: 0x00450afc <+0>: push {r4, r5, r6, r7, r8, r9, r10, r11, lr} 0x00450b00 <+4>: sub sp, sp, #60 ; 0x3c 0x00450b04 <+8>: ldr r5, [sp, #96] ; 0x60 => 0x00450b08 <+12>: ldm r3, {r4, r7} 0x00450b0c <+16>: ldm r5, {r6, r9} 0x00450b10 <+20>: eor r7, r7, r9 0x00450b14 <+24>: eor r6, r4, r6 0x00450b18 <+28>: orrs r7, r6, r7 (gdb) p/x r3 $3 = 0x3e300f6e "However, load and store multiple instructions (LDM and STM) and load and store double-word (LDRD or STRD) must be aligned to at least a word boundary." https://developer.arm.com/documentation/den0013/d/Porting/Alignment >>> 0x3e300f6e % 4 2 Program received signal SIGBUS, Bus error. 0x00450b08 in testing::AssertionResult testing::internal::CmpHelperEQ<unsigned long long, unsigned long long>(char const*, char const*, unsigned long long const&, unsigned long long const&) () The struct is packed with 1 byte alignment, but it needs to start at an aligned address for us to ldm from it. So I've done that with alignas. Also fixed some compiler warnings in the test itself.
2 participants
See ARM-software/LLVM-embedded-toolchain-for-Arm#628 for context.
This enables newlib-nano as a multilib. I didn't find any release scripts, so I don't know how to add it to the CI such that it also generates an overlay package for newlib-nano on release.
Note: this also fixes an issue with the current newlib builds where they don't get built correctly for anything above ARMv6 as the
-march,-mfloat-abiand-mfpuarguments weren't being passed on to the newlib compilation configuration. A little unsure why this hasn't popped up before...