Re: [PATCH] newlib: libc: Improved the readability of strcspn with minor optimization

[Brian Inglis <Brian.Inglis@SystematicSW.ab.ca>]

On 2023-12-19 21:24, Jeff Johnston wrote:
> Patch merged to master.
> On Sat, Dec 16, 2023 at 4:31 AM Xiao Zeng wrote:
>     2023-12-15 18:28  Torbjorn SVENSSON wrote:
>> On 2023-12-15 09:31, Xiao Zeng wrote:
>>> Signed-off-by: Xiao Zeng <zengxiao@eswincomputing.com
>     <mailto:zengxiao@eswincomputing.com>>
>>> ---
>>> newlib/libc/string/strcspn.c | 6 ++----
>>> 1 file changed, 2 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/newlib/libc/string/strcspn.c b/newlib/libc/string/strcspn.c
>>> index abaa93ad6..8ac0bf10c 100644
>>> --- a/newlib/libc/string/strcspn.c
>>> +++ b/newlib/libc/string/strcspn.c
>>> @@ -37,12 +37,10 @@ strcspn (const char *s1,
>      >>         for (c = s2; *c; c++)
>      >>   {
>      >>     if (*s1 == *c)
>      >> -        break;
>      >> +        goto end;
>      >>   }
>      >> -      if (*c)
>      >> -    break;
>      >>         s1++;
>      >>       }
>      >> -
>      >> +end:
>      >>     return s1 - s;
>      >>   }

>> Just looking at this small snippet of code, I would say that the previous
>> code and your suggestion won't do the same thing.
>> 
>> Do you have unit tests that confirm that the behavior is identical with the
>> current implementation and your suggested change?
>> 
>> When I run your suggestion, I get return value 0, but with the current 
>> implementation it's 3 for this call: strspn("129th", "1234567890").

> After applying this patch, provide a comparison of assembly code under the
> risc-v architecture, with default compilation parameters used in both of
> them:
These "micro-optimizations" improve code generation by a few instructions on a 
single (RISC-V) target at a single optimization level of a single compiler and 
version, but what is the cost in execution time and the cache imoact?

Using gotos throw away potential optimizations in modern compilers, where 
goto-less code may have control and/or data flow optimized, with branches 
altered or eliminated, depending on target instruction sets and compiler 
supported optimizations selected and implemented.

For example, in these small functions with few branches, conditional execution 
instructions could be generated, eliminating branches, cache, and lookaside 
buffer impacts, possibly allowing inlining.

Who knows what impacts this has on all of the other targets, compilers, 
versions, and optimization levels?

Should we even consider making these kinds of non-bug-fix minor changes to 
non-target specific sources, unless there are algorithm changes with 
demonstrated benefits across multiple targets, compilers, versions, and 
optimization levels?

-- 
Take care. Thanks, Brian Inglis              Calgary, Alberta, Canada

La perfection est atteinte                   Perfection is achieved
non pas lorsqu'il n'y a plus rien à ajouter  not when there is no more to add
mais lorsqu'il n'y a plus rien à retirer     but when there is no more to cut
                                 -- Antoine de Saint-Exupéry