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

[Jeff Johnston]

Patch merged.  Thanks.

-- Jeff J.

On Wed, Dec 20, 2023 at 8:16 PM Xiao Zeng <zengxiao@eswincomputing.com>
wrote:

> 2023-12-21 06:48  Jeff Johnston <jjohnstn@redhat.com> wrote:
> >
>
> >Unless I am missing something, I think the code could be made even simpler
> >to return the pointer when found and return NULL otherwise.
> Yes, you have found a more efficient method.
> After my testing, fewer assembly instructions were generated. This is
> really exciting.
>
> Xiao's patch:
> ---------------------------------------------------------
> libc_a-strpbrk.o:     file format elf64-littleriscv
>
>
> Disassembly of section .text:
>
> 0000000000000000 <strpbrk>:
>    0:   00054683                lbu     a3,0(a0)
>
> 0000000000000004 <.LVL1>:
>    4:   04068063                beqz    a3,44 <.L10>
>    8:   0005c603                lbu     a2,0(a1)
>    c:   04060063                beqz    a2,4c <.L14>
>
> 0000000000000010 <.L5>:
>   10:   00058793                mv      a5,a1
>   14:   00060713                mv      a4,a2
>   18:   00c0006f                j       24 <.L4>
>
> 000000000000001c <.L17>:
>   1c:   0007c703                lbu     a4,0(a5)
>   20:   00070863                beqz    a4,30 <.L16>
>
> 0000000000000024 <.L4>:
>   24:   00178793                addi    a5,a5,1
>
> 0000000000000028 <.LVL5>:
>   28:   fee69ae3                bne     a3,a4,1c <.L17>
>
> 000000000000002c <.L1>:
>   2c:   00008067                ret
>
> 0000000000000030 <.L16>:
>   30:   00154683                lbu     a3,1(a0)
>   34:   00150513                addi    a0,a0,1
>
> 0000000000000038 <.LVL8>:
>   38:   fc069ce3                bnez    a3,10 <.L5>
>
> 000000000000003c <.L6>:
>   3c:   0007c783                lbu     a5,0(a5)
>
> 0000000000000040 <.LVL9>:
>   40:   fe0796e3                bnez    a5,2c <.L1>
>
> 0000000000000044 <.L10>:
>   44:   00000513                li      a0,0
>
> 0000000000000048 <.LVL11>:
>   48:   00008067                ret
>
> 000000000000004c <.L14>:
>   4c:   00154683                lbu     a3,1(a0)
>   50:   00150513                addi    a0,a0,1
>   54:   fe069ce3                bnez    a3,4c <.L14>
>   58:   00058793                mv      a5,a1
>   5c:   fe1ff06f                j       3c <.L6>
> ---------------------------------------------------------
>
> Jeff's patch
> ---------------------------------------------------------
> libc_a-strpbrk.o:     file format elf64-littleriscv
>
>
> Disassembly of section .text:
>
> 0000000000000000 <strpbrk>:
>    0:   00054683                lbu     a3,0(a0)
>    4:   02068c63                beqz    a3,3c <.L8>
>    8:   0005c603                lbu     a2,0(a1)
>    c:   02060c63                beqz    a2,44 <.L12>
>
> 0000000000000010 <.L5>:
>   10:   00058793                mv      a5,a1
>   14:   00060713                mv      a4,a2
>   18:   00c0006f                j       24 <.L4>
>
> 000000000000001c <.L15>:
>   1c:   0007c703                lbu     a4,0(a5)
>   20:   00070863                beqz    a4,30 <.L14>
>
> 0000000000000024 <.L4>:
>   24:   00178793                addi    a5,a5,1
>
> 0000000000000028 <.LVL4>:
>   28:   fed71ae3                bne     a4,a3,1c <.L15>
>   2c:   00008067                ret
>
> 0000000000000030 <.L14>:
>   30:   00154683                lbu     a3,1(a0)
>   34:   00150513                addi    a0,a0,1
>
> 0000000000000038 <.LVL6>:
>   38:   fc069ce3                bnez    a3,10 <.L5>
>
> 000000000000003c <.L8>:
>   3c:   00000513                li      a0,0
>
> 0000000000000040 <.LVL7>:
>   40:   00008067                ret
>
> 0000000000000044 <.L12>:
>   44:   00154683                lbu     a3,1(a0)
>   48:   00150513                addi    a0,a0,1
>   4c:   fe069ce3                bnez    a3,44 <.L12>
>   50:   fedff06f                j       3c <.L8>
> ---------------------------------------------------------
> After careful comparison, it was found that there are fewer assembly
> instructions after the Jeff's patch.
>
> >
> >index 774db1e..d984745 100644
> >--- a/newlib/libc/string/strpbrk.c
> >+++ b/newlib/libc/string/strpbrk.c
> >@@ -29,23 +29,16 @@ strpbrk (const char *s1,
> >        const char *s2)
> > {
> >   const char *c = s2;
> >-  if (!*s1)
> >-    return (char *) NULL;
> >
> >   while (*s1)
> >     {
> >       for (c = s2; *c; c++)
> >        {
> >          if (*s1 == *c)
> >-           break;
> >+           return (char *) s1;
> >        }
> >-      if (*c)
> >-       break;
> >       s1++;
> >     }
> >
> >-  if (*c == '\0')
> >-    s1 = NULL;
> >-
> >-  return (char *) s1;
> >+  return (char *) NULL;
> > }
> >
> >On Wed, Dec 20, 2023 at 1:12 AM Xiao Zeng <zengxiao@eswincomputing.com>
> >wrote:
> >
> >> Signed-off-by: Xiao Zeng <zengxiao@eswincomputing.com>
> >> ---
> >>  newlib/libc/string/strpbrk.c | 5 ++---
> >>  1 file changed, 2 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/newlib/libc/string/strpbrk.c b/newlib/libc/string/strpbrk.c
> >> index 774db1e6d..95e89c20c 100644
> >> --- a/newlib/libc/string/strpbrk.c
> >> +++ b/newlib/libc/string/strpbrk.c
> >> @@ -37,15 +37,14 @@ strpbrk (const char *s1,
> >>        for (c = s2; *c; c++)
> >>         {
> >>           if (*s1 == *c)
> >> -           break;
> >> +           goto end;
> >>         }
> >> -      if (*c)
> >> -       break;
> >>        s1++;
> >>      }
> >>
> >>    if (*c == '\0')
> >>      s1 = NULL;
> >>
> >> +end:
> >>    return (char *) s1;
> >>  }
> >> --
> >> 2.17.1
> >>
> >>
>
> Thanks
> Xiao Zeng
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://sourceware.org/pipermail/newlib/attachments/20231221/eeb2edb1/attachment.htm>