Since NuttX conform to POSIX standard, the code need to add is very simple
Details
Details
- Reviewers
ldionne - Group Reviewers
Restricted Project - Commits
- rGf0785c1f7acd: [libcxx] Port to NuttX (https://nuttx.apache.org) RTOS
Diff Detail
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Comment Actions
A general comment - is someone going to run a libc++ test bot for NuttX? Otherwise, how will we know when changes that we make to libc++ break NuttX support?
Comment Actions
Thanks for the contribution. It's nice to see a new platform that's as easy to support as this.
However, I'll echo Marshall's comment here. If we are to add support for a new platform, we need CI for it. Are you willing and able to provide that? We use BuildKite for CI and it's fairly simple to setup.
| libcxx/include/__config | ||
|---|---|---|
| 1124 | It get defined in two ways: https://github.com/NuttX/buildroot The result binary define __NuttX__ automatically just like how Linux toolchain define __linux__ 2.Define NuttX from the board/toolchain specific Makefile, like this: https://github.com/apache/incubator-nuttx/blob/master/boards/arm/imxrt/imxrt1050-evk/configs/libcxxtest/Make.defs#L77 | |
| libcxx/include/support/nuttx/xlocale.h | ||
| 18 | Yes, could be, but I want to minize the change as small as possible. But if you want, I will move the change to __config. | |
| 25 | NuttX support a special target platform named sim which is basically a normal process running on Linux/macOS/Cygwin, but simulate all OS API inside it. Since it is built by the host toolchain, I have to undef these macros to avoid libcxx detect the wrong target. Here is more info about simulator: | |
Comment Actions
NuttX is RTOS, so the setup for both build and test is much hard than normal development environment(e.g. Linux, macOS or Windows):
1.We need prepare the cross build toolchain
2.We need build libcxx with NuttX OS together
3.We need run the test with the real hardware or simulator
It isn't suitable to incorporate this type of change into libcxx because it's too specific to NuttX environment. On the other hand, we plan to integrate libc++ test into NuttX daliy testing infrastructure, like what we have already done for Linux Test Project here:
https://github.com/apache/incubator-nuttx-apps/pull/410
Comment Actions
(Also from the Apache NuttX team)
If what we set up on our end in CI is not sufficient enough, I would be willing to look at what needs to be done to provide that here via what you have in BuildKite. Supporting the simulator target probably would not be too difficult.
Comment Actions
That would be great.
Otherwise, have you considered building with -DLIBCXX_ENABLE_LOCALIZATION=OFF? I added this "flavor" of libc++ specifically to cater to the needs of smaller OSes that don't provide localization support. The best case scenario would be if libc++ could work on your platform out-of-the-box without support for localization: that way, we wouldn't even have to add another configuration for libc++.
Comment Actions
LIBCXX_ENABLE_LOCALIZATION is a great feature what we find it consume a large code size for small OS. Could you point me the change? I can't find LIBCXX_ENABLE_LOCALIZATION in libc++ 11.0.0 release.
Comment Actions
Just find it here: https://reviews.llvm.org/D90072
I will change the porting to adapter the new flag, which is very important for the small embeded system.
Comment Actions
@ldionne and @mclow.lists, the update should address your comment except the test and local. both of them could be addressed in the new patch.
| libcxx/include/__config | ||
|---|---|---|
| 27 | I'm not comfortable with undefining these macros here in libc++. Those are compiler-defined macros, and it's not libc++'s job to undefine them. It seems to me that you either need a compiler target triple that understands NuttX, or to do whatever macro magic you want to do lower in the stack. | |
| libcxx/include/__locale | ||
| 26 | Do you need this anymore? You shouldn't need it if you're using LIBCXX_ENABLE_LOCALIZATION=OFF. | |
| libcxx/include/__config | ||
|---|---|---|
| 27 | Simulator is a special case here: It's a normal program run on Ubunutu or macOS and then should be compiled by host gcc toolchain, but all functions(POSIX, libc...) are provided by NuttX. If we don't undefine these macros, we have to modify every location where reference them. The later approach is very fragile and hard to maintain. Do you have other suggestion? | |
| libcxx/include/__locale | ||
| 26 | Since stream and regex isn't supported when LIBCXX_ENABLE_LOCALIZATION=OFF, nuttx/xlocale.h is still needed to let the user turn on/off LIBCXX_ENABLE_LOCALIZATION as needed. | |
Comment Actions
@ldionne I move the toolchain macro undefinition to NuttX's Makefile, please review again.
Comment Actions
Do you have commit privilege? If so, please go ahead. Otherwise, please provide Author Name <email@domain> I should use when committing this for you.
| libcxx/include/__config | ||
|---|---|---|
| 27 |
Yes, a compiler target triple that actually understands your platform. | |
Comment Actions
I have the memory of a goldfish. I asked you the same in D91074. I'll commit this for you, thanks!
Comment Actions
Just FYI, you also forgot to add support/nuttx/xlocale.h to libcxx/include/CMakeLists.txt, but I realized it when applying the patch locally and added it myself.
I'm not comfortable with undefining these macros here in libc++. Those are compiler-defined macros, and it's not libc++'s job to undefine them.
It seems to me that you either need a compiler target triple that understands NuttX, or to do whatever macro magic you want to do lower in the stack.