Skip to content
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.

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

C++ Roadmap #45785

Open
1 of 8 tasks
stephanosio opened this issue May 19, 2022 · 15 comments
Open
1 of 8 tasks

C++ Roadmap #45785

stephanosio opened this issue May 19, 2022 · 15 comments
Assignees
Labels
area: C++ Feature A planned feature with a milestone Meta A collection of features, enhancements or bugs

Comments

@stephanosio
Copy link
Member

stephanosio commented May 19, 2022

This issue describes the future plans for the C++ support in the Zephyr RTOS.

Tasks

Resources

@stephanosio stephanosio added Feature A planned feature with a milestone RFC Request For Comments: want input from the community Meta A collection of features, enhancements or bugs labels May 19, 2022
@stephanosio stephanosio self-assigned this May 19, 2022
@stephanosio stephanosio removed the RFC Request For Comments: want input from the community label May 19, 2022
@stephanosio
Copy link
Member Author

cc @cfriedt @yperess @alexanderwachter

@XenuIsWatching
Copy link
Member

any reason why std::condition_variable, std::counting_semaphore, and std::binary_semaphore isn't included?

@cfriedt
Copy link
Member

cfriedt commented Jun 23, 2022

any reason why std::condition_variable, std::counting_semaphore, and std::binary_semaphore isn't included?

Good call - definitely on the roadmap. I updated the description slightly. A lot of things we would get "for free" with #43729.

Big blocker for std::thread (at least dynamically) is #44397 (and having time to work on upstream)

@aborisovich
Copy link
Collaborator

Might be a chance to review existing C style rules.
I am talking about positioning "*" on the "right side" of the type definition.
For C nobody cares where it is positioned (shamely), in C++ it starts to make more sense with introduction of reference types.
Basically it should be:
image
When & and * characters are "glued to the left" side during type declaration
(following are standard rules - note how other specifiers behave - everything type related to the left then variable name on the right, we don't do like in the line 0 here:
image

C++ brings more to typing on the table. Please consider this change it really makes sense.

@stephanosio
Copy link
Member Author

As far as C is concerned, int * is the only correct form because:

int *a, *b;

For C++, we could potentially adopt int* and ban multiple variable declarations in one line.

@aborisovich
Copy link
Collaborator

aborisovich commented Jul 10, 2022

As far as C is concerned, int * is the only correct form because:

int *a, *b;

For C++, we could potentially adopt int* and ban multiple variable declarations in one line.

I've just read about it... It is so "broken". Both in C and C++...
Existing style makes some sense then.
Seems like this "problem" carries over to multiple reference types inline declaration.
int a = 1, b = 2, c = 3;
int& c = a, &d = b, &e = c;
However I've never seem this syntax in C++ production code until @stephanosio brought up this subject.
And I think I've rarely seen anyone declaring reference types like
int &a = b;
Or going further:
int &&a = b;
Maybe I'm too young and just haven't seen enough in my life but it seems to me that the very same problem exists in C++ and it is mostly resolved by "don't use that confusing many inline syntax"...
I still believe that my previous post brings more intuitive code at the only cost of that multiple inline definition.

@dberlin
Copy link
Contributor

dberlin commented Aug 12, 2022

Just to try to help a little i can confirm that hacking up the c++conf.h and gthr.h for the compiler is sufficient to make shared_mutex/condition_variable/etc compile fine without changes.
In particular, c++conf.h (for the toolchains in zephyr-sdk 0.14.2) just needs needs:

#define _GTHREAD_USE_MUTEX_INIT_FUNC 1
#define _GTHREAD_USE_COND_INIT_FUNC 1

(to not use the initializers which zephyr doesn't support)

and gthr.h needs:

#if _GLIBCXX___ANDROID__ || CONFIG_PTHREAD_IPC
#include <bits/gthr-posix.h>
#else
#include <bits/gthr-default.h>
#endif

(I have no earthly idea why it's gated on ANDROID and not a GLIBCXX__PTHREADs macro or something)

to use gthr-posix.h, you need
`CONFIG_PTHREAD_IPC=y

CONFIG_POSIX_API=y

CONFIG_APP_LINK_WITH_POSIX_SUBSYS=y
`

With these changes, the compiler will compile shared_mutex/etc fine.
(it appears CONFIG_PTHREAD_IPC should force CONFIG_POSIX_API to 1, but it doesn't)

Note that condition_variable (and i suspect other stuff) compiles but doesn't link.

This is because all of this requires the libstc++ be compiled with threading support (and the config change), and at least the ones in zephyr-sdk (and my NRF sdk) are compiled both single threaded and obviously without the gthr changes.
The above changes really have to be made in the right toolchain configure bits/etc, and the toolchains changed to enable threads.

It's possible but a bad idea to do it without changing the threading model on the toolchains:
I was a GCC/LLVM hacker for 10+ years, and saw it done, but it is a really bad idea:
https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_concurrency.html#manual.intro.using.concurrency.thread_safety

agrees, saying:
"multithreaded C++ application are only supported when libstdc++ and all user code was built with compilers which report (via gcc/g++ -v ) the same thread model and that model is not single."

@dberlin
Copy link
Contributor

dberlin commented Aug 13, 2022

If you'd like to play around with a working mutex/condvar/shared_mutex, the following will let you do it.
You have to set the POSIX options i mention in prj.cnf.

Then unzip
stl_threaded.ZIP
into the project root, and then add the following to CMakeLists.txt

FILE(GLOB stl_threaded_sources lib/stl_threaded/*.cpp lib/stl_threaded/*.c)
target_sources(app PRIVATE ${stl_threaded_sources})
target_include_directories(app BEFORE PUBLIC "lib/stl_threaded")

This will ensure the bits/c++config.h changes override the toolchain ones.
The condition_var/mutex source is just the correspnding ones for the toolchain from the sdk-ng project.

I took a quick pass at hacking the toolchain to enable threads (IE CT_THREADS_POSIX=y) and change the os defines, but i'm still working on getting the environment to be the same as the CI workflow uses (I tried using act to execute it locally, and even if i revert back to pre-s3-download versions, still no luck).

@dberlin
Copy link
Contributor

dberlin commented Aug 14, 2022

So, i got the toolchain to build normally (yay). Finally all my years of managing the crosstool team come in handy.

Unfortunately, enabling threads doesn't "just work", as might be expected. It gets pretty far but eventually fails trying to compile emutls with the final toolchain due to some errors.

Here's a preliminary checklist of work needed to make it work:

  • The crosstool configs need to be updated. The config commands y'all execute are actually warning you to run upgradeconfig on them. It took me a while to figure out why some of my options just disappeared into the ether, and this was one cause
  • crosstool assumes newlib implies no thread support. newlib.in needs to be modified to not select no thread support
  • gcc config array in the crosstool configs needs to have --enable-threads=posix
  • zephyr needs an OS define defined by GCC. Right now, ZEPHYR is defined on the compile command lines by cmake configs/etc. But this means you can't use it to differentiate zephyr during toolchain builds, which we will need to do to conditonalize some of the libstdc++ support. To make gcc add it should be easy, you just have to add a config file that defines cpp builtins for the os (see vxworks.h for an example), and add it to gcc's configure (again, see vxworks)
  • need to use zephyr pthread.h instead of the newlib one that gets generated (haven't looked into how to do this). It doesn't work at all to compile newlib without thread support and pass enable-threads=posix to the crosstool gcc config array.

Everything but the last one should be straightforward. I hand edited around the last one in the build directory (hand-disabled emutls), and it succeeds in compiling libstdc++ with thread support. So once that header is the right one, it should work.

Unfortunately, this is all the only way to get libstdc++ to compile the right files with threads enabled.

Hacking around all this would be possible (that's what the std::thread pull request does) but the libstdc++ internal impls change quite a bit from version to version, and y'all don't require specific toolchain/zephyr combinations.
So probably gotta just fix the toolchains.

I'm unfortunately out of time for a bit to hack more on this, but hopefully this is helpful

@cmorganBE
Copy link

Is anyone actively working on std::mutex support? I'd be interested in assisting with that (assuming it would be a wrapper around some zephyr primitives) but I'd need some pointers.

@dberlin
Copy link
Contributor

dberlin commented Dec 7, 2022 via email

@cmorganBE
Copy link

@dberlin and pthreads maps through to zephyr threads? why not enabled at the moment?

@dberlin
Copy link
Contributor

dberlin commented Dec 7, 2022

Yes.
I gave the checklist of stuff that would have to be done to make it practically work above.

@cfriedt
Copy link
Member

cfriedt commented Dec 8, 2022

Actually, the only remaining feature to support std::thread and friends is automatic thread stack allocation, which is nearly finished in #44379. It's just missing mmu initialization.

I recently made some POSIX changes which add PTHREAD_MUTEX_INITIALIZER, and PTHREAD_COND_INITIALIZER.

#52316 has some additional POSIX subsystem fixes. To make our POSIX implementation more compatible with that of Newlib / PicoLibC, but it's waiting on reviews.

@cfriedt
Copy link
Member

cfriedt commented Feb 2, 2023

Request: harmonize .clang-format for both C and C++

Rather than have a separate .clang-format for C and another one for C++ (if that was the strategy), it might be better to simply ensure that the one .clang-format that we have works for both C and C++.

The main reason is that no tooling (including clang-format itself, but also e.g. VS Code) exoects to have to deal with multiple .clang-format files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: C++ Feature A planned feature with a milestone Meta A collection of features, enhancements or bugs
Projects
None yet
Development

No branches or pull requests

6 participants