diff mbox series

linux: mips: Fix syscall_cancell build for __mips_isa_rev >= 6

Message ID 20240829195341.1997338-1-adhemerval.zanella@linaro.org
State Accepted
Commit 1927f718fcc48bdaea03086bdc2adf11279d655b
Headers show
Series linux: mips: Fix syscall_cancell build for __mips_isa_rev >= 6 | expand

Commit Message

Adhemerval Zanella Aug. 29, 2024, 7:53 p.m. UTC
Use beqzc instead of bnel.

Checked with a mipsisa64r6el-n64-linux-gnu build and some nptl
cancellation tests on qemu.
---
 sysdeps/unix/sysv/linux/mips/mips64/syscall_cancel.S | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Adhemerval Zanella Sept. 2, 2024, 1:46 p.m. UTC | #1
On 29/08/24 16:53, Adhemerval Zanella wrote:
> Use beqzc instead of bnel.
> 
> Checked with a mipsisa64r6el-n64-linux-gnu build and some nptl
> cancellation tests on qemu.
The syscall_cancell is spelled wrongly, I will fit it.

I will also commit this shortly if no one opposes.

> ---
>  sysdeps/unix/sysv/linux/mips/mips64/syscall_cancel.S | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/sysdeps/unix/sysv/linux/mips/mips64/syscall_cancel.S b/sysdeps/unix/sysv/linux/mips/mips64/syscall_cancel.S
> index f172041324..cfc0596b6a 100644
> --- a/sysdeps/unix/sysv/linux/mips/mips64/syscall_cancel.S
> +++ b/sysdeps/unix/sysv/linux/mips/mips64/syscall_cancel.S
> @@ -77,7 +77,11 @@ __syscall_cancel_arch_end:
>  
>  	.set noreorder
>  	.set nomacro
> +#if __mips_isa_rev >= 6
> +	beqzc	$7, 1f
> +#else
>  	bnel	a3, zero, 1f
> +#endif
>  	SUBU	v0, zero, v0
>  	.set macro
>  	.set reorder
Maciej W. Rozycki Sept. 6, 2024, 1:46 a.m. UTC | #2
On Mon, 2 Sep 2024, Adhemerval Zanella Netto wrote:

> > Use beqzc instead of bnel.
> > 
> > Checked with a mipsisa64r6el-n64-linux-gnu build and some nptl
> > cancellation tests on qemu.
> The syscall_cancell is spelled wrongly, I will fit it.

 It seems like you didn't after all. :(

> > diff --git a/sysdeps/unix/sysv/linux/mips/mips64/syscall_cancel.S b/sysdeps/unix/sysv/linux/mips/mips64/syscall_cancel.S
> > index f172041324..cfc0596b6a 100644
> > --- a/sysdeps/unix/sysv/linux/mips/mips64/syscall_cancel.S
> > +++ b/sysdeps/unix/sysv/linux/mips/mips64/syscall_cancel.S
> > @@ -77,7 +77,11 @@ __syscall_cancel_arch_end:
> >  
> >  	.set noreorder
> >  	.set nomacro
> > +#if __mips_isa_rev >= 6
> > +	beqzc	$7, 1f
> > +#else
> >  	bnel	a3, zero, 1f
> > +#endif
> >  	SUBU	v0, zero, v0
> >  	.set macro
> >  	.set reorder

 Why $7 inconsistently rather than a3 as in the existing code?  All the 
remaining code uses ABI rather than raw register names.

 NB `bnezl' could be used for consistency/readability, and likewise `negu' 
rather than `SUBU' (why is it capitalised anyway?).

  Maciej
Adhemerval Zanella Sept. 6, 2024, 1:03 p.m. UTC | #3
On 05/09/24 22:46, Maciej W. Rozycki wrote:
> On Mon, 2 Sep 2024, Adhemerval Zanella Netto wrote:
> 
>>> Use beqzc instead of bnel.
>>>
>>> Checked with a mipsisa64r6el-n64-linux-gnu build and some nptl
>>> cancellation tests on qemu.
>> The syscall_cancell is spelled wrongly, I will fit it.
> 
>  It seems like you didn't after all. :(

Yeah, my mistake here.

> 
>>> diff --git a/sysdeps/unix/sysv/linux/mips/mips64/syscall_cancel.S b/sysdeps/unix/sysv/linux/mips/mips64/syscall_cancel.S
>>> index f172041324..cfc0596b6a 100644
>>> --- a/sysdeps/unix/sysv/linux/mips/mips64/syscall_cancel.S
>>> +++ b/sysdeps/unix/sysv/linux/mips/mips64/syscall_cancel.S
>>> @@ -77,7 +77,11 @@ __syscall_cancel_arch_end:
>>>  
>>>  	.set noreorder
>>>  	.set nomacro
>>> +#if __mips_isa_rev >= 6
>>> +	beqzc	$7, 1f
>>> +#else
>>>  	bnel	a3, zero, 1f
>>> +#endif
>>>  	SUBU	v0, zero, v0
>>>  	.set macro
>>>  	.set reorder
> 
>  Why $7 inconsistently rather than a3 as in the existing code?  All the 
> remaining code uses ABI rather than raw register names.

Because it come mainly from inspecting the code generation from compilers,
and then testing on qemu.

> 
>  NB `bnezl' could be used for consistency/readability, and likewise `negu' 
> rather than `SUBU' (why is it capitalised anyway?).

I don't have a strong preference, I am far from a mips expert, and I could
use some help on improve things here.
Maciej W. Rozycki Sept. 9, 2024, 1:37 p.m. UTC | #4
On Fri, 6 Sep 2024, Adhemerval Zanella Netto wrote:

> >>> diff --git a/sysdeps/unix/sysv/linux/mips/mips64/syscall_cancel.S b/sysdeps/unix/sysv/linux/mips/mips64/syscall_cancel.S
> >>> index f172041324..cfc0596b6a 100644
> >>> --- a/sysdeps/unix/sysv/linux/mips/mips64/syscall_cancel.S
> >>> +++ b/sysdeps/unix/sysv/linux/mips/mips64/syscall_cancel.S
> >>> @@ -77,7 +77,11 @@ __syscall_cancel_arch_end:
> >>>  
> >>>  	.set noreorder
> >>>  	.set nomacro
> >>> +#if __mips_isa_rev >= 6
> >>> +	beqzc	$7, 1f
> >>> +#else
> >>>  	bnel	a3, zero, 1f
> >>> +#endif
> >>>  	SUBU	v0, zero, v0
> >>>  	.set macro
> >>>  	.set reorder
> > 
> >  Why $7 inconsistently rather than a3 as in the existing code?  All the 
> > remaining code uses ABI rather than raw register names.
> 
> Because it come mainly from inspecting the code generation from compilers,
> and then testing on qemu.

 Ack.  Sadly I wasn't available earlier to review this change and it went 
in so quickly.

> >  NB `bnezl' could be used for consistency/readability, and likewise `negu' 
> > rather than `SUBU' (why is it capitalised anyway?).
> 
> I don't have a strong preference, I am far from a mips expert, and I could
> use some help on improve things here.

 Fair enough.  I'll see what I can do.

  Maciej
diff mbox series

Patch

diff --git a/sysdeps/unix/sysv/linux/mips/mips64/syscall_cancel.S b/sysdeps/unix/sysv/linux/mips/mips64/syscall_cancel.S
index f172041324..cfc0596b6a 100644
--- a/sysdeps/unix/sysv/linux/mips/mips64/syscall_cancel.S
+++ b/sysdeps/unix/sysv/linux/mips/mips64/syscall_cancel.S
@@ -77,7 +77,11 @@  __syscall_cancel_arch_end:
 
 	.set noreorder
 	.set nomacro
+#if __mips_isa_rev >= 6
+	beqzc	$7, 1f
+#else
 	bnel	a3, zero, 1f
+#endif
 	SUBU	v0, zero, v0
 	.set macro
 	.set reorder