Skip to content

A few SDK fixes from picolibc work #580

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

A few SDK fixes from picolibc work #580

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2c286d1
Disable TLS for GCC on targets with no Zephyr support
keith-packard Oct 18, 2022
56c5427
arc: Add -mtp-regno=26 to CFLAGS
keith-packard Oct 18, 2022
73cddd2
aarch64: Add -moverride=tune=no_ldp_stp_qregs to TARGET_CFLAGS
keith-packard Oct 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions configs/aarch64-zephyr-elf.config
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ CT_ARCH_ARM=y
CT_MULTILIB=y
CT_ARCH_64=y
CT_TARGET_VENDOR="zephyr"
CT_TARGET_CFLAGS="-moverride=tune=no_ldp_stp_qregs"
CT_BINUTILS_SRC_CUSTOM=y
CT_BINUTILS_CUSTOM_LOCATION="${GITHUB_WORKSPACE}/binutils"
CT_NEWLIB_SRC_CUSTOM=y
Expand Down
2 changes: 1 addition & 1 deletion configs/arc-zephyr-elf.config
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Collaborator

@evgeniy-paltsev evgeniy-paltsev Oct 26, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@keith-packard should we specify -mtp-regno=26 for picolibc only? we can make picolibc depend on TLS.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@keith-packard should we specify -mtp-regno=26 for 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.

Copy link
Member

@stephanosio stephanosio Nov 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and, of course, would allow us to build multilib configurations that use r25 or r26 even, if the Arc team wants to transition.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 ...

CT_TARGET_VENDOR="zephyr"
CT_BINUTILS_SRC_CUSTOM=y
CT_BINUTILS_CUSTOM_LOCATION="${GITHUB_WORKSPACE}/binutils"
Expand Down
1 change: 1 addition & 0 deletions configs/mips-zephyr-elf.config
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ CT_LIBC_NEWLIB_EXTRA_SECTIONS=y
CT_GCC_SRC_CUSTOM=y
CT_GCC_CUSTOM_LOCATION="${GITHUB_WORKSPACE}/gcc"
CT_CC_GCC_EXTRA_CONFIG_ARRAY="--with-gnu-ld --with-gnu-as --enable-initfini-array"
CT_CC_GCC_CONFIG_TLS=n
CT_CC_LANG_CXX=y
CT_DEBUG_GDB=y
CT_GDB_SRC_CUSTOM=y
Expand Down
1 change: 1 addition & 0 deletions configs/nios2-zephyr-elf.config
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ CT_LIBC_NEWLIB_EXTRA_SECTIONS=y
CT_GCC_SRC_CUSTOM=y
CT_GCC_CUSTOM_LOCATION="${GITHUB_WORKSPACE}/gcc"
CT_CC_GCC_EXTRA_CONFIG_ARRAY="--with-gnu-ld --with-gnu-as --enable-initfini-array"
CT_CC_GCC_CONFIG_TLS=n
CT_CC_LANG_CXX=y
CT_DEBUG_GDB=y
CT_GDB_SRC_CUSTOM=y
Expand Down
1 change: 1 addition & 0 deletions configs/xtensa-espressif_esp32_zephyr-elf.config
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ CT_LIBC_NEWLIB_EXTRA_SECTIONS=y
CT_GCC_SRC_CUSTOM=y
CT_GCC_CUSTOM_LOCATION="${GITHUB_WORKSPACE}/gcc"
CT_CC_GCC_EXTRA_CONFIG_ARRAY="--with-gnu-ld --with-gnu-as --enable-initfini-array"
CT_CC_GCC_CONFIG_TLS=n
CT_CC_LANG_CXX=y
CT_DEBUG_GDB=y
CT_GDB_SRC_CUSTOM=y
Expand Down
1 change: 1 addition & 0 deletions configs/xtensa-espressif_esp32s2_zephyr-elf.config
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ CT_LIBC_NEWLIB_EXTRA_SECTIONS=y
CT_GCC_SRC_CUSTOM=y
CT_GCC_CUSTOM_LOCATION="${GITHUB_WORKSPACE}/gcc"
CT_CC_GCC_EXTRA_CONFIG_ARRAY="--with-gnu-ld --with-gnu-as --enable-initfini-array"
CT_CC_GCC_CONFIG_TLS=n
CT_CC_LANG_CXX=y
CT_DEBUG_GDB=y
CT_GDB_SRC_CUSTOM=y
Expand Down
1 change: 1 addition & 0 deletions configs/xtensa-intel_apl_adsp_zephyr-elf.config
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ CT_LIBC_NEWLIB_EXTRA_SECTIONS=y
CT_GCC_SRC_CUSTOM=y
CT_GCC_CUSTOM_LOCATION="${GITHUB_WORKSPACE}/gcc"
CT_CC_GCC_EXTRA_CONFIG_ARRAY="--with-gnu-ld --with-gnu-as --enable-initfini-array"
CT_CC_GCC_CONFIG_TLS=n
CT_CC_LANG_CXX=y
CT_DEBUG_GDB=y
CT_GDB_SRC_CUSTOM=y
Expand Down
1 change: 1 addition & 0 deletions configs/xtensa-intel_bdw_adsp_zephyr-elf.config
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ CT_LIBC_NEWLIB_EXTRA_SECTIONS=y
CT_GCC_SRC_CUSTOM=y
CT_GCC_CUSTOM_LOCATION="${GITHUB_WORKSPACE}/gcc"
CT_CC_GCC_EXTRA_CONFIG_ARRAY="--with-gnu-ld --with-gnu-as --enable-initfini-array"
CT_CC_GCC_CONFIG_TLS=n
CT_CC_LANG_CXX=y
CT_DEBUG_GDB=y
CT_GDB_SRC_CUSTOM=y
Expand Down
1 change: 1 addition & 0 deletions configs/xtensa-intel_byt_adsp_zephyr-elf.config
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ CT_LIBC_NEWLIB_EXTRA_SECTIONS=y
CT_GCC_SRC_CUSTOM=y
CT_GCC_CUSTOM_LOCATION="${GITHUB_WORKSPACE}/gcc"
CT_CC_GCC_EXTRA_CONFIG_ARRAY="--with-gnu-ld --with-gnu-as --enable-initfini-array"
CT_CC_GCC_CONFIG_TLS=n
CT_CC_LANG_CXX=y
CT_DEBUG_GDB=y
CT_GDB_SRC_CUSTOM=y
Expand Down
1 change: 1 addition & 0 deletions configs/xtensa-intel_s1000_zephyr-elf.config
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ CT_LIBC_NEWLIB_EXTRA_SECTIONS=y
CT_GCC_SRC_CUSTOM=y
CT_GCC_CUSTOM_LOCATION="${GITHUB_WORKSPACE}/gcc"
CT_CC_GCC_EXTRA_CONFIG_ARRAY="--with-gnu-ld --with-gnu-as --enable-initfini-array"
CT_CC_GCC_CONFIG_TLS=n
CT_CC_LANG_CXX=y
CT_DEBUG_GDB=y
CT_GDB_SRC_CUSTOM=y
Expand Down
1 change: 1 addition & 0 deletions configs/xtensa-nxp_imx8m_adsp_zephyr-elf.config
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ CT_LIBC_NEWLIB_EXTRA_SECTIONS=y
CT_GCC_SRC_CUSTOM=y
CT_GCC_CUSTOM_LOCATION="${GITHUB_WORKSPACE}/gcc"
CT_CC_GCC_EXTRA_CONFIG_ARRAY="--with-gnu-ld --with-gnu-as --enable-initfini-array"
CT_CC_GCC_CONFIG_TLS=n
CT_CC_LANG_CXX=y
CT_DEBUG_GDB=y
CT_GDB_SRC_CUSTOM=y
Expand Down
1 change: 1 addition & 0 deletions configs/xtensa-nxp_imx_adsp_zephyr-elf.config
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ CT_LIBC_NEWLIB_EXTRA_SECTIONS=y
CT_GCC_SRC_CUSTOM=y
CT_GCC_CUSTOM_LOCATION="${GITHUB_WORKSPACE}/gcc"
CT_CC_GCC_EXTRA_CONFIG_ARRAY="--with-gnu-ld --with-gnu-as --enable-initfini-array"
CT_CC_GCC_CONFIG_TLS=n
CT_CC_LANG_CXX=y
CT_DEBUG_GDB=y
CT_GDB_SRC_CUSTOM=y
Expand Down
1 change: 1 addition & 0 deletions configs/xtensa-sample_controller_zephyr-elf.config
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ CT_LIBC_NEWLIB_EXTRA_SECTIONS=y
CT_GCC_SRC_CUSTOM=y
CT_GCC_CUSTOM_LOCATION="${GITHUB_WORKSPACE}/gcc"
CT_CC_GCC_EXTRA_CONFIG_ARRAY="--with-gnu-ld --with-gnu-as --enable-initfini-array"
CT_CC_GCC_CONFIG_TLS=n
CT_CC_LANG_CXX=y
CT_DEBUG_GDB=y
CT_GDB_SRC_CUSTOM=y
Expand Down