DNM: arch/arc: Let toolchain select tp-regno

[keith-packard]

Zephyr cannot unilaterally set the -mtp-regno option as any libraries used from the toolchain must be built with the same option. This means that builds enabling thread local storage will fail when using a toolchain which does not specify a tp-regno value. That is better than potentially using toolchain library code which overwrites the selected register.

Signed-off-by: Keith Packard [email protected]

See also zephyrproject-rtos/gcc#17 which makes the Zephyr SDK use register 25 by default.

[ruuddw]

There are many compiler settings that need to be consistent for zephyr kernel and libraries, not just the tls one. Don't see why this needs to be handled differently. This change breaks TLS support on ARC.

[stephanosio]

There are many compiler settings that need to be consistent for zephyr kernel and libraries, not just the tls one.

The idea is that the future releases of Zephyr SDK will automatically use/reserve r25 as the TLS pointer register by default even if -mtp-regno=25 (or -mtp-regno=26 if we decide to not change the regno) is not specified, so this flag will technically be no longer needed.

But, there will be a problem if you are using an ARC toolchain other than the one from Zephyr SDK; for instance, GNU ARC toolchain from Synopsys or even Metaware assuming it also requires the flag?

A simpler alternative would be to just leave this flag in and update the Zephyr SDK ARC toolchain to default to r26 (to ensure that the TLS pointer register does not get clobbered). Specifying -mtp-regno=26 here should not be a problem in that case.

The patch for normalising the TLS pointer register from r26 (some random reg that Zephyr uses) to r25 (Linux default) can be submitted by the ARC maintainers later if they want to do this.

[ruuddw]

Sorry, I still don't understand what's special about this specific option. There are many options that need to be set consistently when building libraries used in Zephyr, we don't rely on toolchain defaults for these either.

Probably best to discuss in a toolchain meeting with @abrodkin and @evgeniy-paltsev

[keith-packard]

Sorry, I still don't understand what's special about this specific option. There are many options that need to be set consistently when building libraries used in Zephyr, we don't rely on toolchain defaults for these either.

Building the toolchain without defining the TP register number means any code delivered with the toolchain (libc, libstdc++, libgcc) could end up using the register for its own purposes. Most of the time, this isn't an issue as the register will be restored upon return, but if Zephyr code is called while library code is using this register, and that Zephyr code accesses thread-local variables, then that code will fail. I wonder, for instance, whether interrupt handlers might expect this register to reflect the current context? Callbacks (for stdio), overloaded virtual functions (for libstdc++) are also potential sources of adventure here.

Linux for Arc V2 sets the thread register to 25 in the toolchain to resolve this issue. Arc V3 reserves r30 for this.

Anything which affects ABI must be consistent between libraries delivered with the toolchain and those built in the application; if you have other options in this class, they should either be fixed in the toolchain or configured as multilib options (like -mcpu).

[evgeniy-paltsev]

@keith-packard
for me the implementation looks pretty simple, I'll comment the details in a few days (I'm little busy right now with other stuff, sorry).

[keith-packard]

@keith-packard for me the implementation looks pretty simple, I'll comment the details in a few days (I'm little busy right now with other stuff, sorry).

no hurry; @stephanosio suggested we change the toolchain to use r26 so that the existing Zephyr code will 'just work'. This change will enable moving from r26 to r25 in the future, if the Arc community wants to do that.

[evgeniy-paltsev]

@keith-packard , @stephanosio , @ruuddw , @abrodkin

Sorry for delay, here is my proposal:

For ARCv3 toolchain (arc64-zephyr-elf):

Nothing need to be done in both sdk-ng and Zephyr, r30 is reserved by default for tls pointer

For ARCv2 toolchain (arc-zephyr-elf):

Currently we use r26 in Zephyr and there is no register for tls pointer is selected when we build toolchain libraries. For other ARCv2 toolchains where we reserve register for tls pointer the r25 is used.

So, let's specify default tls registers for ARCv2 toolchain and let's switch to r25 to align it with other ARCv2 toolchains. For that:

• Specify tls register on Zephyr toolchain configuration step with -mtp-regno=25
• Don't specify -mtp-regno=26 on Zephyr side:

  zephyr/arch/arc/CMakeLists.txt

  Line 19 in dbd2176

  zephyr_cc_option_ifdef(CONFIG_THREAD_LOCAL_STORAGE -mtp-regno=26)

  
  Implemented by this PR: DNM: arch/arc: Let toolchain select tp-regno #51910
• [DONE] Put tls pointer to proper register on new thread setup. Implemented by arch/arc: Detect arcv2 TLS register automatically #51922
• Drop arc: Set tp regno to 26 for Zephyr gcc#17 PR. As we can do it without changes in GCC by simple toolchain configuration tweak - let's do it that way. We prefer to not have extra patches for GCC for ARC in Zephyr GCC fork which we don't upstream to GCC.
• In Zephyr set minimum SDK version to 0.15.2 for ARC.

This won't introduce new compatibility issues for main Zephyr branch as:

• Currently we allow TLS only for Zephyr toolchain in case of ARC. For any external gcc provided as cross-compile toolchain TLS is disabled. For ARC MWDT we don't enable it either.

This won't introduce new compatibility issues for LTS Zephyr branch (even if de-facto we don't support building LTS branch with latest Zephyr SDKs) as:

• In LTS we still specify -mtp-regno=26 in Zephyr itself which will override toolchain defaults.
• We still don't reserve r26 register for TLS pointer in toolchain libraries - like in old Zephyr toolchains. It's an issue wit it isn't a new issue we introduce.

This will fix currently existing issue that we build ARCv2 libraries in Zephyr toolchain without reservation register for TLS pointer, so currently r26 can be used by libraries. We haven't bumped into this issue, but it's just pure luck.

Actions:

If there is no objections I'll prepare PR's for the rest of the changes.

[keith-packard]

* Drop [arc: Set tp regno to 26 for Zephyr gcc#17](https://github.com/zephyrproject-rtos/gcc/pull/17) PR. As we can do it without changes in GCC by simple toolchain configuration tweak - let's do it that way. We prefer to not have extra patches for GCC for ARC in Zephyr GCC fork which we don't upstream to GCC.

I think this is the key patch -- it removes any need for all compilation uses to ensure they set -mtp-regno=25 on the command line. Without this patch, any mis-configured component may silently damage the thread pointer register and lead to unexpected failures.

Once this patch is in place, there is only one thing that needs changing to switch from 26 to 25 -- change this value and then ensure that a consistent SDK is used to build all the pieces of the system.

As to submitting upstream, I think we could find a way to do that, perhaps by adding another arc-specific GCC configuration value to set this, or plan on making this a part of the switch from arc-zephyr-elf to arc-none-zephyr (or however we want to name the toolchain) as that would allow us to create a zephyr.h file in gcc to include this kind of change.

[evgeniy-paltsev]

Hi @keith-packard,

As to submitting upstream, I think we could find a way to do that, perhaps by adding another arc-specific GCC configuration value to set this

The initial reason why it wasn't specified for ARC baremetal toolchain is because baremetal targets wouldn't use TLS anyway - so reservation register for TLS is a register waste. The Zephyr is kinda exception here - probably it's not worth to tweak generic baremetal toolchain configuration because of Zephyr only.

---

Ideally we should only build toolchain libraries with -mtp-regno=** - so if we don't use TLS the Zephyr and application code will be build without -mtp-regno=**. It would be useful for tiny targets to not waste one register for nothing.

Not an expert in crosstool-ng, so not sure if CT_TARGET_CFLAGS are only used for target libraries building or they become also default GCC flagst, so I need to check. Any comments about that are appreciated :)

[keith-packard]

The initial reason why it wasn't specified for ARC baremetal toolchain is because baremetal targets wouldn't use TLS anyway - so reservation register for TLS is a register waste. The Zephyr is kinda exception here - probably it's not worth to tweak generic baremetal toolchain configuration because of Zephyr only.

Note that RISC-V has made the other choice -- all RISC-V ABIs reserve a register for thread local storage. But, I don't know the ARC ecosystem at all, so perhaps having another register is more useful there? I can see how setting this in every embedded toolchain wouldn't be welcome by all users. Making it a arc-specific GCC configuration option would make a lot of sense though, and aligning the embedded tp register number with the linux choice would also be a good plan.

Ideally we should only build toolchain libraries with -mtp-regno=** - so if we don't use TLS the Zephyr and application code will be build without -mtp-regno=**. It would be useful for tiny targets to not waste one register for nothing.

The alternative is to have targets that do want an extra register to use -mtp-regno=none while building. The only effect of having -mtp-regno=** has is to make that the default when building code. Applications can always override that by turning that feature off.

Not an expert in crosstool-ng, so not sure if CT_TARGET_CFLAGS are only used for target libraries building or they become also default GCC flagst, so I need to check. Any comments about that are appreciated :)

No, there's nothing taken from crosstool-ng into applications in the way of compiler flags.

Note that as far as the toolchain build goes, the only effect of having this set there is that code delivered with the toolchain will use this setting -- that's just libgcc and libc, which aren't generally performance critical bits of code for embedded applications.

The problem is that if any code is built without the correct -mtp-regno parameter, then it might cause corruption in subtle ways. So, having the toolchain use a default value is an easy way to avoid that by just removing the command line parameter everywhere, and letting applications not using TLS to use -mtp-regno=none to gain back that extra register.

Having the toolchain and Zephyr use matching configurations should work, but I feel it's a bit more fragile as anything which somehow neglects to include this command line parameter could cause weird bugs.

[github-actions]

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

[keith-packard]

It seems like the arc team wants to leave this as-is.