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