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