Hi, Willy
Since we have already finished the size inflate regression task [1], to share and discuss the progress about the -ENOSYS return work, here launchs a new thread, it is split from [2].
[1]: https://lore.kernel.org/lkml/ZNtszQeigYuItaKA@1wt.eu/ [2]: https://lore.kernel.org/lkml/20230814172233.225944-1-falcon@tinylab.org/#R
This is only for brain storming, it is far from a solution ;-)
[...]
/* __systry2() is used to select one of two provided low level syscalls */ #define __systry2(a, sys_a, sys_b) \ ((NOLIBC__NR_##a != NOLIBC__NR_NOSYS) ? (sys_a) : (sys_b))
But this supposes that all of them are manually defined as you did above. I'd rather implement an ugly is_numeric() macro based on argument resolution. I've done it once in another project, I don't remember precisely where it is but I vaguely remember that it used to check that the string resolution of the argument gave a letter (when it does not exist) or a digit (when it does). I can look into that later if needed. But please avoid extra macro definitions as much as possible, they're a real pain to handle in the code. There's no error when one is missing or has a typo, it's difficult to follow them and they don't appear in the debugger.
Yeah, your reply inspired me to look into the IS_ENABLED() from ../include/linux/kconfig.h macro again, there was a __is_defined() there, let's throw away the ugly sysnr.h. I thought of IS_ENABLED() was only for y/n/m before, but it does return 0 when the macro is not defined, it uses the same trick in syscall() to calculate the number of arguments, if the macro is not defined, then, 0 "argument".
The above trick is only for ""#define something 1" ;-)
Here shares a little progress on this, I have found it is easy to implement an ugly is_numeric() like macro as following:
/* Imported from include/linux/stringify.h */ #define __stringify_1(x...) #x #define __stringify(x...) __stringify_1(x)
/* * Check __NR_* definition by stringizing * * - The stringizing is to silence compile error about undefined macro * - If defined, the result looks like "3", "(4000 + 168)", not begin with '_' * - If not defined, the result looks like "__NR_read", begins with '_' */
#define __is_nr_defined(nr) ___is_nr_defined(__stringify(nr)) #define ___is_nr_defined(str) (str[0] != '_')
__is_nr_defined() is able to check if __NR_xxx is defined, but the harder part is getting the number of defined __NR_* without the error about undefined macro.
Of course, we can also use the __stringify() trick to do so, but it is expensive (bigger size, worse performance) to unstringify and get the number again, the expensive atoi() 'works' for the numeric __NR_*, but not work for (__NR_*_base + offset) like __NR_* definitions (used by ARM and MIPS), a simple interpreter is required for such cases and it is more expensive than atoi().
/* not for ARM and MIPS */
static int atoi(const char *s); #define __get_nr(name) __nr_atoi(__stringify(__NR_##name)) #define __nr_atoi(str) (str[0] == '_' ? -1L : ___nr_atoi(str)) #define ___nr_atoi(str) (str[0] == '(' ? -1L : atoi(str))
Welcome more discussion or let's simply throw away this direction ;-)
But it may really help us to drop tons of duplicated code pieces like this:
#ifdef __NR_xxxx ... #else return -ENOSYS; #endif
David, Thomas and Arnd, any inspiration on this, or is this really impossible (or make things worse) in language level? ;-)
What I'm thinking about is something like this or similar (As Willy commented before, the __sysdef() itself is not that good, please ignore itself, the core target here is using a single -ENOSYS return for all of the undefined branches):
#define __sysdef(name, ...) \ (__is_nr_defined(__NR_##name) ? my_syscall(__get_nr(name), ##__VA_ARGS__) : (long)-ENOSYS)
Or as Arnd replied in an old email thread before, perhaps the whole #ifdef's code piece (and even the input types and return types of sys_*) above can be generated from .tbl or the generic unistd.h automatically in the sysroot installation stage?
BR, Zhangjin
Hi Zhangjin,
thanks for the RFC discussion!
On 2023-08-27 16:32:25+0800, Zhangjin Wu wrote:
Since we have already finished the size inflate regression task [1], to share and discuss the progress about the -ENOSYS return work, here launchs a new thread, it is split from [2].
This is only for brain storming, it is far from a solution ;-)
[...]
/* __systry2() is used to select one of two provided low level syscalls */ #define __systry2(a, sys_a, sys_b) \ ((NOLIBC__NR_##a != NOLIBC__NR_NOSYS) ? (sys_a) : (sys_b))
But this supposes that all of them are manually defined as you did above. I'd rather implement an ugly is_numeric() macro based on argument resolution. I've done it once in another project, I don't remember precisely where it is but I vaguely remember that it used to check that the string resolution of the argument gave a letter (when it does not exist) or a digit (when it does). I can look into that later if needed. But please avoid extra macro definitions as much as possible, they're a real pain to handle in the code. There's no error when one is missing or has a typo, it's difficult to follow them and they don't appear in the debugger.
Yeah, your reply inspired me to look into the IS_ENABLED() from ../include/linux/kconfig.h macro again, there was a __is_defined() there, let's throw away the ugly sysnr.h. I thought of IS_ENABLED() was only for y/n/m before, but it does return 0 when the macro is not defined, it uses the same trick in syscall() to calculate the number of arguments, if the macro is not defined, then, 0 "argument".
The above trick is only for ""#define something 1" ;-)
Here shares a little progress on this, I have found it is easy to implement an ugly is_numeric() like macro as following:
/* Imported from include/linux/stringify.h */ #define __stringify_1(x...) #x #define __stringify(x...) __stringify_1(x) /* * Check __NR_* definition by stringizing * * - The stringizing is to silence compile error about undefined macro * - If defined, the result looks like "3", "(4000 + 168)", not begin with '_' * - If not defined, the result looks like "__NR_read", begins with '_' */ #define __is_nr_defined(nr) ___is_nr_defined(__stringify(nr)) #define ___is_nr_defined(str) (str[0] != '_')
__is_nr_defined() is able to check if __NR_xxx is defined, but the harder part is getting the number of defined __NR_* without the error about undefined macro.
Of course, we can also use the __stringify() trick to do so, but it is expensive (bigger size, worse performance) to unstringify and get the number again, the expensive atoi() 'works' for the numeric __NR_*, but not work for (__NR_*_base + offset) like __NR_* definitions (used by ARM and MIPS), a simple interpreter is required for such cases and it is more expensive than atoi().
/* not for ARM and MIPS */ static int atoi(const char *s); #define __get_nr(name) __nr_atoi(__stringify(__NR_##name)) #define __nr_atoi(str) (str[0] == '_' ? -1L : ___nr_atoi(str)) #define ___nr_atoi(str) (str[0] == '(' ? -1L : atoi(str))
Welcome more discussion or let's simply throw away this direction ;-)
But it may really help us to drop tons of duplicated code pieces like this:
#ifdef __NR_xxxx ... #else return -ENOSYS; #endif
David, Thomas and Arnd, any inspiration on this, or is this really impossible (or make things worse) in language level? ;-)
What I'm thinking about is something like this or similar (As Willy commented before, the __sysdef() itself is not that good, please ignore itself, the core target here is using a single -ENOSYS return for all of the undefined branches):
#define __sysdef(name, ...) \ (__is_nr_defined(__NR_##name) ? my_syscall(__get_nr(name), ##__VA_ARGS__) : (long)-ENOSYS)
Or as Arnd replied in an old email thread before, perhaps the whole #ifdef's code piece (and even the input types and return types of sys_*) above can be generated from .tbl or the generic unistd.h automatically in the sysroot installation stage?
To be honest I don't see a problem with the current aproach. It is very obvious what is going on, the same pattern is used by other projects and the "overhead" is very small.
It seems the macros will only work for simple cases which only test the availability of a single syscall number.
Of these we currently only have: gettimeofday(), lseek(), statx(), wait4()
So in it's current form we save 4 * 4 = 16 lines of code. The proposed solution introduces 14 + 2 (empty) = 16 lines of new code, and a bunch of mental overhead.
In case multiple underlying syscalls can be used these take different arguments which a simple macro won't be able to encode sanely.
Thomas
Hi all,
On Sun, Aug 27, 2023 at 11:17:19AM +0200, Thomas Weißschuh wrote:
To be honest I don't see a problem with the current aproach. It is very obvious what is going on, the same pattern is used by other projects and the "overhead" is very small.
It seems the macros will only work for simple cases which only test the availability of a single syscall number.
Of these we currently only have: gettimeofday(), lseek(), statx(), wait4()
So in it's current form we save 4 * 4 = 16 lines of code. The proposed solution introduces 14 + 2 (empty) = 16 lines of new code, and a bunch of mental overhead.
In case multiple underlying syscalls can be used these take different arguments which a simple macro won't be able to encode sanely.
I totally agree, I would prefer all this to be manageable by humans with no preprocessor brain implant as much as possible as well.
Thanks, Willy
Hi, Thomas, Willy and David
Hi all,
On Sun, Aug 27, 2023 at 11:17:19AM +0200, Thomas Wei�schuh wrote:
To be honest I don't see a problem with the current aproach. It is very obvious what is going on, the same pattern is used by other projects and the "overhead" is very small.
It seems the macros will only work for simple cases which only test the availability of a single syscall number.
Good news, as I just replied [1] and as the test [2] shows, the __stringify() based __is_nr_defined() macro (proposed in this RFC thread) can test syscall definition for us, and the __stringify() and __nrtoi() macro (Derive from David's NR_toi() work [3],[4]) based __get_nr() macro can get the syscall number constant, both of them are compiling stage cost and the compiling is not slow for all of the current supported architectures, no size cost no runtime cost and even help us to get smaller binary ;-)
[1]: https://lore.kernel.org/lkml/20230829233912.63097-1-falcon@tinylab.org/ [2]: https://godbolt.org/z/a7hxWj83E [3]: https://lore.kernel.org/lkml/6819b8e273dc44e18f14be148549b828@AcuMS.aculab.c... [4]: https://godbolt.org/z/rear4c1hj
Of these we currently only have: gettimeofday(), lseek(), statx(), wait4()
So in it's current form we save 4 * 4 = 16 lines of code. The proposed solution introduces 14 + 2 (empty) = 16 lines of new code, and a bunch of mental overhead.
Yes, as also suggested by Willy, the old proposed method redefined NOLIBC__NR_* macros for every __NR_* and it must be avoided, and now, the __is_nr_defined() and __get_nr() macros will simply avoid defining new NOLIBC__NR_* for exisitng __NR_*, they can be used to test and get the existing __NR_* directly.
In my local repo, we have saved 500+ lines ;-)
$ git show nolibc/next:tools/include/nolibc/sys.h | wc -l 1190 $ cat tools/include/nolibc/sys.h | wc -l 690
Including all of the -ENOSYS and #ifdef's:
$ git grep -r ENOSYS nolibc/next:tools/include/nolibc/sys.h | wc -l 17 $ git grep -Er "#ifdef|#el|#endif" nolibc/next:tools/include/nolibc/sys.h | wc -l 77
In case multiple underlying syscalls can be used these take different arguments which a simple macro won't be able to encode sanely.
I totally agree, I would prefer all this to be manageable by humans with no preprocessor brain implant as much as possible as well.
Yeah, for the sys_* definitions, it is ok for us to use explicit arguments intead of the '...'/__VA_ARGS__ to avoid losing some arguments sometimes, let's do it in the RFC patchset but it should come after the tinyconfig patchset.
BTW, Willy, when will you prepare the branch for v6.7 developmlent? ;-)
Thanks, Zhangjin
Thanks, Willy
On Wed, Aug 30, 2023 at 08:19:07AM +0800, Zhangjin Wu wrote:
Yes, as also suggested by Willy, the old proposed method redefined NOLIBC__NR_* macros for every __NR_* and it must be avoided, and now, the __is_nr_defined() and __get_nr() macros will simply avoid defining new NOLIBC__NR_* for exisitng __NR_*, they can be used to test and get the existing __NR_* directly.
In my local repo, we have saved 500+ lines ;-)
$ git show nolibc/next:tools/include/nolibc/sys.h | wc -l 1190 $ cat tools/include/nolibc/sys.h | wc -l 690
Including all of the -ENOSYS and #ifdef's:
$ git grep -r ENOSYS nolibc/next:tools/include/nolibc/sys.h | wc -l 17 $ git grep -Er "#ifdef|#el|#endif" nolibc/next:tools/include/nolibc/sys.h | wc -l 77
And how many hacks or bugs for the rare special cases ? I'm not kidding, this obsession for removing lines has already caused us quite some trouble around sysret() in the previous cycle, and I yet have to see the gain for maintenance.
I do have comparable macros that I never employed in my projects just because each time I needed them I found a corner case at one particular optimization level or with a particular compiler version where you manage to break them, and suddenly all the world falls apart. I'm fine for taking that risk when there is a *real* benefit, but here we're speaking about replacing existing, readable and auditable code by something more compact that becomes completely unauditable. I could understand that if it was a specific required step in a more long-term project of factorizing something, but there still hasn't been any such project defined, so all we're doing is "let's see if we can do this or that and see if it looks better". I continue to strongly disagree with this approach, it causes all of us a lot of extra work, introduces regressions and nobody sees the benefits in the end.
Instead of using tricks here and there to remove lines, I'd rather have an approach centered on the code's architecture and modularity to see what are the current problems and how they should be addressed.
For now I still find it complicated to explain other maintainers how to test their changes on all architectures. I've found it difficult to switch between arm and thumb modes for arm when trying to explain it lately (now with more analysis I'm seeing that I could have placed it into CFLAGS_arm for example) so it means we're missing some doc in the makefile itself or on the usage in general. I've faced the problem you met with some builds failing on "you need to run mrproper first", which makes me think that in fact it should probably be "make defconfig" or "make prepare" that does this. Just checking the makefile and that's already the case, yet I faced the issue, so maybe it's caused by -j being passed through all the chain and we need to serialize the operations, I don't know.
I would also like that we clarify some use cases. Originally the project started as the single-file zero-installation file that allowed to build static binaries, retrieving the linux/ subdir from wherever it was (i.e. from the local system libc for native builds or from the toolchain used for cross-builds). Since we've started to focus a bit too much on the nolibc-test program only with its preparation stages, I think we've lost this focus a little bit, and I'd like to add some tests to make sure this continues to work (I know that my primary usage already got broken by the statx change with the toolchain I was using).
Also, maybe it could be useful to make it easier to produce tar.gz sysroots form tools/include/nolibc for various (even all?) archs to make it easier for users to test their own code against it.
So in short, we need to make it easier to use and easier to test, not to just remove lines that nobody needs to maintain.
Yeah, for the sys_* definitions, it is ok for us to use explicit arguments intead of the '...'/__VA_ARGS__ to avoid losing some arguments sometimes, let's do it in the RFC patchset but it should come after the tinyconfig patchset.
BTW, Willy, when will you prepare the branch for v6.7 developmlent? ;-)
You can continue to use the latest branch as a starting point, we'll create the new for-6.7 branch once 6.6-rc1 is out.
Thanks, Willy
Hi, Willy
On Wed, Aug 30, 2023 at 08:19:07AM +0800, Zhangjin Wu wrote:
Yes, as also suggested by Willy, the old proposed method redefined NOLIBC__NR_* macros for every __NR_* and it must be avoided, and now, the __is_nr_defined() and __get_nr() macros will simply avoid defining new NOLIBC__NR_* for exisitng __NR_*, they can be used to test and get the existing __NR_* directly.
In my local repo, we have saved 500+ lines ;-)
$ git show nolibc/next:tools/include/nolibc/sys.h | wc -l 1190 $ cat tools/include/nolibc/sys.h | wc -l 690
Including all of the -ENOSYS and #ifdef's:
$ git grep -r ENOSYS nolibc/next:tools/include/nolibc/sys.h | wc -l 17 $ git grep -Er "#ifdef|#el|#endif" nolibc/next:tools/include/nolibc/sys.h | wc -l 77
And how many hacks or bugs for the rare special cases ? I'm not kidding, this obsession for removing lines has already caused us quite some trouble around sysret() in the previous cycle, and I yet have to see the gain for maintenance.
Agree very much, Willy, I must clarify again about why I'm working on this and that ;-)
Almost all of current work are preparation for rv32 support (exactly, time64 and size64 syscalls), some are for basic compiling support, some are for test speed up (for fast verification of changes), and the one sysret() you mentioned above and this RFC discuss are preparation for our new time64 syscalls and size64 syscalls, removing lines is just a side effect and I have no interest and no time in removing lines ;-) it is not the original goal.
Although the sysret() has introduced a size regression but perhaps we could reserve more review and test cycles (or even more test cases, for example, size test, -O0,1,2,3,s test, different toolchains test, I'm interested in opening a standalone github repo to do so and perhaps Yuan could work on this too ...) for such new changes in the future, and this RFC also targets the above goal, discuss before real patchset.
To be honest, although I do understand your worry, this is only a RFC, it is far far from killing the maintainability, I'm not expecting the coming RFC patchset will happen in v6.7, v6.8 or even any future versions, I'm just posting a discussion for some comments on a possibility and David did proposed a very good suggestion and we are discussing and verifying its correctness and performance carefully and we did get some important progress ;-)
Now let's back to the background behind this RFC, before sending the new time64 syscalls and the size64 syscalls, perhaps we need to clear some of these questions:
1. Where should we put #ifdef in the new syscalls? libc-level funcs or sys_* funcs?
Currently, multiple my_syscall* are called in sys_* funcs, so, the name of sys_* doesn't really reflect the kernel-level syscall, as Arnd suggested before (Willy also replied), it may be possible to map my_syscall* to sys_* one by one, and then call differnt variants of sys_* funcs in libc-level funcs. And also, as we discussed before, #ifdef generates smaller binary size than -ENOSYS check.
There are three possible ways:
- one is aligning with the current style, and reorg them in the future, but many new syscalls coming means more reorg work in the future.
- another is adding new syscalls with new method, but this will mix two different styles together.
- the third one is thinking about reorg in this stage, that is why this RFC is talking about if it is possible to remove #ifdef's completely, if possible, then, no need to move any of the #ifdef's, the reorg will not become that simply moving #ifdef's from sys_* to their libc-level interfaces.
2. Is it possible to clear more about the relationship between sys_* and libc-level interfaces?
The relationship among my_syscall*, sys_* and libc-level interfaces are documented very well in tools/include/nolibc/nolibc.h.
As Yuan asked me in the last month, the prototype of sys_* is almost the same as libc-level interfaces, they almost share the same input arguments, return the same type, that's why I proposed the __sysdef() macro to simply inherit the arguments from libc-level interfaces and return the same return types from my_syscall* (only brk and mmap requires (void *) conversions currently).
The issues of __sysdef() pointed out by you require to be solved is making sure people not lose one of the tailing arguments accidentally, so, instead of using .../__VA_ARGS, let's use explicit arguments instead, but it may still avoid some unnecessary input types and return types conversions or passing and also save lots of boring copy and paste.
This is also related to the first question, that is this __sysdef() also helps to clear the sys_* roles, it is possible to only map sys_* to the low-level kernel syscall with the same name and not mix them with its variants, and it is possible to do some normalization among architectures or among different kernel versions, at last, try the best to provide the same sys_* interfaces to higher libc-level ones, but doing minimal normalization here may be better, perhaps only for sys_* with the same name is ok, then, sys_* will be very clean and with very few exceptions (such as select, mmap and clone, we have seen s390 defined its own variants, we should do this in sys_* level and let architecture choose a right version via __ARCH_WANT_SYS_*), as a result, the sys_* will become a very thin and unified layer between kernel and libc-level interfaces, and the libc-level interfaces should take over the work to choose the right sys_* or their variants the target architectures provided.
3. Is it possible to tune the order of #ifdef's to get smaller binary
As local test shows, benifit from the types inherit of the above __sysdef(), less type conversions generate smaller binary, besides, I have found, tuning #ifdef's order also helps size optimization. sys_* with less arguments and smaller arguments has smaller binary size, but it is not that attractive if only want to get smaller binary size because the changing of the #ifdef's will be very ugly, but the possibility of removing them completely may be another situation. Will measure and report the size reduce percentage in our RFC patchset.
I will back to the time64 and size64 syscalls (necessary for rv32) soon, the above questions are generic to all of them, that's why I post this RFC discussion (perhaps, the RFC title should be something like tools/nolibc: preparation for time64 and size64 syscalls).
And we may have more questions, for example, we may need to wrap all size64 syscalls under _LARGEFILE64_SOURCE to allow get smaller size, but time64 syscalls should be compellent for the coming y2038 issues, before sending the syscalls one be one, some generic issues should be discussed and cleared.
I do have comparable macros that I never employed in my projects just because each time I needed them I found a corner case at one particular optimization level or with a particular compiler version where you manage to break them, and suddenly all the world falls apart.
Willy, based on nolibc-test, we do need a standalone testsuite, as I mentioned above and the issues we have encountered before (for example, _start regression with -O0), this is really an urgent task. Perhaps, your local test scripts is a very good base.
And seems both David and Ammar are using https://godbolt.org/ frequently, a local godbolt.org may be good for toolchains coverage, and it is a very good test platform for some code pieces under different toolchain versions and different compiler options, I have used it to tune the __nrtoi() macro for some randomly chosen clang and gcc versions but it is not enough, it must be at first right at language level and then fix up the implementation issues reported by toolchains or review.
A test robot may be important, especially for a new feature or a big change, test coverage is required.
I'm fine for taking that risk when there is a *real* benefit, but here we're speaking about replacing existing, readable and auditable code by something more compact that becomes completely unauditable. I could understand that if it was a specific required step in a more long-term project of factorizing something, but there still hasn't been any such project defined, so all we're doing is "let's see if we can do this or that and see if it looks better". I continue to strongly disagree with this approach, it causes all of us a lot of extra work, introduces regressions and nobody sees the benefits in the end.
It is a long-term project, it is a very long preparation for adding the new time64 and size64 syscalls, all of my work in past weeks are for this goal, but it is very hard to define everything clearly before we really work on the real patchsets, sometimes, new status, new suggestions ... I'm trying to discuss before sending any new patchsets.
Instead of using tricks here and there to remove lines, I'd rather have an approach centered on the code's architecture and modularity to see what are the current problems and how they should be addressed.
That's true, again, removing lines is only a side effect ;-)
For now I still find it complicated to explain other maintainers how to test their changes on all architectures.
The same to me, that is why I'm working on tinyconfig, O=, and CROSS_COMPILE customize support, tinyconfig is 10+ times faster than defconfig, for all of the architectures, it may save us by one day ...
And also, as we discussed before, perhaps the test repo should also provide the prebuilt qemu-user, qemu-system and qemu bios for a target architecture, especially when the architecture is very new.
I've found it difficult to switch between arm and thumb modes for arm when trying to explain it lately (now with more analysis I'm seeing that I could have placed it into CFLAGS_arm for example)
I used CFLAGS_i386 with x86_64 toolchain before, perhaps it is time to add more variants with our new XARCH variable.
so it means we're missing some doc in the makefile itself or on the usage in general. I've faced the problem you met with some builds failing on "you need to run mrproper first", which makes me think that in fact it should probably be "make defconfig" or "make prepare" that does this. Just checking the makefile and that's already the case, yet I faced the issue, so maybe it's caused by -j being passed through all the chain and we need to serialize the operations, I don't know.
Perhaps we should fix up this issue before docing it, I have asked Yuan helping me to analyze the O= issue, that may be something about the top-level kernel headers_install target, the generic-y in scripts/Makefile.asm-generic may require to be revised to make it work well with O= when the top-level source code is not clean (and I have seen kselftest itself also not support O= very well when I'm trying to run nolibc from kselftest, that means O= is not that a always stable argument). Thomas' -nostdinc patchset is a good start to avoid hiding the kernel or nolibc issues from the toolchain sides.
I would also like that we clarify some use cases. Originally the project started as the single-file zero-installation file that allowed to build static binaries, retrieving the linux/ subdir from wherever it was (i.e. from the local system libc for native builds or from the toolchain used for cross-builds). Since we've started to focus a bit too much on the nolibc-test program only with its preparation stages, I think we've lost this focus a little bit, and I'd like to add some tests to make sure this continues to work (I know that my primary usage already got broken by the statx change with the toolchain I was using).
Seems we have discussed this before, to get zero-installation, musl is a good idea, it has no need to install sysroot, but it may increase the size of nolibc source code for it requires to define our own structures and therefore not depends on the others.
Also, maybe it could be useful to make it easier to produce tar.gz sysroots form tools/include/nolibc for various (even all?) archs to make it easier for users to test their own code against it.
So in short, we need to make it easier to use and easier to test, not to just remove lines that nobody needs to maintain.
Yeah, for the sys_* definitions, it is ok for us to use explicit arguments intead of the '...'/__VA_ARGS__ to avoid losing some arguments sometimes, let's do it in the RFC patchset but it should come after the tinyconfig patchset.
BTW, Willy, when will you prepare the branch for v6.7 developmlent? ;-)
You can continue to use the latest branch as a starting point, we'll create the new for-6.7 branch once 6.6-rc1 is out.
Ok, will wait for 6.6-rc1.
Thanks, Zhangjin
Thanks, Willy
Hi Zhangjin,
On Sat, Sep 02, 2023 at 03:03:17AM +0800, Zhangjin Wu wrote:
To be honest, although I do understand your worry, this is only a RFC, it is far far from killing the maintainability, I'm not expecting the coming RFC patchset will happen in v6.7, v6.8 or even any future versions, I'm just posting a discussion for some comments on a possibility and David did proposed a very good suggestion and we are discussing and verifying its correctness and performance carefully and we did get some important progress ;-)
But both Thomas and I explicitly stated multiple times that we did find that that all of this significantly complicates the code for little benefit. If we both find it more complicated, it *does* affect maintainabilityi, and I find it a bit tiresome that we have to justify ourselves when expressing a feeling of more difficult maintenance.
There are undoubtly good points but also bad ones that come from such "simplification". The good ones save some trivial copy- pastes and the bad one induce potential issues that are difficult to fix in the long term. See how long the discussions about integer detection in macros have lasted! I do have similar macros that I think I posted already, whose purpose was precisely to turn a name into a number when the defined macro exists. Oh sure it was quite simple and appeared to work fine at first glance. Then one compiler complained about possilbe risk of out-of-bounds so I prepended "00..." to each string and solved it like this, I was happy. And later I discovered that some compilers (don't remember which ones) are not happy when using such complex expressions as array indexes. Then I found that when some values were defined in hexadecimal with "0x" in front, it didn't work. Then that it didn't work either when macros are defined as expressions with parenthesis. So finally I gave up because the whole approach was wrong.
And by the way you should have a look at archs that have multiple ABIs like MIPS and ARM, as I'm pretty sure that at least these two ones define their syscalls in a way looking like (__NR_base + index_write), which is not compatible with a direct macro-to-number conversion. That's just an example of course, but it's a perfect example of the stuff that I don't want to have to deal with in the long term solely for the sake of saving a few trivial lines. The effort required to maintain this working and trustable totally outweighs the benefits.
Now let's back to the background behind this RFC, before sending the new time64 syscalls and the size64 syscalls, perhaps we need to clear some of these questions:
- Where should we put #ifdef in the new syscalls? libc-level funcs or sys_* funcs?
First, we should not reference non-existing __NR_sysfoo anywhere, so this means that we have to ifdef sys_* functions anyway. Given that changes over time come from API variations and unification between archs, such as select vs newselect vs pselect6 etc, it doesn't seem absurd for now to provide a compatible sys_* interface on top of that. As you've seen, the main difference between sys_foo and foo is that sys_* does not touch errno and is only a syscall, while the other one may also remap some types to better match what applications need. For example stat() is now an emulation of the stat() call that calls a different syscall by rebuilding completely different arguments with a local struct. We could also have a statx() function as well. Here it's visible that if both of them rely on sys_statx(), the remapping code has to be in stat().
It's difficult for me to establish a rigid line between this, I think it mostly comes from common sense and judgement, trying to think whether we're adapting the user-facing API or the kernel-facing one. chmod() based on either chmod() or fchmodat() seems like it adapts to the kernel which may provide either, while the stat example above seems a bit different. Maybe this difference will fade away over time and one day we'll even find it simpler to just get rid of these sys_* intermediate functions and directly involve the syscalls from the libc-level functions, I don't know.
Currently, multiple my_syscall* are called in sys_* funcs, so, the name of sys_* doesn't really reflect the kernel-level syscall, as Arnd suggested before (Willy also replied), it may be possible to map my_syscall* to sys_* one by one, and then call differnt variants of sys_* funcs in libc-level funcs. And also, as we discussed before, #ifdef generates smaller binary size than -ENOSYS check.
There are three possible ways:
- one is aligning with the current style, and reorg them in the future, but many new syscalls coming means more reorg work in the future.
another is adding new syscalls with new method, but this will mix two different styles together.
the third one is thinking about reorg in this stage, that is why this RFC is talking about if it is possible to remove #ifdef's completely, if possible, then, no need to move any of the #ifdef's, the reorg will not become that simply moving #ifdef's from sys_* to their libc-level interfaces.
I think that having the ability to ifdef out any such generic function when it's already defined by the architecture like is already done for s390 is interesting. I think we should reason in terms of most desirable to fallback code. I mean, letting the architecture define something, then having the early sys_* code define certain options, then having the end of the file provide compatible fallbacks for the not-yet defined cases sounds fine and flexible. We could then continue to mostly focus on the generic stuff and let arch maintainers propose improvements for their archs, or others suggest new, more optimal approaches, etc.
Is it possible to clear more about the relationship between sys_* and libc-level interfaces?
The relationship among my_syscall*, sys_* and libc-level interfaces are documented very well in tools/include/nolibc/nolibc.h.
As Yuan asked me in the last month, the prototype of sys_* is almost the same as libc-level interfaces, they almost share the same input arguments, return the same type, that's why I proposed the __sysdef() macro to simply inherit the arguments from libc-level interfaces and return the same return types from my_syscall* (only brk and mmap requires (void *) conversions currently).
The issues of __sysdef() pointed out by you require to be solved is making sure people not lose one of the tailing arguments accidentally, so, instead of using .../__VA_ARGS, let's use explicit arguments instead, but it may still avoid some unnecessary input types and return types conversions or passing and also save lots of boring copy and paste.
This is also related to the first question, that is this __sysdef() also helps to clear the sys_* roles, it is possible to only map sys_* to the low-level kernel syscall with the same name and not mix them with its variants,
This would result in even more ifdefs because you'd have to test for their presence in every call place.
and it is possible to do some normalization among architectures or among different kernel versions, at last, try the best to provide the same sys_* interfaces to higher libc-level ones, but doing minimal normalization here may be better, perhaps only for sys_* with the same name is ok, then, sys_* will be very clean and with very few exceptions (such as select, mmap and clone, we have seen s390 defined its own variants, we should do this in sys_* level and let architecture choose a right version via __ARCH_WANT_SYS_*), as a result, the sys_* will become a very thin and unified layer between kernel and libc-level interfaces, and the libc-level interfaces should take over the work to choose the right sys_* or their variants the target architectures provided.
That's the way I thought we would do in the past but it came with many more ifdefs. I'm not necessarily against this, but that's something to be aware of. Another approach might be to check if we *really* need to have this separation layer. Maybe we could simply just call the defined syscalls from the libc functions, and for the rare archs that need something different, then implement the arch-specific sys_* that's called as a fallback.
Is it possible to tune the order of #ifdef's to get smaller binary
As local test shows, benifit from the types inherit of the above __sysdef(), less type conversions generate smaller binary, besides, I have found, tuning #ifdef's order also helps size optimization. sys_* with less arguments and smaller arguments has smaller binary size, but it is not that attractive if only want to get smaller binary size because the changing of the #ifdef's will be very ugly, but the possibility of removing them completely may be another situation. Will measure and report the size reduce percentage in our RFC patchset.
Note that every time you're observing a size change related to using macros instead of functions, it almost always means you're having type differences causing sign extension for example. That's something to be extremely careful about. It's not just about size, it's mostly about correctness.
I will back to the time64 and size64 syscalls (necessary for rv32) soon, the above questions are generic to all of them, that's why I post this RFC discussion (perhaps, the RFC title should be something like tools/nolibc: preparation for time64 and size64 syscalls).
These ones are probably a good example where we might think about keeping the separation between sys_foo and foo, because there's no such time64 at the user level, but the syscalls differ at the kernel level for one arch, and different wrappers might require some type conversion to preserve compatibility, that might be easier done in sys_* functions. But I'm not sure, that's something to see when facing it.
And we may have more questions, for example, we may need to wrap all size64 syscalls under _LARGEFILE64_SOURCE to allow get smaller size, but time64 syscalls should be compellent for the coming y2038 issues, before sending the syscalls one be one, some generic issues should be discussed and cleared.
My goal is not to engrave in stone one choice or another. Reasonable people adapt to evolving conditions, and that's also why it's not always easy to respond to generic questions like above out of context. However I do want the maintenance cost to remain low. May maintenance effort over the last few months has more than quadrupled, requiring me to rewrite my test scripts several times, to change my development machine due to the breakage of compatibility for some parts, and to spend a lot of time trying to figure how to best address issues caused by a bug in a function meant to factor everything, and what I can say is thta deciding to change everything and to "simplify" everything adds lots more work than what it took to simply add the needed syscalls one at a time. So I'll be more careful about this.
And seems both David and Ammar are using https://godbolt.org/ frequently, a local godbolt.org may be good for toolchains coverage, and it is a very good test platform for some code pieces under different toolchain versions and different compiler options, I have used it to tune the __nrtoi() macro for some randomly chosen clang and gcc versions but it is not enough, it must be at first right at language level and then fix up the implementation issues reported by toolchains or review.
Godbolt is convenient for use by those proposing to *change* the code, it is not meant to be used by those having to *maintain* it, or it's an endless effort.
A test robot may be important, especially for a new feature or a big change, test coverage is required.
Test robots are sometimes convenient, but the simple fact that something works with all available robots is not necessarily a justification that it is correct nor desirable. It just saves a lot of time trying multiple combinations, helping to spot anomalies, warnings, unexpected code elimination or de-optimization that might sometimes happen.
I'm fine for taking that risk when there is a *real* benefit, but here we're speaking about replacing existing, readable and auditable code by something more compact that becomes completely unauditable. I could understand that if it was a specific required step in a more long-term project of factorizing something, but there still hasn't been any such project defined, so all we're doing is "let's see if we can do this or that and see if it looks better". I continue to strongly disagree with this approach, it causes all of us a lot of extra work, introduces regressions and nobody sees the benefits in the end.
It is a long-term project, it is a very long preparation for adding the new time64 and size64 syscalls, all of my work in past weeks are for this goal, but it is very hard to define everything clearly before we really work on the real patchsets, sometimes, new status, new suggestions ... I'm trying to discuss before sending any new patchsets.
That's great. But I'm not seeing how nor why a few syscalls couldn't be added without systematically having to reorganize and break everything. The two must be completely separate.
For now I still find it complicated to explain other maintainers how to test their changes on all architectures.
The same to me, that is why I'm working on tinyconfig, O=, and CROSS_COMPILE customize support, tinyconfig is 10+ times faster than defconfig, for all of the architectures, it may save us by one day ...
Yes very possibly.
And also, as we discussed before, perhaps the test repo should also provide the prebuilt qemu-user, qemu-system and qemu bios for a target architecture, especially when the architecture is very new.
For me this goes out of the scope of the project. You don't find prebuilt qemu in any other place in the kernel either, despite it being used quite a lot. Instead when a new architecture arrives, it should generally point to the tools required to build for it, and expect that not everyone is able to test it yet. It has always worked like this and that's fine.
I've found it difficult to switch between arm and thumb modes for arm when trying to explain it lately (now with more analysis I'm seeing that I could have placed it into CFLAGS_arm for example)
I used CFLAGS_i386 with x86_64 toolchain before, perhaps it is time to add more variants with our new XARCH variable.
Yes I think so.
I would also like that we clarify some use cases. Originally the project started as the single-file zero-installation file that allowed to build static binaries, retrieving the linux/ subdir from wherever it was (i.e. from the local system libc for native builds or from the toolchain used for cross-builds). Since we've started to focus a bit too much on the nolibc-test program only with its preparation stages, I think we've lost this focus a little bit, and I'd like to add some tests to make sure this continues to work (I know that my primary usage already got broken by the statx change with the toolchain I was using).
Seems we have discussed this before, to get zero-installation, musl is a good idea, it has no need to install sysroot, but it may increase the size of nolibc source code for it requires to define our own structures and therefore not depends on the others.
I don't understand why you're speaking about musl here. If we have musl we don't need nolibc. I was speaking about the fact that for a decade I could simply build my preinit code using whatever toolchain I had that already embedded linux headers with its libc and that these ones were sufficient to get a working binary, i.e.:
/path/to/$cross-gcc -nostdlib -include nolibc.h -static -Os -o init init.c
And it was convenient because most toolchains that users have are built with a libc and package kernel headers under linux/. That's also why there were some type definitions in certain files by the way, in order to cover those that were missing from my toolchains. I think that as long as we continue to be careful about only using our own headers for sys/ etc this should continue to work, and I'll need to think about ways to test this.
Regards, Willy
...
Of course, we can also use the __stringify() trick to do so, but it is expensive (bigger size, worse performance) to unstringify and get the number again, the expensive atoi() 'works' for the numeric __NR_*, but not work for (__NR_*_base + offset) like __NR_* definitions (used by ARM and MIPS), a simple interpreter is required for such cases and it is more expensive than atoi().
/* not for ARM and MIPS */ static int atoi(const char *s); #define __get_nr(name) __nr_atoi(__stringify(__NR_##name)) #define __nr_atoi(str) (str[0] == '_' ? -1L : ___nr_atoi(str)) #define ___nr_atoi(str) (str[0] == '(' ? -1L : atoi(str))
Welcome more discussion or let's simply throw away this direction ;-)
While it will look horrid the it ought to be possible to get the compiler to evaluate the string.
Since "abc"[2] (etc) is converted to a constant (by gcc and clang except at -O0) and you only need to process "n" "nn" "nnn" "(n + m)" (with variable length n and m) then append some spaces and convert the characters back to digits.
So something that starts: #define dig(c) (c < '0' || c > '9' ? 999999 : c - '0') str[0] == '_' ? -1 : str[0] != '(' ? str[1] == ' ' ? dig(str[0]) : str[2] == '1' ? (dig(str[0]) * 10 + dig(str[1]) : Any unexpected character will expand the 99999 and generate an over-large result. I'm not sure how constant the array index need to be. They may well have to be 'integer constant expressions' so cant depend on a previous str[const] value.
I just found a(nother) clang bug: int f(void) { return "a"[2]; } compiles to just a 'return'.
David
- Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
From: David Laight
Sent: 27 August 2023 22:52
...
Of course, we can also use the __stringify() trick to do so, but it is expensive (bigger size, worse performance) to unstringify and get the number again, the expensive atoi() 'works' for the numeric __NR_*, but not work for (__NR_*_base + offset) like __NR_* definitions (used by ARM and MIPS), a simple interpreter is required for such cases and it is more expensive than atoi().
/* not for ARM and MIPS */ static int atoi(const char *s); #define __get_nr(name) __nr_atoi(__stringify(__NR_##name)) #define __nr_atoi(str) (str[0] == '_' ? -1L : ___nr_atoi(str)) #define ___nr_atoi(str) (str[0] == '(' ? -1L : atoi(str))
Welcome more discussion or let's simply throw away this direction ;-)
While it will look horrid the it ought to be possible to get the compiler to evaluate the string.
...
So something that starts: #define dig(c) (c < '0' || c > '9' ? 999999 : c - '0') str[0] == '_' ? -1 : str[0] != '(' ? str[1] == ' ' ? dig(str[0]) : str[2] == '1' ? (dig(str[0]) * 10 + dig(str[1]) : Any unexpected character will expand the 99999 and generate an over-large result.
See https://godbolt.org/z/rear4c1hj
That will convert "1234" or "(1234 + 5678)" (or shorter numbers) as a compile-time constant.
David
- Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
Hi, David
From: David Laight
Sent: 27 August 2023 22:52
...
Of course, we can also use the __stringify() trick to do so, but it is expensive (bigger size, worse performance) to unstringify and get the number again, the expensive atoi() 'works' for the numeric __NR_*, but not work for (__NR_*_base + offset) like __NR_* definitions (used by ARM and MIPS), a simple interpreter is required for such cases and it is more expensive than atoi().
/* not for ARM and MIPS */ static int atoi(const char *s); #define __get_nr(name) __nr_atoi(__stringify(__NR_##name)) #define __nr_atoi(str) (str[0] == '_' ? -1L : ___nr_atoi(str)) #define ___nr_atoi(str) (str[0] == '(' ? -1L : atoi(str))
Welcome more discussion or let's simply throw away this direction ;-)
While it will look horrid the it ought to be possible to get the compiler to evaluate the string.
...
So something that starts: #define dig(c) (c < '0' || c > '9' ? 999999 : c - '0') str[0] == '_' ? -1 : str[0] != '(' ? str[1] == ' ' ? dig(str[0]) : str[2] == '1' ? (dig(str[0]) * 10 + dig(str[1]) : Any unexpected character will expand the 99999 and generate an over-large result.
See https://godbolt.org/z/rear4c1hj
That will convert "1234" or "(1234 + 5678)" (or shorter numbers) as a compile-time constant.
Thanks very much, it works perfectly.
I tuned it for more complicated cases, including ((0x900000+0x0f0000)+5) used by ARM+OABI (not used by nolibc), now, it should work for all of the architectures: https://godbolt.org/z/a7hxWj83E ;-)
To get fast building, we can provide different versions for different architectures. A simple test shows, only two versions (as you mentioned above, one is "1234" converting, another is "(1234 + 5678)" calculating) are enough for current nolibc supported architectures and the building of nolibc-test.c is not slow.
With the __stringify() based __is_nr_defined() macro and this new __nrtoi() macro based __get_nr() macro, there is no need to redefine the old NOLIBC__NR_* macros, as a result, all of the duplicated -ENOSYS return lines and even all of the #ifdef's from sys.h could be dropped and even no need to add them for new future syscalls, and also, the old syscall() macro can return -ENOSYS at the runtime instead of any compiling failures.
For the sys_* definitions, to avoid forgetting passing the arguments, instead of using __VA_ARGS__, perhaps we should simply passing all of the arguments.
Best Regards, Zhangjin
David
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
From: Ammar Faizi
Sent: 30 August 2023 15:41
On 8/28/23 4:51 AM, David Laight wrote:
I just found a(nother) clang bug: int f(void) { return "a"[2]; } compiles to just a 'return'.
I don't think that's a bug. It's undefined behavior due to an out-of-bound read. What do you expect it to return?
I was actually expecting a warning/error if it didn't just read the byte after the end of the string.
Just silently doing nothing didn't seem right for a modern compiler.
David
- Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
Hi, David, Hi Ammar
From: Ammar Faizi
Sent: 30 August 2023 15:41
On 8/28/23 4:51 AM, David Laight wrote:
I just found a(nother) clang bug: int f(void) { return "a"[2]; } compiles to just a 'return'.
I don't think that's a bug. It's undefined behavior due to an out-of-bound read. What do you expect it to return?
I was actually expecting a warning/error if it didn't just read the byte after the end of the string.
Just silently doing nothing didn't seem right for a modern compiler.
godbolt.org uses 'orange' color (see its right top) to indicate a warning, it does report a warning (see Output label in right bottom) when we access the byte after the end of the string, but gcc doesn't report a warning ;-)
int test_outofbound(void) { return "a"[10]; }
see the last test case in https://godbolt.org/z/9jY4xoWrT
But it is safe if we use the trick like David used for the above __atoi() macro:
if (str[0]) { /* always make sure the index is safe and stop at the end of string */ if (str[1]) { if (str[2]) { .... } } }
We also need this style of checking for the delta logic in __atoi_add(). have randomly tried different clang and gcc versions, seems all of them work correctly, but the compiling speed is not that good if we want to support the worst cases like "((0x900000 + 0x0f0000) + 5)", the shorter one "((0x900000+0x0f0000)+5)" is used by ARM+OABI (not supported by nolibc currently), therefore, we can strip some tailing branches but it is either not that fast, of course, the other architectures/variants can use faster __atoi_add() versions with less branches and without hex detection, comparison and calculating.
As a short summary, the compling speed should not be a big problem for most of the architectures but to support the worst case __NR_*, the compiling speed will be very slow (for these cases, perhaps we can use a C version of atoi_add() instead or convert them to a more generic style: (6000 + 111), no hex and no multiple add), and the .i output is a little ugly and the debugging may be also a problem: for we can not assume the kernel developers always define a short and a simple style of __NR_* as we expected. So, the __nrtoi() requires more work, let's delay the whole RFC patchset and work on some more urgent tasks at first as suggested by Willy, but David's NR_toi() prototype is really a very valuable base for future work, really appreciate, I will back to this discussion if have any new progress, thanks!
Thanks very much, Zhangjin
David
...
We also need this style of checking for the delta logic in __atoi_add(). have randomly tried different clang and gcc versions, seems all of them work correctly, but the compiling speed is not that good if we want to support the worst cases like "((0x900000 + 0x0f0000) + 5)", the shorter one "((0x900000+0x0f0000)+5)" is used by ARM+OABI (not supported by nolibc currently), therefore, we can strip some tailing branches but it is either not that fast, of course, the other architectures/variants can use faster __atoi_add() versions with less branches and without hex detection, comparison and calculating.
If there are only a few prefix offsets then the code can be optimised to explicitly detect them - rather than decoding arbitrary hex values. After all it only needs to decode the values that actually appear.
The code also needs a compile-time assert that the result is constant (__buitin_constant_p() will do the check. But you can't use _Static_assert() to report the error because that requires an 'integer constant expression'.
David
- Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
Hi, David
...
We also need this style of checking for the delta logic in __atoi_add(). have randomly tried different clang and gcc versions, seems all of them work correctly, but the compiling speed is not that good if we want to support the worst cases like "((0x900000 + 0x0f0000) + 5)", the shorter one "((0x900000+0x0f0000)+5)" is used by ARM+OABI (not supported by nolibc currently), therefore, we can strip some tailing branches but it is either not that fast, of course, the other architectures/variants can use faster __atoi_add() versions with less branches and without hex detection, comparison and calculating.
If there are only a few prefix offsets then the code can be optimised to explicitly detect them - rather than decoding arbitrary hex values. After all it only needs to decode the values that actually appear.
The code also needs a compile-time assert that the result is constant (__buitin_constant_p() will do the check. But you can't use _Static_assert() to report the error because that requires an 'integer constant expression'.
Thanks a lot, your above suggestion inspired me a lot.
I have explored ARM and MIPS again and found their __NR_* definitions have only a 'dynamic' part, that is the right part:
arch/mips/include/generated/uapi/asm/unistd_o32.h:#define __NR_io_uring_register (__NR_Linux + 427) arch/mips/include/generated/uapi/asm/unistd_o32.h:#define __NR_open_tree (__NR_Linux + 428) arch/mips/include/generated/uapi/asm/unistd_o32.h:#define __NR_move_mount (__NR_Linux + 429) arch/mips/include/generated/uapi/asm/unistd_o32.h:#define __NR_fsopen (__NR_Linux + 430) arch/mips/include/generated/uapi/asm/unistd_o32.h:#define __NR_fsconfig (__NR_Linux + 431)
arch/arm/include/generated/uapi/asm/unistd-eabi.h:#define __NR_io_uring_setup (__NR_SYSCALL_BASE + 425) arch/arm/include/generated/uapi/asm/unistd-eabi.h:#define __NR_io_uring_enter (__NR_SYSCALL_BASE + 426) arch/arm/include/generated/uapi/asm/unistd-eabi.h:#define __NR_io_uring_register (__NR_SYSCALL_BASE + 427) arch/arm/include/generated/uapi/asm/unistd-eabi.h:#define __NR_open_tree (__NR_SYSCALL_BASE + 428) arch/arm/include/generated/uapi/asm/unistd-eabi.h:#define __NR_move_mount (__NR_SYSCALL_BASE + 429)
The left part: __NR_Linux and __NR_SYSCALL_BASE are always defined, so, we can get their values directly, without the need of stringify and unstringify, as a result, the delta addition work becomes:
base + __atoi_from(str, sizeof(#base) + 3)
And we can simply convert our old __atoi() to __atoi_from(), change the fixed 0 'from' to a dynamic 'from'. and a simple __get_from() can help us to get the right offset for more complicated cases, such as: (__NR_Linux+1), (__NR_Linux + 1).
So, the new __atoi_add() becomes:
__atoi_add(str, base):
--> __atoi_add(__stringify(__NR_open_tree), __NR_Linux) --> __atoi_add("(4000 + 428)", 4000) --> __atoi_from("(4000 + 428)", sizeof(#4000) + 3) + 4000 --> __atoi_from("(4000 + 428)", 8) + 4000 ~~~~ ^ / ~~~~ base ___/ base from --> 428 + 4000 --> 4428
It is very fast and the cost time is deterministic. It also works for the most complicated case we have mentioned:
__atoi_add("((0x900000+0x0f0000)+5)", (0x900000+0x0f0000))
--> __atoi_from("((0x900000+0x0f0000)+5)", sizeof(#(0x900000+0x0f0000)) + 1) + (0x900000+0x0f0000) ^ / _________________/ --> ... --> 5 + (0x900000+0x0f0000)
So, the calculating of the most complicated part can be simply skipped, we only need to convert the minimal 'dynamic' part from string to integer and since the 'dynamic' part is not that big, most of them may be less than 1000 in the not long future, only 4 characters and therefore only 4-level depth branches for __atoi_from(), so, even with hex 'dynamic' part conversion (but we may don't need it any more), the compile speed is also very fast.
A simple local test on most of the architectures shows, the compile speed is very near to the one with our old proposed NOLIBC__NR_* macros for every __NR_* (defined as (-1L) when __NR_* not defined) and their generated binary size is the same, so, we are near the ultimate solution, but still need more tests. Thanks again for your positive suggestion!
Best regards, Zhangjin
David
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
linux-kselftest-mirror@lists.linaro.org