diff mbox series

[1/3] mips: add hp-timing support for MIPS R2

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

Commit Message

Huang Pei Nov. 28, 2020, 8:18 a.m. UTC
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

Comments

Adhemerval Zanella Nov. 30, 2020, 12:25 p.m. UTC | #1
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 */
>
Maciej W. Rozycki Dec. 4, 2020, 10:58 a.m. UTC | #2
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
Adhemerval Zanella Dec. 4, 2020, 12:31 p.m. UTC | #3
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.
diff mbox series

Patch

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 */