libc: Add C23 stdbit.h with optional Kconfig and generic implementation#18347
libc: Add C23 stdbit.h with optional Kconfig and generic implementation#18347arjav1528 wants to merge 4 commits intoapache:masterfrom
Conversation
michallenc
left a comment
There was a problem hiding this comment.
That was fast, nice work! Please squash the commits ideally into two - one implementing stdbit and other updating the documentation. Also fix long lines in stdbit.h causing CI to fail.
We definitely need proper tests before merging this. These should go to app repository. I can write some, but won't get to it until next week probably.
ad2e28a to
277c277
Compare
8abb829 to
8e473c0
Compare
|
Seems the merge request now contains completely different commits unrelated to the title and previous changes. |
linguini1
left a comment
There was a problem hiding this comment.
Please check that your PR follows the contributing guidelines. Also, a mistake might have happened during rebase/squashing that has caused your commits to disappear, as Michal pointed out.
8e473c0 to
9d7969a
Compare
Signed-off-by: Arjav Patel <arjav1528@gmail.com>
9d7969a to
495af7a
Compare
yep, that was a rebase issue, nvm I have reapplied the changes, you can check now |
There was a problem hiding this comment.
Are there any test-suites freely available we could bring to NuttX to make sure this implementation works? I don't think we have any way to test it right now (not really a way around that though).
There was a problem hiding this comment.
I think we could write our own tests in apps directory. They should be pretty straightforward based on stdbit specification https://cppreference.com/w/c/header/stdbit.html. The only complicated part is to test it for every possible int size.
Not familiar enough with a method to test this implementation. Thank you for adding docs though!
Please add the Impact and Testing sections to your PR description (even if there's no way to test, just mention that).
Signed-off-by: Arjav Patel <arjav1528@gmail.com>
Signed-off-by: Arjav Patel <arjav1528@gmail.com>
495af7a to
4bbd833
Compare
| (unsigned long)__builtin_clzl(x)) | ||
| # define stdc_leading_zeros_ull(x) \ | ||
| ((x) == 0 ? (unsigned long long)(8*sizeof(unsigned long long)) : \ | ||
| (unsigned long long)__builtin_clzll(x)) |
There was a problem hiding this comment.
Just realized we should add common stdc_leading_zeros. It can be implemented as
#define stdc_leading_zeros(x) (stdc_leading_zeros_ull(x) - (unsigned int)(8 * (sizeof(0ULL) - sizeof(x)))and similarly for other stdc defines.
There was a problem hiding this comment.
Me too!, I have commited the change, kindly review the PR
Signed-off-by: Arjav Patel <arjav1528@gmail.com>
Fixes: #18311
Summary
Add optional C23
stdbit.hsupport so builds can provide<stdbit.h>when the toolchain does not. Follows the same pattern asstdarg.h/math.h: redirect atinclude/nuttx/lib/stdbit.h, copied toinclude/stdbit.hwhen enabled.ARCH_HAVE_STDBIT_H,ARCH_STDBIT_H(arch-specific),LIBC_STDBIT_GENERIC(generic on any arch).__builtin_clz/__builtin_ctz/__builtin_popcountfor all C23 stdbit macros; archs may providearch/<arch>/include/stdbit.h.libs/libc/stdbit/stdbit_verify.c(compile-time check when stdbit enabled).Documentation/legacy_README.mdandDocumentation/components/libs/libc/index.rst.Impact
ARCH_STDBIT_HorLIBC_STDBIT_GENERICis enabled. Code can then use C23<stdbit.h>macros when the toolchain lacks them.include/stdbit.hwhen stdbit support is enabled; build system (Unix.mk, Win.mk, CMake) is extended only for this copy.stdbit.h. No impact on existing configs that don’t enable it.Testing
There is no freely available, upstream test suite for C23
stdbit.hthat we could add to NuttX to validate this implementation. Verification is limited to:LIBC_STDBIT_GENERIC(or arch-specificARCH_STDBIT_Hwhere implemented) and confirming the build completes andstdbit_verify.ccompiles (compile-time checks that the macros expand as expected).