On Fri, Jan 05, 2018 at 03:33:55PM -0800, Vineet Gupta wrote:
> On 12/13/2017 10:29 PM, Stafford Horne wrote:
> > Traditionally arc has had a generic syscall implementation of syscall()
> > matchig the common implementation.
> >
> > During an audit it was found that this implementation seems to duplicate
> > common implementation and is no longer needed.
> >
> > Signed-off-by: Stafford Horne <shorne at gmail.com>
> > ---
> > libc/sysdeps/linux/arc/Makefile.arch | 2 +-
> > libc/sysdeps/linux/arc/syscall.c | 17 -----------------
> > 2 files changed, 1 insertion(+), 18 deletions(-)
>
> [snip]
> -
> > -extern long syscall(long int sysnum, long a, long b, long c, long d, long e, long f);
> > -
> > -long syscall(long int sysnum, long a, long b, long c, long d, long e, long f)
> > -{
> > - return INLINE_SYSCALL_NCS(sysnum, 6, a, b, c, d, e, f);
> > -}
>
> varargs generate pathetic code on ARC atleast, it is partly due to the ABI
> (the args need to be stored on stack)
>
> With current implementation we get
>
> 000100ac <syscall>:
> 100ac: mov_s r8,r0
> 100ae: mov_s r0,r1
> 100b0: mov_s r1,r2
> 100b2: mov_s r2,r3
> 100b4: mov_s r3,r4
> 100b6: mov_s r4,r5
> 100b8: mov_s r5,r6
> 100ba: trap_s 0
> 100bc: cmp r0,-1024
> 100c0: jls [blink]
> 100c4: st.aw blink,[sp,-4]
> 100c8: bl 84 ;1011c <__syscall_error>
> 100cc: ld.ab blink,[sp,4]
> 100d0: j_s [blink]
> 100d2: nop_s
>
> With varagrs implementation we get tons of useless stores. So I'd prefer we
> keep the current implementation for ARC - although granted syscall() may not
> be the fast path syscall.
Hello Vineet,
Thanks for testing, I did realize varargs might generate slightly less optimal
code. But I figured a few thing things
- syscall() is probably not going to be used in any critical code sections.
- the compiler could be optimized to make the varargs handling more optimal
- we could remove code (but actually ends up with a bigger binary, so not
great)
But for now, lets just drop this patch. Thanks for the feedback.
-Stafford
> 00010398 <syscall>:
> 10398: sub_s sp,sp,0x1c
> 1039a: mov_s r8,r0
> 1039c: st_s r1,[sp,0]
> 1039e: st_s r2,[sp,0x4]
> 103a0: st_s r3,[sp,0x8]
> 103a2: st r4,[sp,12]
> 103a6: st r5,[sp,16]
> 103aa: st r6,[sp,20]
> 103ae: st r7,[sp,24]
> 103b2: mov_s r0,r1
> 103b4: mov_s r1,r2
> 103b6: mov_s r2,r3
> 103b8: mov_s r3,r4
> 103ba: mov_s r4,r5
> 103bc: mov_s r5,r6
> 103be: trap_s 0
> 103c0: brhs.nt 0xfffffc00,r0,20 ;103d4 <syscall+0x3c>
> 103c8: st.aw blink,[sp,-4]
> 103cc: bl 84 ;10420 <__syscall_error>
> 103d0: ld.ab blink,[sp,4]
> 103d4: j_s.d [blink]
> 103d6: add_s sp,sp,0x1c
