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

2023-12-21 06:48  Jeff Johnston <[email protected]> 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 <[email protected]>
>wrote:
>
>> Signed-off-by: Xiao Zeng <[email protected]>
>> ---
>>  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