public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [Patch V3]: add hp-timing support and minor fix
@ 2020-11-28  8:18 Huang Pei
  2020-11-28  8:18 ` [PATCH 1/3] mips: add hp-timing support for MIPS R2 Huang Pei
                   ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Huang Pei @ 2020-11-28  8:18 UTC (permalink / raw)
  To: Joseph Myers; +Cc: libc-alpha, Huacai Chen, Chenghua Xu

Patch 1: add discription for Linux/MIPS emulation for 'rdhwr %0, $2',
so it is ok to run in unsupported hardware, but not recommended

Patch 3: add discription for old syscall restart convention, explain
why



^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 1/3] mips: add hp-timing support for MIPS R2
  2020-11-28  8:18 [Patch V3]: add hp-timing support and minor fix Huang Pei
@ 2020-11-28  8:18 ` Huang Pei
  2020-11-30 12:25   ` Adhemerval Zanella
  2020-11-28  8:18 ` [PATCH 2/3] mips: make sp 16-byte aligned on N64/N32 Huang Pei
  2020-11-28  8:18 ` [PATCH 3/3] mips: remove register spill Huang Pei
  2 siblings, 1 reply; 17+ messages in thread
From: Huang Pei @ 2020-11-28  8:18 UTC (permalink / raw)
  To: Joseph Myers; +Cc: libc-alpha, Huacai Chen, Chenghua Xu

MIPS R2 only support 32 bit TSC(AKA "rdhwr %0, $2"), but it should be
enough for glibc.

DO remember Linux/MIPS added emulation for 'rdhwr %0, $2',when disabled
or not supported, which would make the precision worse. If you got
unreasonable result, check your CPU Manual for whether your CPU
implemnted it or not

Signed-off-by: Huang Pei <huangpei@loongson.cn>
---
 sysdeps/mips/hp-timing.h | 43 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)
 create mode 100644 sysdeps/mips/hp-timing.h

diff --git a/sysdeps/mips/hp-timing.h b/sysdeps/mips/hp-timing.h
new file mode 100644
index 0000000000..128315d0bf
--- /dev/null
+++ b/sysdeps/mips/hp-timing.h
@@ -0,0 +1,43 @@
+/* High precision, low overhead timing functions. MIPS version.
+   Copyright (C) 2002-2020 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#ifndef _HP_TIMING_H
+#define _HP_TIMING_H	1
+
+#if __mips_isa_rev >= 2
+/* We always assume having the timestamp register.  */
+#define HP_TIMING_AVAIL		(0)
+#define HP_SMALL_TIMING_AVAIL	(1)
+
+/* We indeed have inlined functions.  */
+#define HP_TIMING_INLINE	(1)
+
+/* We use 32bit values for the times.  */
+typedef unsigned int hp_timing_t;
+
+/* Read the cp0 count, this maybe inaccurate.  */
+#define HP_TIMING_NOW(Var) \
+  ({ unsigned int _count; \
+     asm volatile ("rdhwr\t%0,$2" : "=r" (_count)); \
+     (Var) = _count; })
+
+#include <hp-timing-common.h>
+
+#endif
+
+#endif /* hp-timing.h */
-- 
2.17.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 2/3] mips: make sp 16-byte aligned on N64/N32
  2020-11-28  8:18 [Patch V3]: add hp-timing support and minor fix Huang Pei
  2020-11-28  8:18 ` [PATCH 1/3] mips: add hp-timing support for MIPS R2 Huang Pei
@ 2020-11-28  8:18 ` Huang Pei
  2020-11-30 13:03   ` Adhemerval Zanella
  2020-11-28  8:18 ` [PATCH 3/3] mips: remove register spill Huang Pei
  2 siblings, 1 reply; 17+ messages in thread
From: Huang Pei @ 2020-11-28  8:18 UTC (permalink / raw)
  To: Joseph Myers; +Cc: libc-alpha, Huacai Chen, Chenghua Xu

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(-)

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
-- 
2.17.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 3/3] mips: remove register spill
  2020-11-28  8:18 [Patch V3]: add hp-timing support and minor fix Huang Pei
  2020-11-28  8:18 ` [PATCH 1/3] mips: add hp-timing support for MIPS R2 Huang Pei
  2020-11-28  8:18 ` [PATCH 2/3] mips: make sp 16-byte aligned on N64/N32 Huang Pei
@ 2020-11-28  8:18 ` Huang Pei
  2020-11-30 14:22   ` Adhemerval Zanella
  2 siblings, 1 reply; 17+ messages in thread
From: Huang Pei @ 2020-11-28  8:18 UTC (permalink / raw)
  To: Joseph Myers; +Cc: libc-alpha, Huacai Chen, Chenghua Xu

Before Linux/MIPS 2.6.36, kernel expected setting syscall number(aka
"li v0, #sys_number") right precedes "syscall", so the kernel syscall
restart sequence can use CP0 EPC - 4 to restart the syscall, because
kernel DID NOT save v0 during syscall handling. Linux 2.6.36 canceled
this restriction.

See sysdeps/unix/sysv/linux/mips/{mips32/sysdep.h,mips64/sysdep.h,sysdep.h}

Since glibc-2.24 the minimum kernel version is 3.2(much higer than
2.6.36), I think it is OK to remove the ugly register spill in
syscall.S just because of the old convention

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

diff --git a/sysdeps/unix/sysv/linux/mips/mips64/syscall.S b/sysdeps/unix/sysv/linux/mips/mips64/syscall.S
index aab1f389aa..089524a40b 100644
--- a/sysdeps/unix/sysv/linux/mips/mips64/syscall.S
+++ b/sysdeps/unix/sysv/linux/mips/mips64/syscall.S
@@ -27,14 +27,9 @@
 
 	.text
 NESTED (syscall, SZREG, ra)
-	.mask 0x00010000, -2 * SZREG
+	.mask 0x00000000, 0
 	.fmask 0x00000000, 0
-	PTR_ADDIU sp, -2 * SZREG
-	cfi_adjust_cfa_offset (2 * SZREG)
-	REG_S s0, (sp)
-	cfi_rel_offset (s0, 0)
-
-	move s0, a0
+	move v0, a0
 	move a0, a1		/* shift arg1 - arg7.  */
 	move a1, a2
 	move a2, a3
@@ -43,13 +38,8 @@ NESTED (syscall, SZREG, ra)
 	move a5, a6
 	move a6, a7
 
-	move v0, s0		/* Syscall number -> v0 */
 	syscall			/* Do the system call.  */
 
-	REG_L s0, (sp)
-	cfi_restore (s0)
-	PTR_ADDIU sp, 2 * SZREG
-	cfi_adjust_cfa_offset (-2 * 2 * SZREG)
 	bne a3, zero, L(error)
 
 	ret
-- 
2.17.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/3] mips: add hp-timing support for MIPS R2
  2020-11-28  8:18 ` [PATCH 1/3] mips: add hp-timing support for MIPS R2 Huang Pei
@ 2020-11-30 12:25   ` Adhemerval Zanella
  2020-12-04 10:58     ` Maciej W. Rozycki
  0 siblings, 1 reply; 17+ messages in thread
From: Adhemerval Zanella @ 2020-11-30 12:25 UTC (permalink / raw)
  To: libc-alpha



On 28/11/2020 05:18, Huang Pei wrote:
> MIPS R2 only support 32 bit TSC(AKA "rdhwr %0, $2"), but it should be
> enough for glibc.
> 
> DO remember Linux/MIPS added emulation for 'rdhwr %0, $2',when disabled
> or not supported, which would make the precision worse. If you got
> unreasonable result, check your CPU Manual for whether your CPU
> implemnted it or not

It seems that rdhwr trap simulation has been on kernel since 2.6.15-rc1
(3c37026d43c47be), so it should be safe to assume current minimum kernel
support for it.

However it does not only make the precision worse, but a missing rdwhr
support would also slow down the loader since the hp-timing support is
assumed to be 'fast' and loader code will use it regardless 
LD_DEBUG=statistics is set or not.  It should be suffice to enabled it
iff __mips_isa_rev is higher than 2, but do you know if there ARE chips
which implement ISA higher than r2 that do not support this instruction?
If so, how common are they?

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

We do not use SCO, but rather Copyright assignments.

> ---
>  sysdeps/mips/hp-timing.h | 43 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
>  create mode 100644 sysdeps/mips/hp-timing.h
> 
> diff --git a/sysdeps/mips/hp-timing.h b/sysdeps/mips/hp-timing.h
> new file mode 100644
> index 0000000000..128315d0bf
> --- /dev/null
> +++ b/sysdeps/mips/hp-timing.h
> @@ -0,0 +1,43 @@
> +/* High precision, low overhead timing functions. MIPS version.
> +   Copyright (C) 2002-2020 Free Software Foundation, Inc.

I think it should be on 2020.

> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#ifndef _HP_TIMING_H
> +#define _HP_TIMING_H	1
> +
> +#if __mips_isa_rev >= 2
> +/* We always assume having the timestamp register.  */
> +#define HP_TIMING_AVAIL		(0)
> +#define HP_SMALL_TIMING_AVAIL	(1)

It seems to be based on a older glibc version, the hp-timing.h has been
refactored on 2.30 (1e372ded4f83362) and since then simplified a bit.

Please check the generic interface 'sysdeps/generic/hp-timing.h' to
see how to proceed it.  For instance, alpha has a similar constraint
where its timestamp register only has 4s range and then it is only 
enabled for ld.so statistics.

I am not sure which the usual frequency for mips chips, so maybe we
should also enable it only for loader (since using it on benchmarks
might get us bogus value due overflow).  So you might need to do
something like:

  #if IS_IN(rtld) && __mips_isa_rev >= 2
  typedef unsigned int hp_timing_t;
  # define HP_TIMING_NOW(Var) \
   ({ unsigned int _count; \
     asm volatile ("rdhwr\t%0,$2" : "=r" (_count)); \
     (Var) = _count; })
  #else
  # include <sysdeps/generic/hp-timing.h>
  #endif

Also, the generic implementation uses clock_gettime and which a recent
kernel it will use the vDSO and thus reducing latency).

> +
> +/* We indeed have inlined functions.  */
> +#define HP_TIMING_INLINE	(1)
> +
> +/* We use 32bit values for the times.  */
> +typedef unsigned int hp_timing_t;
> +
> +/* Read the cp0 count, this maybe inaccurate.  */
> +#define HP_TIMING_NOW(Var) \
> +  ({ unsigned int _count; \
> +     asm volatile ("rdhwr\t%0,$2" : "=r" (_count)); \
> +     (Var) = _count; })
> +
> +#include <hp-timing-common.h>
> +
> +#endif
> +
> +#endif /* hp-timing.h */
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] mips: make sp 16-byte aligned on N64/N32
  2020-11-28  8:18 ` [PATCH 2/3] mips: make sp 16-byte aligned on N64/N32 Huang Pei
@ 2020-11-30 13:03   ` Adhemerval Zanella
  2020-12-04 10:57     ` Maciej W. Rozycki
  0 siblings, 1 reply; 17+ messages in thread
From: Adhemerval Zanella @ 2020-11-30 13:03 UTC (permalink / raw)
  To: libc-alpha, Huacai Chen, Chenghua Xu



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
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/3] mips: remove register spill
  2020-11-28  8:18 ` [PATCH 3/3] mips: remove register spill Huang Pei
@ 2020-11-30 14:22   ` Adhemerval Zanella
  2020-12-01  9:39     ` Huang Pei
  0 siblings, 1 reply; 17+ messages in thread
From: Adhemerval Zanella @ 2020-11-30 14:22 UTC (permalink / raw)
  To: Huang Pei, Joseph Myers; +Cc: Huacai Chen, Chenghua Xu, libc-alpha



On 28/11/2020 05:18, Huang Pei wrote:
> Before Linux/MIPS 2.6.36, kernel expected setting syscall number(aka
> "li v0, #sys_number") right precedes "syscall", so the kernel syscall
> restart sequence can use CP0 EPC - 4 to restart the syscall, because
> kernel DID NOT save v0 during syscall handling. Linux 2.6.36 canceled
> this restriction.
> 
> See sysdeps/unix/sysv/linux/mips/{mips32/sysdep.h,mips64/sysdep.h,sysdep.h}
> 
> Since glibc-2.24 the minimum kernel version is 3.2(much higer than
> 2.6.36), I think it is OK to remove the ugly register spill in
> syscall.S just because of the old convention
> 
> Signed-off-by: Huang Pei <huangpei@loongson.cn>

We do not use SCO, but rather Copyright assignment.	

The rest of the patch looks ok.

> ---
>  sysdeps/unix/sysv/linux/mips/mips64/syscall.S | 14 ++------------
>  1 file changed, 2 insertions(+), 12 deletions(-)
> 
> diff --git a/sysdeps/unix/sysv/linux/mips/mips64/syscall.S b/sysdeps/unix/sysv/linux/mips/mips64/syscall.S
> index aab1f389aa..089524a40b 100644
> --- a/sysdeps/unix/sysv/linux/mips/mips64/syscall.S
> +++ b/sysdeps/unix/sysv/linux/mips/mips64/syscall.S
> @@ -27,14 +27,9 @@
>  
>  	.text
>  NESTED (syscall, SZREG, ra)
> -	.mask 0x00010000, -2 * SZREG
> +	.mask 0x00000000, 0
>  	.fmask 0x00000000, 0
> -	PTR_ADDIU sp, -2 * SZREG
> -	cfi_adjust_cfa_offset (2 * SZREG)
> -	REG_S s0, (sp)
> -	cfi_rel_offset (s0, 0)
> -
> -	move s0, a0
> +	move v0, a0
>  	move a0, a1		/* shift arg1 - arg7.  */
>  	move a1, a2
>  	move a2, a3
> @@ -43,13 +38,8 @@ NESTED (syscall, SZREG, ra)
>  	move a5, a6
>  	move a6, a7
>  
> -	move v0, s0		/* Syscall number -> v0 */
>  	syscall			/* Do the system call.  */
>  
> -	REG_L s0, (sp)
> -	cfi_restore (s0)
> -	PTR_ADDIU sp, 2 * SZREG
> -	cfi_adjust_cfa_offset (-2 * 2 * SZREG)
>  	bne a3, zero, L(error)
>  
>  	ret
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/3] mips: remove register spill
  2020-11-30 14:22   ` Adhemerval Zanella
@ 2020-12-01  9:39     ` Huang Pei
  2020-12-01 11:58       ` Adhemerval Zanella
  0 siblings, 1 reply; 17+ messages in thread
From: Huang Pei @ 2020-12-01  9:39 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: Joseph Myers, Huacai Chen, Chenghua Xu, libc-alpha

hi,

On Mon, Nov 30, 2020 at 11:22:03AM -0300, Adhemerval Zanella wrote:
> 
> 
> On 28/11/2020 05:18, Huang Pei wrote:
> > Before Linux/MIPS 2.6.36, kernel expected setting syscall number(aka
> > "li v0, #sys_number") right precedes "syscall", so the kernel syscall
> > restart sequence can use CP0 EPC - 4 to restart the syscall, because
> > kernel DID NOT save v0 during syscall handling. Linux 2.6.36 canceled
> > this restriction.
> > 
> > See sysdeps/unix/sysv/linux/mips/{mips32/sysdep.h,mips64/sysdep.h,sysdep.h}
> > 
> > Since glibc-2.24 the minimum kernel version is 3.2(much higer than
> > 2.6.36), I think it is OK to remove the ugly register spill in
> > syscall.S just because of the old convention
> > 
> > Signed-off-by: Huang Pei <huangpei@loongson.cn>
> 
> We do not use SCO, but rather Copyright assignment.	
> 
> The rest of the patch looks ok.
> 

I would like to combine the Patch 2/3 and Patch 3/3 into one patch,
since Patch 2/3 only fix sp alignment, but the key point is folllowing syscall
restart convention, any advice?

> > ---
> >  sysdeps/unix/sysv/linux/mips/mips64/syscall.S | 14 ++------------
> >  1 file changed, 2 insertions(+), 12 deletions(-)
> > 
> > diff --git a/sysdeps/unix/sysv/linux/mips/mips64/syscall.S b/sysdeps/unix/sysv/linux/mips/mips64/syscall.S
> > index aab1f389aa..089524a40b 100644
> > --- a/sysdeps/unix/sysv/linux/mips/mips64/syscall.S
> > +++ b/sysdeps/unix/sysv/linux/mips/mips64/syscall.S
> > @@ -27,14 +27,9 @@
> >  
> >  	.text
> >  NESTED (syscall, SZREG, ra)
> > -	.mask 0x00010000, -2 * SZREG
> > +	.mask 0x00000000, 0
> >  	.fmask 0x00000000, 0
> > -	PTR_ADDIU sp, -2 * SZREG
> > -	cfi_adjust_cfa_offset (2 * SZREG)
> > -	REG_S s0, (sp)
> > -	cfi_rel_offset (s0, 0)
> > -
> > -	move s0, a0
> > +	move v0, a0
> >  	move a0, a1		/* shift arg1 - arg7.  */
> >  	move a1, a2
> >  	move a2, a3
> > @@ -43,13 +38,8 @@ NESTED (syscall, SZREG, ra)
> >  	move a5, a6
> >  	move a6, a7
> >  
> > -	move v0, s0		/* Syscall number -> v0 */
> >  	syscall			/* Do the system call.  */
> >  
> > -	REG_L s0, (sp)
> > -	cfi_restore (s0)
> > -	PTR_ADDIU sp, 2 * SZREG
> > -	cfi_adjust_cfa_offset (-2 * 2 * SZREG)
> >  	bne a3, zero, L(error)
> >  
> >  	ret
> > 


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/3] mips: remove register spill
  2020-12-01  9:39     ` Huang Pei
@ 2020-12-01 11:58       ` Adhemerval Zanella
  2020-12-04 11:03         ` Maciej W. Rozycki
  0 siblings, 1 reply; 17+ messages in thread
From: Adhemerval Zanella @ 2020-12-01 11:58 UTC (permalink / raw)
  To: Huang Pei; +Cc: Joseph Myers, Huacai Chen, Chenghua Xu, libc-alpha



On 01/12/2020 06:39, Huang Pei wrote:
> hi,
> 
> On Mon, Nov 30, 2020 at 11:22:03AM -0300, Adhemerval Zanella wrote:
>>
>>
>> On 28/11/2020 05:18, Huang Pei wrote:
>>> Before Linux/MIPS 2.6.36, kernel expected setting syscall number(aka
>>> "li v0, #sys_number") right precedes "syscall", so the kernel syscall
>>> restart sequence can use CP0 EPC - 4 to restart the syscall, because
>>> kernel DID NOT save v0 during syscall handling. Linux 2.6.36 canceled
>>> this restriction.
>>>
>>> See sysdeps/unix/sysv/linux/mips/{mips32/sysdep.h,mips64/sysdep.h,sysdep.h}
>>>
>>> Since glibc-2.24 the minimum kernel version is 3.2(much higer than
>>> 2.6.36), I think it is OK to remove the ugly register spill in
>>> syscall.S just because of the old convention
>>>
>>> Signed-off-by: Huang Pei <huangpei@loongson.cn>
>>
>> We do not use SCO, but rather Copyright assignment.	
>>
>> The rest of the patch looks ok.
>>
> 
> I would like to combine the Patch 2/3 and Patch 3/3 into one patch,
> since Patch 2/3 only fix sp alignment, but the key point is folllowing syscall
> restart convention, any advice?

Send a v2 with the patches merged and the hp-inline fix.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] mips: make sp 16-byte aligned on N64/N32
  2020-11-30 13:03   ` Adhemerval Zanella
@ 2020-12-04 10:57     ` Maciej W. Rozycki
  2020-12-04 12:32       ` Adhemerval Zanella
  0 siblings, 1 reply; 17+ messages in thread
From: Maciej W. Rozycki @ 2020-12-04 10:57 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: libc-alpha, Huacai Chen, Chenghua Xu

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/3] mips: add hp-timing support for MIPS R2
  2020-11-30 12:25   ` Adhemerval Zanella
@ 2020-12-04 10:58     ` Maciej W. Rozycki
  2020-12-04 12:31       ` Adhemerval Zanella
  0 siblings, 1 reply; 17+ messages in thread
From: Maciej W. Rozycki @ 2020-12-04 10:58 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: libc-alpha

On Mon, 30 Nov 2020, Adhemerval Zanella via Libc-alpha wrote:

> > MIPS R2 only support 32 bit TSC(AKA "rdhwr %0, $2"), but it should be
> > enough for glibc.
> > 
> > DO remember Linux/MIPS added emulation for 'rdhwr %0, $2',when disabled
> > or not supported, which would make the precision worse. If you got
> > unreasonable result, check your CPU Manual for whether your CPU
> > implemnted it or not
> 
> It seems that rdhwr trap simulation has been on kernel since 2.6.15-rc1
> (3c37026d43c47be), so it should be safe to assume current minimum kernel
> support for it.

 Nope, that was commit 1f5826bd0ed6c ("[MIPS] Added missing cases for 
rdhwr emulation") and Linux 2.6.25 respectively.  The other commit only 
added User Local Register (ULR) access emulation for TLS pointer access.

 Also CP0 Counter (CC) register access emulation relies on `read_c0_count',
which is NOT universally supported, as not all MIPS CPUs have the CC, in 
which case rubbish will be returned (the relevant instruction does not 
trap on invalid CP0 register accesses).

 None of this seems to matter however if we only handle this for R2 and 
higher ISA versions where both the CC and RDHWR are required by the 
architecture.

> However it does not only make the precision worse, but a missing rdwhr
> support would also slow down the loader since the hp-timing support is
> assumed to be 'fast' and loader code will use it regardless 
> LD_DEBUG=statistics is set or not.  It should be suffice to enabled it
> iff __mips_isa_rev is higher than 2, but do you know if there ARE chips
> which implement ISA higher than r2 that do not support this instruction?
> If so, how common are they?

 The RDHWR instruction is mandatory, though access to individual registers 
is controlled by the kernel, via the CP0 HWREna register bitmask.  We have 
had access enabled to registers 3:0 (CC is $2) unconditionally however:

	if (cpu_has_mips_r2_r6)
		hwrena |= MIPS_HWRENA_CPUNUM |
			  MIPS_HWRENA_SYNCISTEP |
			  MIPS_HWRENA_CC |
			  MIPS_HWRENA_CCRES;

ever since commit e01402b115cc ("More AP / SP bits for the 34K, the Malta 
bits and things."), that is Linux 2.6.15, and we can consider that a part 
of the Linux ABI.  Mentioning emulation in the context of R2 is therefore 
misleading in my opinion and the change description needs to be rewritten.

  Maciej

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/3] mips: remove register spill
  2020-12-01 11:58       ` Adhemerval Zanella
@ 2020-12-04 11:03         ` Maciej W. Rozycki
  2020-12-04 12:33           ` Adhemerval Zanella
  0 siblings, 1 reply; 17+ messages in thread
From: Maciej W. Rozycki @ 2020-12-04 11:03 UTC (permalink / raw)
  To: Adhemerval Zanella
  Cc: Huang Pei, Huacai Chen, Chenghua Xu, libc-alpha, Joseph Myers

On Tue, 1 Dec 2020, Adhemerval Zanella via Libc-alpha wrote:

> > I would like to combine the Patch 2/3 and Patch 3/3 into one patch,
> > since Patch 2/3 only fix sp alignment, but the key point is folllowing syscall
> > restart convention, any advice?
> 
> Send a v2 with the patches merged and the hp-inline fix.

 I would defer the removal of the old syscall restart convention in this 
single place only.  I don't think we need to introduce such a mess.

 Instead an audit should be made and all the places adjusted with a single 
change.  That would include at least MIPS16 wrappers, and IIRC some 
microMIPS support code, and then the comment I referred previously needs 
to be adjusted accordingly (maybe removed altogether even).

  Maciej

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/3] mips: add hp-timing support for MIPS R2
  2020-12-04 10:58     ` Maciej W. Rozycki
@ 2020-12-04 12:31       ` Adhemerval Zanella
  0 siblings, 0 replies; 17+ messages in thread
From: Adhemerval Zanella @ 2020-12-04 12:31 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: libc-alpha



On 04/12/2020 07:58, Maciej W. Rozycki wrote:
> On Mon, 30 Nov 2020, Adhemerval Zanella via Libc-alpha wrote:
> 
>>> MIPS R2 only support 32 bit TSC(AKA "rdhwr %0, $2"), but it should be
>>> enough for glibc.
>>>
>>> DO remember Linux/MIPS added emulation for 'rdhwr %0, $2',when disabled
>>> or not supported, which would make the precision worse. If you got
>>> unreasonable result, check your CPU Manual for whether your CPU
>>> implemnted it or not
>>
>> It seems that rdhwr trap simulation has been on kernel since 2.6.15-rc1
>> (3c37026d43c47be), so it should be safe to assume current minimum kernel
>> support for it.
> 
>  Nope, that was commit 1f5826bd0ed6c ("[MIPS] Added missing cases for 
> rdhwr emulation") and Linux 2.6.25 respectively.  The other commit only 
> added User Local Register (ULR) access emulation for TLS pointer access.
> 
>  Also CP0 Counter (CC) register access emulation relies on `read_c0_count',
> which is NOT universally supported, as not all MIPS CPUs have the CC, in 
> which case rubbish will be returned (the relevant instruction does not 
> trap on invalid CP0 register accesses).
> 
>  None of this seems to matter however if we only handle this for R2 and 
> higher ISA versions where both the CC and RDHWR are required by the 
> architecture.
> 
>> However it does not only make the precision worse, but a missing rdwhr
>> support would also slow down the loader since the hp-timing support is
>> assumed to be 'fast' and loader code will use it regardless 
>> LD_DEBUG=statistics is set or not.  It should be suffice to enabled it
>> iff __mips_isa_rev is higher than 2, but do you know if there ARE chips
>> which implement ISA higher than r2 that do not support this instruction?
>> If so, how common are they?
> 
>  The RDHWR instruction is mandatory, though access to individual registers 
> is controlled by the kernel, via the CP0 HWREna register bitmask.  We have 
> had access enabled to registers 3:0 (CC is $2) unconditionally however:
> 
> 	if (cpu_has_mips_r2_r6)
> 		hwrena |= MIPS_HWRENA_CPUNUM |
> 			  MIPS_HWRENA_SYNCISTEP |
> 			  MIPS_HWRENA_CC |
> 			  MIPS_HWRENA_CCRES;
> 
> ever since commit e01402b115cc ("More AP / SP bits for the 34K, the Malta 
> bits and things."), that is Linux 2.6.15, and we can consider that a part 
> of the Linux ABI.  Mentioning emulation in the context of R2 is therefore 
> misleading in my opinion and the change description needs to be rewritten.
> 
>   Maciej
> 

Thanks for the thoroughly explanation, it answer my concerns about enabling 
hp-timing for MIPS R2.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] mips: make sp 16-byte aligned on N64/N32
  2020-12-04 10:57     ` Maciej W. Rozycki
@ 2020-12-04 12:32       ` Adhemerval Zanella
  0 siblings, 0 replies; 17+ messages in thread
From: Adhemerval Zanella @ 2020-12-04 12:32 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: libc-alpha, Huacai Chen, Chenghua Xu



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
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/3] mips: remove register spill
  2020-12-04 11:03         ` Maciej W. Rozycki
@ 2020-12-04 12:33           ` Adhemerval Zanella
  0 siblings, 0 replies; 17+ messages in thread
From: Adhemerval Zanella @ 2020-12-04 12:33 UTC (permalink / raw)
  To: Maciej W. Rozycki
  Cc: Huang Pei, Huacai Chen, Chenghua Xu, libc-alpha, Joseph Myers



On 04/12/2020 08:03, Maciej W. Rozycki wrote:
> On Tue, 1 Dec 2020, Adhemerval Zanella via Libc-alpha wrote:
> 
>>> I would like to combine the Patch 2/3 and Patch 3/3 into one patch,
>>> since Patch 2/3 only fix sp alignment, but the key point is folllowing syscall
>>> restart convention, any advice?
>>
>> Send a v2 with the patches merged and the hp-inline fix.
> 
>  I would defer the removal of the old syscall restart convention in this 
> single place only.  I don't think we need to introduce such a mess.
> 
>  Instead an audit should be made and all the places adjusted with a single 
> change.  That would include at least MIPS16 wrappers, and IIRC some 
> microMIPS support code, and then the comment I referred previously needs 
> to be adjusted accordingly (maybe removed altogether even).
> 
>   Maciej

Alright, it seems better indeed.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 2/3] mips: make sp 16-byte aligned on N64/N32
  2020-11-27  9:22 [Patch V2]: add hp-timing support and fix syscall.S Huang Pei
@ 2020-11-27  9:22 ` Huang Pei
  0 siblings, 0 replies; 17+ messages in thread
From: Huang Pei @ 2020-11-27  9:22 UTC (permalink / raw)
  To: Joseph Myers; +Cc: libc-alpha, Huacai Chen, Chenghua Xu

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(-)

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
-- 
2.17.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 2/3] mips: make sp 16-byte aligned on N64/N32
  2020-11-14  6:41 [Patch V2] mips: minior fix Huang Pei
@ 2020-11-14  6:41 ` Huang Pei
  0 siblings, 0 replies; 17+ messages in thread
From: Huang Pei @ 2020-11-14  6:41 UTC (permalink / raw)
  To: Joseph Myers; +Cc: libc-alpha, Huacai Chen, Chenghua Xu

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(-)

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
-- 
2.17.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2020-12-04 12:33 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-28  8:18 [Patch V3]: add hp-timing support and minor fix Huang Pei
2020-11-28  8:18 ` [PATCH 1/3] mips: add hp-timing support for MIPS R2 Huang Pei
2020-11-30 12:25   ` Adhemerval Zanella
2020-12-04 10:58     ` Maciej W. Rozycki
2020-12-04 12:31       ` Adhemerval Zanella
2020-11-28  8:18 ` [PATCH 2/3] mips: make sp 16-byte aligned on N64/N32 Huang Pei
2020-11-30 13:03   ` Adhemerval Zanella
2020-12-04 10:57     ` Maciej W. Rozycki
2020-12-04 12:32       ` Adhemerval Zanella
2020-11-28  8:18 ` [PATCH 3/3] mips: remove register spill Huang Pei
2020-11-30 14:22   ` Adhemerval Zanella
2020-12-01  9:39     ` Huang Pei
2020-12-01 11:58       ` Adhemerval Zanella
2020-12-04 11:03         ` Maciej W. Rozycki
2020-12-04 12:33           ` Adhemerval Zanella
  -- strict thread matches above, loose matches on Subject: below --
2020-11-27  9:22 [Patch V2]: add hp-timing support and fix syscall.S Huang Pei
2020-11-27  9:22 ` [PATCH 2/3] mips: make sp 16-byte aligned on N64/N32 Huang Pei
2020-11-14  6:41 [Patch V2] mips: minior fix Huang Pei
2020-11-14  6:41 ` [PATCH 2/3] mips: make sp 16-byte aligned on N64/N32 Huang Pei

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).