[2/3] mips: make sp 16-byte aligned on N64/N32

Message ID 20201128081817.15463-3-huangpei@loongson.cn
State Superseded
Headers
Series [1/3] mips: add hp-timing support for MIPS R2 |

Commit Message

Huang Pei Nov. 28, 2020, 8:18 a.m. UTC
  MIPS N64/N32 ABI request stack pointer be 16-byte alinged

Signed-off-by: Huang Pei <huangpei@loongson.cn>
---
 sysdeps/unix/sysv/linux/mips/mips64/syscall.S | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
  

Comments

Adhemerval Zanella Nov. 30, 2020, 1:03 p.m. UTC | #1
On 28/11/2020 05:18, Huang Pei wrote:
> MIPS N64/N32 ABI request stack pointer be 16-byte alinged
> 

It seems to align what gcc assumes:

gcc/config/mips/mips.h

 364 #define TARGET_NEWABI               (mips_abi == ABI_N32 || mips_abi == ABI_64)

2525 /* Treat LOC as a byte offset from the stack pointer and round it up
2526    to the next fully-aligned offset.  */
2527 #define MIPS_STACK_ALIGN(LOC) \
2528   (TARGET_NEWABI ? ROUND_UP ((LOC), 16) : ROUND_UP ((LOC), 8))

(It would be easier that https://www.linux-mips.org/ could show 
this information easier, I could only find the old o32 ABI documentation).

> Signed-off-by: Huang Pei <huangpei@loongson.cn>

We do not use SCO, but rather Copyright assignments.

Patch looks ok regardless.

> ---
>  sysdeps/unix/sysv/linux/mips/mips64/syscall.S | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/sysdeps/unix/sysv/linux/mips/mips64/syscall.S b/sysdeps/unix/sysv/linux/mips/mips64/syscall.S
> index a9baff3c17..aab1f389aa 100644
> --- a/sysdeps/unix/sysv/linux/mips/mips64/syscall.S
> +++ b/sysdeps/unix/sysv/linux/mips/mips64/syscall.S
> @@ -27,10 +27,10 @@
>  
>  	.text
>  NESTED (syscall, SZREG, ra)
> -	.mask 0x00010000, -SZREG
> +	.mask 0x00010000, -2 * SZREG
>  	.fmask 0x00000000, 0
> -	PTR_ADDIU sp, -SZREG
> -	cfi_adjust_cfa_offset (SZREG)
> +	PTR_ADDIU sp, -2 * SZREG
> +	cfi_adjust_cfa_offset (2 * SZREG)
>  	REG_S s0, (sp)
>  	cfi_rel_offset (s0, 0)
>  
> @@ -48,8 +48,8 @@ NESTED (syscall, SZREG, ra)
>  
>  	REG_L s0, (sp)
>  	cfi_restore (s0)
> -	PTR_ADDIU sp, SZREG
> -	cfi_adjust_cfa_offset (-SZREG)
> +	PTR_ADDIU sp, 2 * SZREG
> +	cfi_adjust_cfa_offset (-2 * 2 * SZREG)
>  	bne a3, zero, L(error)
>  
>  	ret
>
  
Maciej W. Rozycki Dec. 4, 2020, 10:57 a.m. UTC | #2
On Mon, 30 Nov 2020, Adhemerval Zanella via Libc-alpha wrote:

> > MIPS N64/N32 ABI request stack pointer be 16-byte alinged
> > 
> 
> It seems to align what gcc assumes:
> 
> gcc/config/mips/mips.h
> 
>  364 #define TARGET_NEWABI               (mips_abi == ABI_N32 || mips_abi == ABI_64)
> 
> 2525 /* Treat LOC as a byte offset from the stack pointer and round it up
> 2526    to the next fully-aligned offset.  */
> 2527 #define MIPS_STACK_ALIGN(LOC) \
> 2528   (TARGET_NEWABI ? ROUND_UP ((LOC), 16) : ROUND_UP ((LOC), 8))

 This is correct (the alignment being twice the register width), although 
in this particular case obviously hardly matters in practice, as the 
syscall will switch to the kernel stack right away.  It's fine to get it 
fixed regardless of course, once a correct change has been proposed.

> (It would be easier that https://www.linux-mips.org/ could show 
> this information easier, I could only find the old o32 ABI documentation).

 We largely continue relying on old SGI ABI documentation, although there 
used to be a wiki hosted by MIPS Technologies (MTI) where such details 
were summarised.  Sadly with the demise of both companies this stuff is 
either gone or hard to track down, except for individual people's storage 
devices.

 See:

<https://web.archive.org/web/20180829093347/https://dmz-portal.mips.com/wiki/Main_Page>

for the old MTI wiki; unfortunately not all stuff has been archived.

 I can see if I can upload some stuff to linux-mips.org, either the wiki 
itself or my own FTP area I can link the wiki to, but mind that this site 
is under threat of disappearance as well and as much as I'd like to I may 
not be able to stop it.

> > @@ -48,8 +48,8 @@ NESTED (syscall, SZREG, ra)
> >  
> >  	REG_L s0, (sp)
> >  	cfi_restore (s0)
> > -	PTR_ADDIU sp, SZREG
> > -	cfi_adjust_cfa_offset (-SZREG)
> > +	PTR_ADDIU sp, 2 * SZREG
> > +	cfi_adjust_cfa_offset (-2 * 2 * SZREG)

 The updated CFA adjustment does not look right to me though.

  Maciej
  
Adhemerval Zanella Dec. 4, 2020, 12:32 p.m. UTC | #3
On 04/12/2020 07:57, Maciej W. Rozycki wrote:
> On Mon, 30 Nov 2020, Adhemerval Zanella via Libc-alpha wrote:
> 
>>> MIPS N64/N32 ABI request stack pointer be 16-byte alinged
>>>
>>
>> It seems to align what gcc assumes:
>>
>> gcc/config/mips/mips.h
>>
>>  364 #define TARGET_NEWABI               (mips_abi == ABI_N32 || mips_abi == ABI_64)
>>
>> 2525 /* Treat LOC as a byte offset from the stack pointer and round it up
>> 2526    to the next fully-aligned offset.  */
>> 2527 #define MIPS_STACK_ALIGN(LOC) \
>> 2528   (TARGET_NEWABI ? ROUND_UP ((LOC), 16) : ROUND_UP ((LOC), 8))
> 
>  This is correct (the alignment being twice the register width), although 
> in this particular case obviously hardly matters in practice, as the 
> syscall will switch to the kernel stack right away.  It's fine to get it 
> fixed regardless of course, once a correct change has been proposed.

Indeed.

> 
>> (It would be easier that https://www.linux-mips.org/ could show 
>> this information easier, I could only find the old o32 ABI documentation).
> 
>  We largely continue relying on old SGI ABI documentation, although there 
> used to be a wiki hosted by MIPS Technologies (MTI) where such details 
> were summarised.  Sadly with the demise of both companies this stuff is 
> either gone or hard to track down, except for individual people's storage 
> devices.
> 
>  See:
> 
> <https://web.archive.org/web/20180829093347/https://dmz-portal.mips.com/wiki/Main_Page>
> 
> for the old MTI wiki; unfortunately not all stuff has been archived.
> 
>  I can see if I can upload some stuff to linux-mips.org, either the wiki 
> itself or my own FTP area I can link the wiki to, but mind that this site 
> is under threat of disappearance as well and as much as I'd like to I may 
> not be able to stop it.

It would be good if we have a wiki entry referring to all the architecture
and ABI documentation. I will check if I can start one, maybe using the
distro scatter information.

> 
>>> @@ -48,8 +48,8 @@ NESTED (syscall, SZREG, ra)
>>>  
>>>  	REG_L s0, (sp)
>>>  	cfi_restore (s0)
>>> -	PTR_ADDIU sp, SZREG
>>> -	cfi_adjust_cfa_offset (-SZREG)
>>> +	PTR_ADDIU sp, 2 * SZREG
>>> +	cfi_adjust_cfa_offset (-2 * 2 * SZREG)
> 
>  The updated CFA adjustment does not look right to me though.
> 
>   Maciej
>
  

Patch

diff --git a/sysdeps/unix/sysv/linux/mips/mips64/syscall.S b/sysdeps/unix/sysv/linux/mips/mips64/syscall.S
index a9baff3c17..aab1f389aa 100644
--- a/sysdeps/unix/sysv/linux/mips/mips64/syscall.S
+++ b/sysdeps/unix/sysv/linux/mips/mips64/syscall.S
@@ -27,10 +27,10 @@ 
 
 	.text
 NESTED (syscall, SZREG, ra)
-	.mask 0x00010000, -SZREG
+	.mask 0x00010000, -2 * SZREG
 	.fmask 0x00000000, 0
-	PTR_ADDIU sp, -SZREG
-	cfi_adjust_cfa_offset (SZREG)
+	PTR_ADDIU sp, -2 * SZREG
+	cfi_adjust_cfa_offset (2 * SZREG)
 	REG_S s0, (sp)
 	cfi_rel_offset (s0, 0)
 
@@ -48,8 +48,8 @@  NESTED (syscall, SZREG, ra)
 
 	REG_L s0, (sp)
 	cfi_restore (s0)
-	PTR_ADDIU sp, SZREG
-	cfi_adjust_cfa_offset (-SZREG)
+	PTR_ADDIU sp, 2 * SZREG
+	cfi_adjust_cfa_offset (-2 * 2 * SZREG)
 	bne a3, zero, L(error)
 
 	ret