diff mbox series

S390: Remove not needed stack frame in syscall function.

Message ID 0e66f775-9669-ff29-e605-195b79382b3e@linux.ibm.com
State New
Headers show
Series S390: Remove not needed stack frame in syscall function. | expand

Commit Message

Stefan Liebler Oct. 22, 2019, 8 a.m. UTC 
Hi,

As an svc invocation does not clobber any user space registers
despite of the return value r2 and it does not need a special
stack frame. This patch gets rid of the extra frame.
We just have to save and restore r6 and r7 as those are
preserved across function calls.

Bye,
Stefan

Comments

Florian Weimer Oct. 22, 2019, 10:25 a.m. UTC | #1 
* Stefan Liebler:

> As an svc invocation does not clobber any user space registers
> despite of the return value r2 and it does not need a special
> stack frame. This patch gets rid of the extra frame.
> We just have to save and restore r6 and r7 as those are
> preserved across function calls.

Looks okay to me.  Would it be possible to save r6 and r7 in
caller-saved registers not clobbered by the system call?  That might
provide another small benefit.

Thanks,
Florian
Stefan Liebler Oct. 22, 2019, 11:37 a.m. UTC | #2 
On 10/22/19 12:25 PM, Florian Weimer wrote:
> * Stefan Liebler:
> 
>> As an svc invocation does not clobber any user space registers
>> despite of the return value r2 and it does not need a special
>> stack frame. This patch gets rid of the extra frame.
>> We just have to save and restore r6 and r7 as those are
>> preserved across function calls.
> 
> Looks okay to me.  Would it be possible to save r6 and r7 in
> caller-saved registers not clobbered by the system call?  That might
> provide another small benefit.
> 
> Thanks,
> Florian
> 
The syscall itself just clobbers the return value in r2. But for its 
parameters we have to clobber r6 and r7.

According to the ABI, r0-r5 and r14 are volatile.
We need r1 for the syscall number for "svc 0", r2-r7 as parameters for 
svc and r14 is the return-address.

Thus we could use r0 for saving/restoring r6.
For r7 we have the option to either use the register save area on the 
stack-frame prepared by the caller or one of the volatile fprs. But the 
instructions for transferring gpr <-> fpr are not available with all 
architecture level sets. Thus we would need something like #ifdef / 
#else to provide alternative implementations.

Therefore I think just storing/restoring both registers at once with one 
stmg/lmg instruction is okay.

Thanks,
Stefan
Florian Weimer Oct. 22, 2019, 11:44 a.m. UTC | #3 
* Stefan Liebler:

> The syscall itself just clobbers the return value in r2. But for its
> parameters we have to clobber r6 and r7.
>
> According to the ABI, r0-r5 and r14 are volatile.
> We need r1 for the syscall number for "svc 0", r2-r7 as parameters for
> svc and r14 is the return-address.
>
> Thus we could use r0 for saving/restoring r6.
> For r7 we have the option to either use the register save area on the
> stack-frame prepared by the caller or one of the volatile fprs. But
> the instructions for transferring gpr <-> fpr are not available with
> all architecture level sets. Thus we would need something like #ifdef
> / #else to provide alternative implementations.
>
> Therefore I think just storing/restoring both registers at once with
> one stmg/lmg instruction is okay.

I see.  I didn't know the ABI details.

Thanks,
Florian
Stefan Liebler Oct. 23, 2019, 12:53 p.m. UTC | #4 
On 10/22/19 1:44 PM, Florian Weimer wrote:
> * Stefan Liebler:
> 
>> The syscall itself just clobbers the return value in r2. But for its
>> parameters we have to clobber r6 and r7.
>>
>> According to the ABI, r0-r5 and r14 are volatile.
>> We need r1 for the syscall number for "svc 0", r2-r7 as parameters for
>> svc and r14 is the return-address.
>>
>> Thus we could use r0 for saving/restoring r6.
>> For r7 we have the option to either use the register save area on the
>> stack-frame prepared by the caller or one of the volatile fprs. But
>> the instructions for transferring gpr <-> fpr are not available with
>> all architecture level sets. Thus we would need something like #ifdef
>> / #else to provide alternative implementations.
>>
>> Therefore I think just storing/restoring both registers at once with
>> one stmg/lmg instruction is okay.
> 
> I see.  I didn't know the ABI details.
> 
> Thanks,
> Florian
> 

Committed.
Thanks
diff mbox series

Patch

commit 1349ca6793627f62d0fe96d5e0891e2e0166ba90
Author: Stefan Liebler <stli@linux.ibm.com>
Date:   Thu Oct 17 13:49:03 2019 +0200

    S390: Remove not needed stack frame in syscall function.
    
    As an svc invocation does not clobber any user space registers
    despite of the return value r2 and it does not need a special
    stack frame. This patch gets rid of the extra frame.
    We just have to save and restore r6 and r7 as those are
    preserved across function calls.

diff --git a/sysdeps/unix/sysv/linux/s390/s390-32/syscall.S b/sysdeps/unix/sysv/linux/s390/s390-32/syscall.S
index 69561d0b76..7ed36c0714 100644
--- a/sysdeps/unix/sysv/linux/s390/s390-32/syscall.S
+++ b/sysdeps/unix/sysv/linux/s390/s390-32/syscall.S
@@ -22,37 +22,19 @@ 
    more information about the value -4095 used below.*/
 
 ENTRY (syscall)
-	/* Save registers and setup stack.  */
-	stm     %r6,%r15,24(%r15)  /* save registers */
-	cfi_offset (%r15, -36)
-	cfi_offset (%r14, -40)
-	cfi_offset (%r13, -44)
-	cfi_offset (%r12, -48)
-	cfi_offset (%r11, -52)
-	cfi_offset (%r10, -56)
-	cfi_offset (%r9, -60)
-	cfi_offset (%r8, -64)
+	stm     %r6,%r7,24(%r15)  /* save registers */
 	cfi_offset (%r7, -68)
 	cfi_offset (%r6, -72)
-	lr      %r1,%r15
-	l       %r0,4(0,%r15)      /* load eos */
-	ahi     %r15,-96           /* buy stack space */
-	cfi_adjust_cfa_offset (96)
-	st      %r1,0(0,%r15)      /* store back chain */
-	st      %r0,4(0,%r15)      /* store eos */
 
 	lr     %r1,%r2             /* move syscall number */
 	lr     %r2,%r3             /* first parameter  */
 	lr     %r3,%r4             /* second parameter */
 	lr     %r4,%r5             /* third parameter  */
 	lr     %r5,%r6             /* fourth parameter */
-	l      %r6,192(%r15)       /* fifth parameter  */
-	l      %r7,196(%r15)       /* sixth parameter  */
-
+	lm     %r6,%r7,96(%r15)    /* fifth / sixth parameter  */
 	svc    0
-	l      %r15,0(%r15)        /* load back chain.  */
-	cfi_adjust_cfa_offset (-96)
-	lm     %r6,%r15,24(%r15)   /* load registers.  */
+
+	lm     %r6,%r7,24(%r15)    /* load registers.  */
 
 	lhi    %r0,-4095
 	clr    %r2,%r0             /* check R2 for error */
diff --git a/sysdeps/unix/sysv/linux/s390/s390-64/syscall.S b/sysdeps/unix/sysv/linux/s390/s390-64/syscall.S
index bbe4d79848..442f9e4878 100644
--- a/sysdeps/unix/sysv/linux/s390/s390-64/syscall.S
+++ b/sysdeps/unix/sysv/linux/s390/s390-64/syscall.S
@@ -22,37 +22,19 @@ 
    more information about the value -4095 used below.*/
 
 ENTRY (syscall)
-	/* Save registers and setup stack.  */
-	stmg	%r6,%r15,48(%r15)  /* Save registers.  */
-	cfi_offset (%r15,-40)
-	cfi_offset (%r14,-48)
-	cfi_offset (%r13,-56)
-	cfi_offset (%r12,-64)
-	cfi_offset (%r11,-72)
-	cfi_offset (%r10,-80)
-	cfi_offset (%r9,-88)
-	cfi_offset (%r8,-96)
+	stmg	%r6,%r7,48(%r15)  /* Save registers.  */
 	cfi_offset (%r7,-104)
 	cfi_offset (%r6,-112)
-	lgr	%r1,%r15
-	lg	%r0,8(%r15)	   /* Load eos.	 */
-	aghi	%r15,-160	   /* Buy stack space.	*/
-	cfi_adjust_cfa_offset (160)
-	stg	%r1,0(%r15)	   /* Store back chain.	 */
-	stg	%r0,8(%r15)	   /* Store eos.  */
 
 	lgr    %r1,%r2		   /* Move syscall number.  */
 	lgr    %r2,%r3		   /* First parameter.	*/
 	lgr    %r3,%r4		   /* Second parameter.	 */
 	lgr    %r4,%r5		   /* Third parameter.	*/
 	lgr    %r5,%r6		   /* Fourth parameter.	 */
-	lg     %r6,320(%r15)	   /* Fifth parameter.	*/
-	lg     %r7,328(%r15)	   /* Sixth parameter.	*/
-
+	lmg    %r6,%r7,160(%r15)   /* Fifth / Sixth parameter.	*/
 	svc    0
-	lg     %r15,0(%r15)        /* Load back chain.  */
-	cfi_adjust_cfa_offset (-160)
-	lmg    %r6,%r15,48(%r15)   /* Load registers.  */
+
+	lmg    %r6,%r7,48(%r15)    /* Load registers.  */
 
 	lghi   %r0,-4095
 	clgr   %r2,%r0		   /* Check R2 for error.  */