* [Patch V2] mips: minior fix
@ 2020-11-14 6:41 Huang Pei
2020-11-14 6:41 ` [PATCH 1/3] mips: add hp-timing support for MIPS R2 Huang Pei
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Huang Pei @ 2020-11-14 6:41 UTC (permalink / raw)
To: Joseph Myers; +Cc: libc-alpha, Huacai Chen, Chenghua Xu
All three patched is tested on debian glibc-2.28, no regresssion
found.
here is the output of 'LD_DEBUG=statistics data' with Patch 1:
.....................................................................
depaulose@Board-3A3000:glibc-2.28$ LD_DEBUG=statistics date
441:
441: runtime linker statistics:
441: total startup time in dynamic loader: 297131 cycles
441: time needed for relocation: 150697 cycles (50.7%)
441: number of relocations: 101
441: number of relocations from cache: 0
441: number of relative relocations: 0
441: time needed to load objects: 84028 cycles (28.2%)
Sat 14 Nov 2020 06:33:18 AM UTC
.....................................................................
Patch 2 make the sp follow N32/N64 ABI, however the root cause is the
reg s0 spill, which is unnecessary
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/3] mips: add hp-timing support for MIPS R2
2020-11-14 6:41 [Patch V2] mips: minior fix Huang Pei
@ 2020-11-14 6:41 ` Huang Pei
2020-11-14 6:41 ` [PATCH 2/3] mips: make sp 16-byte aligned on N64/N32 Huang Pei
2020-11-14 6:42 ` [PATCH 3/3] mips: remove useless register spill Huang Pei
2 siblings, 0 replies; 8+ 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 R2 only support 32 bit TSC(AKA "rdhwr %0, $2"), but it should be
enough for glibc
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] 8+ 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 ` [PATCH 1/3] mips: add hp-timing support for MIPS R2 Huang Pei
@ 2020-11-14 6:41 ` Huang Pei
2020-11-14 6:42 ` [PATCH 3/3] mips: remove useless register spill Huang Pei
2 siblings, 0 replies; 8+ 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] 8+ messages in thread
* [PATCH 3/3] mips: remove useless register spill
2020-11-14 6:41 [Patch V2] mips: minior fix Huang Pei
2020-11-14 6:41 ` [PATCH 1/3] mips: add hp-timing support for MIPS R2 Huang Pei
2020-11-14 6:41 ` [PATCH 2/3] mips: make sp 16-byte aligned on N64/N32 Huang Pei
@ 2020-11-14 6:42 ` Huang Pei
2020-11-15 1:22 ` Maciej W. Rozycki
2 siblings, 1 reply; 8+ messages in thread
From: Huang Pei @ 2020-11-14 6:42 UTC (permalink / raw)
To: Joseph Myers; +Cc: libc-alpha, Huacai Chen, Chenghua Xu
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] 8+ messages in thread
* Re: [PATCH 3/3] mips: remove useless register spill
2020-11-14 6:42 ` [PATCH 3/3] mips: remove useless register spill Huang Pei
@ 2020-11-15 1:22 ` Maciej W. Rozycki
[not found] ` <2020111522504369692527@loongson.cn>
0 siblings, 1 reply; 8+ messages in thread
From: Maciej W. Rozycki @ 2020-11-15 1:22 UTC (permalink / raw)
To: Huang Pei; +Cc: Joseph Myers, Huacai Chen, Chenghua Xu, libc-alpha
On Sat, 14 Nov 2020, Huang Pei wrote:
> Signed-off-by: Huang Pei <huangpei@loongson.cn>
Please be a bit more verbose in the change description. This used to be
an implementation of the old syscall restart convention, where $v0 had to
be (re)loaded by the instruction immediately preceding SYSCALL, obviously
from a source that does not get clobbered in syscall processing (i.e.
either memory or a static register). I imagine a good change description
would say at least actually why this convention is no longer relevant.
Refer to the comment in sysdeps/unix/sysv/linux/mips/mips32/sysdep.h or
sysdeps/unix/sysv/linux/mips/mips64/sysdep.h for further details.
Also I am not sure if it makes sense to remove this single instance only,
possibly for the least used syscall in user code, and leave all the other
places unchanged.
Maciej
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Re: [PATCH 3/3] mips: remove useless register spill
[not found] ` <2020111522504369692527@loongson.cn>
@ 2020-11-15 14:56 ` huangpei
2020-12-04 11:24 ` Maciej W. Rozycki
0 siblings, 1 reply; 8+ messages in thread
From: huangpei @ 2020-11-15 14:56 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: Myers, Chen, Xu, libc-alpha
Hi, Maciej,
I must admit that I did not realize that Patch 3 would cause compatibiliy
issue, that is Linux/MIPS kernel before 2.6.36 depended on
"move a0, s0 ' just preceding "syscall" to make SYSCALL Restart
work, while Linux/MIPS after 2.6.36 do not
since glibc 2.24,the minimum kernel version is 3.2, is it ok to add Patch
3 for glibc after version 2.24? If so, I would send V3, which merge Patch
2 and Patch 3 and rewrite the change description with minimium kernel
version requirement.
Huang Pei
>Hi, Maciej,
>
>I must admit that I did not realize that Patch 3 would cause compatibiliy
>issue, that is Linux/MIPS kernel before 2.6.36 depended on
>"move a0, s0 ' just preceding "syscall" to make SYSCALL Restart
>work, while Linux/MIPS after 2.6.36 do not
>
>since glibc 2.24,the minimum kernel version is 3.2, is it ok to add Patch
>3 for glibc after version 2.24? If so, I would send V3, which merge Patch
>2 and Patch 3 and rewrite the change description with minimium kernel
>version requirement.
>
>Huang Pei
>
>
>--------------
>huangpei@loongson.cn
>>On Sat, 14 Nov 2020, Huang Pei wrote:
>>
>>> Signed-off-by: Huang Pei <huangpei@loongson.cn>
>>
>> Please be a bit more verbose in the change description. This used to be
>>an implementation of the old syscall restart convention, where $v0 had to
>>be (re)loaded by the instruction immediately preceding SYSCALL, obviously
>>from a source that does not get clobbered in syscall processing (i.e.
>>either memory or a static register). I imagine a good change description
>>would say at least actually why this convention is no longer relevant.
>>
>> Refer to the comment in sysdeps/unix/sysv/linux/mips/mips32/sysdep.h or
>>sysdeps/unix/sysv/linux/mips/mips64/sysdep.h for further details.
>>
>> Also I am not sure if it makes sense to remove this single instance only,
>>possibly for the least used syscall in user code, and leave all the other
>>places unchanged.
>>
>> Maciej
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Re: [PATCH 3/3] mips: remove useless register spill
2020-11-15 14:56 ` huangpei
@ 2020-12-04 11:24 ` Maciej W. Rozycki
2020-12-05 5:47 ` Huang Pei
0 siblings, 1 reply; 8+ messages in thread
From: Maciej W. Rozycki @ 2020-12-04 11:24 UTC (permalink / raw)
To: huangpei; +Cc: Myers, Chen, Xu, libc-alpha
Hi Huang Pei,
Apologies for my late reply, I've had a hectic time recently and some GCC
stuff had to take priority due to the end of Stage 1 there.
> I must admit that I did not realize that Patch 3 would cause compatibiliy
> issue, that is Linux/MIPS kernel before 2.6.36 depended on
> "move a0, s0 ' just preceding "syscall" to make SYSCALL Restart
> work, while Linux/MIPS after 2.6.36 do not
Or `move v0, s0' (in this case) to be correct.
Yes, this has been quite obscure and used to be hardly documented up to
the point that breakage happened while the old scheme was still supported.
And given that syscall restarts are not at all that frequent any symptom
would be limited to an occasional program crash or other odd behaviour as
the wrong syscall was dispatched upon a restart.
This is why I added the explanation along with fixes for BZ #15054 made
with commit b82ba2f011fc ("MIPS: Respect the legacy syscall restart
convention."). Before that we only had Linux sources as a reference.
NB the old scheme was removed from Linux with commit 8f5a00eb422e ("MIPS:
Sanitize restart logics").
> since glibc 2.24,the minimum kernel version is 3.2, is it ok to add Patch
> 3 for glibc after version 2.24? If so, I would send V3, which merge Patch
> 2 and Patch 3 and rewrite the change description with minimium kernel
> version requirement.
As I noted elsewhere I object such a change if made on its own, as it
would only cause a mess.
You can review my commit referred above, and also commit 43301bd3c281
("Add support for building as MIPS16 code.") to see what has to be done to
remove bits for the old feature in a consistent manner.
Maciej
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] mips: remove useless register spill
2020-12-04 11:24 ` Maciej W. Rozycki
@ 2020-12-05 5:47 ` Huang Pei
0 siblings, 0 replies; 8+ messages in thread
From: Huang Pei @ 2020-12-05 5:47 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: Myers, Chen, Xu, libc-alpha
On Fri, Dec 04, 2020 at 11:24:24AM +0000, Maciej W. Rozycki wrote:
> Hi Huang Pei,
>
> Apologies for my late reply, I've had a hectic time recently and some GCC
> stuff had to take priority due to the end of Stage 1 there.
>
> > I must admit that I did not realize that Patch 3 would cause compatibiliy
> > issue, that is Linux/MIPS kernel before 2.6.36 depended on
> > "move a0, s0 ' just preceding "syscall" to make SYSCALL Restart
> > work, while Linux/MIPS after 2.6.36 do not
>
> Or `move v0, s0' (in this case) to be correct.
Yes
>
> Yes, this has been quite obscure and used to be hardly documented up to
> the point that breakage happened while the old scheme was still supported.
> And given that syscall restarts are not at all that frequent any symptom
> would be limited to an occasional program crash or other odd behaviour as
> the wrong syscall was dispatched upon a restart.
>
> This is why I added the explanation along with fixes for BZ #15054 made
> with commit b82ba2f011fc ("MIPS: Respect the legacy syscall restart
> convention."). Before that we only had Linux sources as a reference.
>
> NB the old scheme was removed from Linux with commit 8f5a00eb422e ("MIPS:
> Sanitize restart logics").
>
> > since glibc 2.24,the minimum kernel version is 3.2, is it ok to add Patch
> > 3 for glibc after version 2.24? If so, I would send V3, which merge Patch
> > 2 and Patch 3 and rewrite the change description with minimium kernel
> > version requirement.
>
> As I noted elsewhere I object such a change if made on its own, as it
> would only cause a mess.
This one was still a mess, since the ".set noreorder" is within
b82ba2f011fc, but missing here(I hate '.set noreorder', which bite me
too much)
I came to this one accidentally when I addressed a unaligned access issue,
but the fix is not the key point(see Patch V1 2/3), and Patch V1 3/3
without touching ".set noreorder"(I focus on) is.
PS:
I think it would be mutch better without both ".set reorder" and ".set
noreorder", and with 8f5a00eb422e, we can remove more ".set noreorder"
>
> You can review my commit referred above, and also commit 43301bd3c281
> ("Add support for building as MIPS16 code.") to see what has to be done to
> remove bits for the old feature in a consistent manner.
>
> Maciej
Now it is OK to remove the mess as a whole, I will send V3 to get rid of it
all
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-12-05 5:47 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-14 6:41 [Patch V2] mips: minior fix Huang Pei
2020-11-14 6:41 ` [PATCH 1/3] mips: add hp-timing support for MIPS R2 Huang Pei
2020-11-14 6:41 ` [PATCH 2/3] mips: make sp 16-byte aligned on N64/N32 Huang Pei
2020-11-14 6:42 ` [PATCH 3/3] mips: remove useless register spill Huang Pei
2020-11-15 1:22 ` Maciej W. Rozycki
[not found] ` <2020111522504369692527@loongson.cn>
2020-11-15 14:56 ` huangpei
2020-12-04 11:24 ` Maciej W. Rozycki
2020-12-05 5:47 ` 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).