Patch merged to master. -- Jeff J. On Sat, Dec 16, 2023 at 4:31 AM Xiao Zeng wrote: > 2023-12-15 18:28 Torbjorn SVENSSON wrote: > > > > >Hello Xiao, > > > >On 2023-12-15 09:31, Xiao Zeng wrote: > >> Signed-off-by: Xiao Zeng > >> --- > >> 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"). > > > >Kind regards, > >Torbjörn > > After applying this patch, provide a comparison of assembly code > under the risc-v architecture, with default compilation parameters > used in both of them: > > no-patch: > --------------------------------------------------------- > libc_a-strcspn.o: file format elf64-littleriscv > > > Disassembly of section .text: > > 0000000000000000 : > 0: 00054683 lbu a3,0(a0) > 4: 06068263 beqz a3,68 <.L9> > 8: 0005c803 lbu a6,0(a1) > c: 00050613 mv a2,a0 > > 0000000000000010 <.LVL1>: > 10: 02080463 beqz a6,38 <.L14> > > 0000000000000014 <.L6>: > 14: 00058793 mv a5,a1 > 18: 00080713 mv a4,a6 > 1c: 00c0006f j 28 <.L5> > > 0000000000000020 <.L4>: > 20: 0007c703 lbu a4,0(a5) > 24: 02070863 beqz a4,54 <.L17> > > 0000000000000028 <.L5>: > 28: 00178793 addi a5,a5,1 > > 000000000000002c <.LVL5>: > 2c: fee69ae3 bne a3,a4,20 <.L4> > > 0000000000000030 <.L7>: > 30: 40a60533 sub a0,a2,a0 > > 0000000000000034 <.LVL7>: > 34: 00008067 ret > > 0000000000000038 <.L14>: > 38: 00164683 lbu a3,1(a2) > 3c: 00160613 addi a2,a2,1 > 40: fe0688e3 beqz a3,30 <.L7> > 44: 00164683 lbu a3,1(a2) > 48: 00160613 addi a2,a2,1 > 4c: fe0696e3 bnez a3,38 <.L14> > 50: fe1ff06f j 30 <.L7> > > 0000000000000054 <.L17>: > 54: 00164683 lbu a3,1(a2) > 58: 00160613 addi a2,a2,1 > 5c: fa069ce3 bnez a3,14 <.L6> > 60: 40a60533 sub a0,a2,a0 > > 0000000000000064 <.LVL13>: > 64: 00008067 ret > > 0000000000000068 <.L9>: > 68: 00000513 li a0,0 > > 000000000000006c <.LVL15>: > 6c: 00008067 ret > --------------------------------------------------------- > > patch > --------------------------------------------------------- > libc_a-strcspn.o: file format elf64-littleriscv > > > Disassembly of section .text: > > 0000000000000000 : > 0: 00054683 lbu a3,0(a0) > 4: 04068a63 beqz a3,58 <.L2> > 8: 0005c803 lbu a6,0(a1) > c: 00050613 mv a2,a0 > > 0000000000000010 <.LVL1>: > 10: 02080c63 beqz a6,48 <.L14> > > 0000000000000014 <.L6>: > 14: 00058793 mv a5,a1 > 18: 00080713 mv a4,a6 > 1c: 00c0006f j 28 <.L5> > > 0000000000000020 <.L4>: > 20: 0007c703 lbu a4,0(a5) > 24: 00070a63 beqz a4,38 <.L16> > > 0000000000000028 <.L5>: > 28: 00178793 addi a5,a5,1 > > 000000000000002c <.LVL5>: > 2c: fee69ae3 bne a3,a4,20 <.L4> > > 0000000000000030 <.L7>: > 30: 40a60533 sub a0,a2,a0 > > 0000000000000034 <.LVL7>: > 34: 00008067 ret > > 0000000000000038 <.L16>: > 38: 00164683 lbu a3,1(a2) > 3c: 00160613 addi a2,a2,1 > 40: fc069ae3 bnez a3,14 <.L6> > 44: fedff06f j 30 <.L7> > > 0000000000000048 <.L14>: > 48: 00164683 lbu a3,1(a2) > 4c: 00160613 addi a2,a2,1 > 50: fe069ce3 bnez a3,48 <.L14> > 54: fddff06f j 30 <.L7> > > 0000000000000058 <.L2>: > 58: 00000513 li a0,0 > > 000000000000005c <.LVL12>: > 5c: 00008067 ret > --------------------------------------------------------- > After careful comparison, it was found that there are fewer assembly > instructions after the patch. > > Thanks > Xiao Zeng > >