mips: Do not include hi and lo in __SYSCALL_CLOBBERS for R6

Message ID 1574175364-5601-1-git-send-email-dmladjenovic@wavecomp.com
State Committed
Headers

Commit Message

Dragan Mladjenovic Nov. 19, 2019, 2:56 p.m. UTC
  From: "Dragan Mladjenovic" <dmladjenovic@wavecomp.com>

GCC 10 (PR 91233) won't silently allow registers that are not architecturally
available to be present in the clobber list anymore, resulting in build failure
for mips*r6 targets in form of:
...
.../sysdep.h:146:2: error: the register ‘lo’ cannot be clobbered in ‘asm’ for the current target
  146 |  __asm__ volatile (      \
      |  ^~~~~~~

This is because base R6 ISA doesn't define hi and lo registers w/o DSP extension.
This patch provides the alternative definitions of __SYSCALL_CLOBBERS for r6
targets that won't include those registers.

	* sysdeps/unix/sysv/linux/mips/mips32/sysdep.h (__SYSCALL_CLOBBERS): Exclude
	hi and lo from the clobber list for __mips_isa_rev >= 6.
	* sysdeps/unix/sysv/linux/mips/mips64/n32/sysdep.h (__SYSCALL_CLOBBERS): Likewise.
	* sysdeps/unix/sysv/linux/mips/mips64/n64/sysdep.h (__SYSCALL_CLOBBERS): Likewise.
---
 sysdeps/unix/sysv/linux/mips/mips32/sysdep.h     | 9 +++++++--
 sysdeps/unix/sysv/linux/mips/mips64/n32/sysdep.h | 9 +++++++--
 sysdeps/unix/sysv/linux/mips/mips64/n64/sysdep.h | 9 +++++++--
 3 files changed, 21 insertions(+), 6 deletions(-)

-- 
1.9.1
  

Comments

Joseph Myers Nov. 19, 2019, 4:57 p.m. UTC | #1
On Tue, 19 Nov 2019, Dragan Mladjenovic wrote:

> From: "Dragan Mladjenovic" <dmladjenovic@wavecomp.com>
> 
> GCC 10 (PR 91233) won't silently allow registers that are not architecturally
> available to be present in the clobber list anymore, resulting in build failure
> for mips*r6 targets in form of:
> ...
> .../sysdep.h:146:2: error: the register ‘lo’ cannot be clobbered in ‘asm’ for the current target
>   146 |  __asm__ volatile (      \
>       |  ^~~~~~~
> 
> This is because base R6 ISA doesn't define hi and lo registers w/o DSP 
> extension. This patch provides the alternative definitions of 
> __SYSCALL_CLOBBERS for r6 targets that won't include those registers.

What is the kernel ABI on r6 systems with the DSP extension - does the 
kernel ABI permit the kernel to clobber those registers on syscall return 
or not?  This patch looks like it's only safe if the kernel guarantees it 
will never clobber those registers on r6 (or later), regardless of 
instruction set extensions present.

Also, this issue suggests that build-many-glibcs.py ought to include MIPS 
r6 configurations to detect such build issues.  It might not be a good 
idea to duplicate all 24 ABIs for r6 (I don't know how many of them make 
sense for r6 anyway), but at least one each of o32, n32 and n64 for r6 (if 
all those make sense for r6) would be a good idea.  (build-many-glibcs.py 
additions should be a separate patch.)
  
Dragan Mladjenovic Nov. 19, 2019, 6:10 p.m. UTC | #2
On 19.11.2019. 17:57, Joseph Myers wrote:
> On Tue, 19 Nov 2019, Dragan Mladjenovic wrote:

>

>> From: "Dragan Mladjenovic" <dmladjenovic@wavecomp.com>

>>

>> GCC 10 (PR 91233) won't silently allow registers that are not architecturally

>> available to be present in the clobber list anymore, resulting in build failure

>> for mips*r6 targets in form of:

>> ...

>> .../sysdep.h:146:2: error: the register ‘lo’ cannot be clobbered in ‘asm’ for the current target

>>    146 |  __asm__ volatile (      \

>>        |  ^~~~~~~

>>

>> This is because base R6 ISA doesn't define hi and lo registers w/o DSP

>> extension. This patch provides the alternative definitions of

>> __SYSCALL_CLOBBERS for r6 targets that won't include those registers.

>

> What is the kernel ABI on r6 systems with the DSP extension - does the

> kernel ABI permit the kernel to clobber those registers on syscall return

> or not?  This patch looks like it's only safe if the kernel guarantees it

> will never clobber those registers on r6 (or later), regardless of

> instruction set extensions present.


The kernel is not allowed to use DSP ASE. From what I see the DSP state 
is not restored on syscall exit. Only some vendor specific extension are 
allowed in the kernel. From what I understand that on "happy path" 
kernel just saves some registers and relies on C ABI to preserve the 
rest. The use of hi and lo is result of them not being preserved across 
the function calls in C.

> Also, this issue suggests that build-many-glibcs.py ought to include MIPS

> r6 configurations to detect such build issues.  It might not be a good

> idea to duplicate all 24 ABIs for r6 (I don't know how many of them make

> sense for r6 anyway), but at least one each of o32, n32 and n64 for r6 (if

> all those make sense for r6) would be a good idea.  (build-many-glibcs.py

> additions should be a separate patch.)

>


That sounds fine. The r6 is nan2008/(by far hard-float) and most if not 
all distros are targeting little-endian, so we can start with 
o32/n32/n64 mips64el variant. We could add the big endian variants for 
the sake of the completes, but I would not suggest bothering with 
soft-float.

Will submit a separate patch for that.
  
Joseph Myers Nov. 19, 2019, 6:20 p.m. UTC | #3
On Tue, 19 Nov 2019, Dragan Mladjenovic wrote:

> > What is the kernel ABI on r6 systems with the DSP extension - does the
> > kernel ABI permit the kernel to clobber those registers on syscall return
> > or not?  This patch looks like it's only safe if the kernel guarantees it
> > will never clobber those registers on r6 (or later), regardless of
> > instruction set extensions present.
> 
> The kernel is not allowed to use DSP ASE. From what I see the DSP state 
> is not restored on syscall exit. Only some vendor specific extension are 
> allowed in the kernel. From what I understand that on "happy path" 
> kernel just saves some registers and relies on C ABI to preserve the 
> rest. The use of hi and lo is result of them not being preserved across 
> the function calls in C.

Thanks.  If this means the kernel always preserves hi and lo for r6, 
regardless of extensions available, then the patch is OK to commit.  Note 
that we no longer include GNU-style ChangeLog entries in commit messages.
  
Dragan Mladjenovic Dec. 16, 2019, 6:54 p.m. UTC | #4
On 19.11.2019. 19:20, Joseph Myers wrote:
> On Tue, 19 Nov 2019, Dragan Mladjenovic wrote:
>
>>> What is the kernel ABI on r6 systems with the DSP extension - does the
>>> kernel ABI permit the kernel to clobber those registers on syscall return
>>> or not?  This patch looks like it's only safe if the kernel guarantees it
>>> will never clobber those registers on r6 (or later), regardless of
>>> instruction set extensions present.
>>
>> The kernel is not allowed to use DSP ASE. From what I see the DSP state
>> is not restored on syscall exit. Only some vendor specific extension are
>> allowed in the kernel. From what I understand that on "happy path"
>> kernel just saves some registers and relies on C ABI to preserve the
>> rest. The use of hi and lo is result of them not being preserved across
>> the function calls in C.
>
> Thanks.  If this means the kernel always preserves hi and lo for r6,
> regardless of extensions available, then the patch is OK to commit.  Note
> that we no longer include GNU-style ChangeLog entries in commit messages.
>

Thanks,

Committed as [1]. I left the ChangeLog anyway.

[1] 
https://sourceware.org/git/?p=glibc.git;a=commit;h=a2e487ce1c59d19345d9ecacc58de79febd869e4
  

Patch

diff --git a/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h b/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h
index 86347fe..cbc2a4b 100644
--- a/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h
+++ b/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h
@@ -346,8 +346,13 @@  libc_hidden_proto (__mips_syscall7, nomips16)
 	_sc_ret.reg.v0;							\
 })
 
-#define __SYSCALL_CLOBBERS "$1", "$3", "$8", "$9", "$10", "$11", "$12", "$13", \
-	"$14", "$15", "$24", "$25", "hi", "lo", "memory"
+#if __mips_isa_rev >= 6
+# define __SYSCALL_CLOBBERS "$1", "$3", "$8", "$9", "$10", "$11", "$12", "$13", \
+	 "$14", "$15", "$24", "$25", "memory"
+#else
+# define __SYSCALL_CLOBBERS "$1", "$3", "$8", "$9", "$10", "$11", "$12", "$13", \
+	 "$14", "$15", "$24", "$25", "hi", "lo", "memory"
+#endif
 
 #endif /* __ASSEMBLER__ */
 
diff --git a/sysdeps/unix/sysv/linux/mips/mips64/n32/sysdep.h b/sysdeps/unix/sysv/linux/mips/mips64/n32/sysdep.h
index ca7a60e..fd16508 100644
--- a/sysdeps/unix/sysv/linux/mips/mips64/n32/sysdep.h
+++ b/sysdeps/unix/sysv/linux/mips/mips64/n32/sysdep.h
@@ -294,8 +294,13 @@ 
 	_sys_result;							\
 })
 
-#define __SYSCALL_CLOBBERS "$1", "$3", "$10", "$11", "$12", "$13", \
-	"$14", "$15", "$24", "$25", "hi", "lo", "memory"
+#if __mips_isa_rev >= 6
+# define __SYSCALL_CLOBBERS "$1", "$3", "$10", "$11", "$12", "$13", \
+	 "$14", "$15", "$24", "$25", "memory"
+#else
+# define __SYSCALL_CLOBBERS "$1", "$3", "$10", "$11", "$12", "$13", \
+	 "$14", "$15", "$24", "$25", "hi", "lo", "memory"
+#endif
 
 #endif /* __ASSEMBLER__ */
 
diff --git a/sysdeps/unix/sysv/linux/mips/mips64/n64/sysdep.h b/sysdeps/unix/sysv/linux/mips/mips64/n64/sysdep.h
index 821dec9..8df4d9b 100644
--- a/sysdeps/unix/sysv/linux/mips/mips64/n64/sysdep.h
+++ b/sysdeps/unix/sysv/linux/mips/mips64/n64/sysdep.h
@@ -290,8 +290,13 @@ 
 	_sys_result;							\
 })
 
-#define __SYSCALL_CLOBBERS "$1", "$3", "$10", "$11", "$12", "$13", \
-	"$14", "$15", "$24", "$25", "hi", "lo", "memory"
+#if __mips_isa_rev >= 6
+# define __SYSCALL_CLOBBERS "$1", "$3", "$10", "$11", "$12", "$13", \
+	 "$14", "$15", "$24", "$25", "memory"
+#else
+# define __SYSCALL_CLOBBERS "$1", "$3", "$10", "$11", "$12", "$13", \
+	 "$14", "$15", "$24", "$25", "hi", "lo", "memory"
+#endif
 
 #endif /* __ASSEMBLER__ */