A few SDK fixes from picolibc work #580
Conversation
By disabling compiler support, we can ensure that nothing else will end up using something that the OS cannot support. Signed-off-by: Keith Packard <[email protected]>
Make sure everything is built with the correct TLS configuration. Signed-off-by: Keith Packard <[email protected]>
From Zephyr arch/arm64/core/CMakeLists.txt: GCC may generate ldp/stp instructions with the Advanced SIMD Qn registers for consecutive 32-byte loads and stores. Saving and restoring the Advanced SIMD context is very expensive, and it is preferable to keep it turned off by not emitting these instructions for better context switching performance. Also, if these instructions happen from an ISR, they will trap and appear to leave interrupts disabled afterwards. That bug should be fixed. Signed-off-by: Keith Packard <[email protected]>
| @@ -4,7 +4,7 @@ CT_EXPERIMENTAL=y | |||
| # CT_LOG_PROGRESS_BAR is not set | |||
| CT_ARCH_ARC=y | |||
| CT_MULTILIB=y | |||
| CT_TARGET_CFLAGS="-mno-sdata" | |||
| CT_TARGET_CFLAGS="-mno-sdata -mtp-regno=26" | |||
There was a problem hiding this comment.
@claziss could you please comment on the register used for bare-metal TLS implementation on both ARCv2 & ARCv3? To me it looks like it's r25 at least on ARCv2.
There was a problem hiding this comment.
The register number itself is fine (I guess), we use r26 in zephyr itself: https://github.com/zephyrproject-rtos/zephyr/blob/9331591ab1f6a59513ded32d877f62dbf4526b9e/arch/arc/CMakeLists.txt#L15-L20I
But I'm wondering if it is OK to -mtp-regno=26 to the toolchain itself, given that we don't always use TLS in Zephyr.
There was a problem hiding this comment.
@keith-packard should we specify -mtp-regno=26 for picolibc only? we can make picolibc depend on TLS.
There was a problem hiding this comment.
Hi, TLS is default off for baremetal apps as they need to provide an environment for make it to work. Now, one can enable it or, if it is enabled, change the thread pointer (TP) register number using mtp-regno option. By default TP is set to r25, and it should work for other values as well, but I haven't test that.
There was a problem hiding this comment.
@keith-packard should we specify
-mtp-regno=26for picolibc only? we can make picolibc depend on TLS.
Hrm. Does the compiler reserve this register when specified like this so that other code doesn't use it? If so, then, yes, we need to specify this for the toolchain so that no other code built by the toolchain conflicts with potential TLS usage by Zephyr.
If the compiler doesn't reserve register 26, then does this parameter have any effect on code that doesn't use __thread variables? If not, then adding it unconditionally should not cause any issues.
In either case, I think we want to have the toolchain built with this parameter so that everything agrees on the TLS ABI.
There was a problem hiding this comment.
We've discussed this - let's use r25 in arc-zephyr-elf as tls to allign with other ARCv2 toolchains.
@evgeniy-paltsev sure, but can you please create a separate PR for this to sdk-ng and zephyr since it is quite a major change and not really something for the SDK 0.15.2?
I think all code linked together would need to have this specified to ensure it doesn't smash the TLS register -- I'd assume the compiler would be free to use it otherwise?
@keith-packard That would come with some performance implications when TLS is not enabled; but, I assume such a performance impact would be small enough.
Given that it probably is not going to be worth building separate multilib variants for non-TLS and TLS, sure.
There was a problem hiding this comment.
Given that it probably is not going to be worth building separate multilib variants for non-TLS and TLS, sure.
This raises a good point -- we should stick the -mtp-regno parameter into the multilib configuration -- that way a conflicting value will cause link failures instead of silent corruption. And, any pieces of the toolchain using the multilib configs (like newlib, picolibc, libstdc++) will automatically get the correct value here. That seems like a good way to keep the toolchain os-independent and ensure that Zephyr uses will work correctly.
There was a problem hiding this comment.
and, of course, would allow us to build multilib configurations that use r25 or r26 even, if the Arc team wants to transition.
There was a problem hiding this comment.
and, of course, would allow us to build multilib configurations that use r25 or r26 even, if the Arc team wants to transition.
Ideally yes, but ARC is one of the those platforms with already many multilib variants; adding r25 and r26 variants will multiply already many variants and significantly increase both the toolchain build time and size.
There was a problem hiding this comment.
I'm giving that a try to see how bad it gets. "only" multiplies the library builds for the toolchain by a factor of three ...
|
DNM until zephyrproject-rtos/gcc#17 is resolved. |
These are generally applicable fixes which address bugs in the current SDK: