| Message ID | 3fb8c58bc662b93e68a992adfb099ad233b40ea2.1540925714.git.noring@nocrew.org |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] MIPS: Use `.set mips2' to emulate LL/SC for the R5900 too | expand |
Hi Fredrik, > It turns out that `.set arch=mips' isn't needed, so the patch can be > simplified. Please find the updated v2 patch here. FAOD, it was me who suggested that `.set arch=mips2' was required, while indeed existing `.set mips2' is just as good and does not have to be changed. For some reason I misremembered that with GAS `.set mipsX' only overrides the ISA level used for assembly and leaves the original CPU architecture intact, while indeed both `.set mipsX' and `.set arch=mipsX' override both settings and are equivalent to each other. Sorry to cause this confusion. This version is: Reviewed-by: Maciej W. Rozycki <macro@linux-mips.org> Maciej
On Tue, 30 Oct 2018, Maciej W. Rozycki wrote: > Hi Fredrik, > > > It turns out that `.set arch=mips' isn't needed, so the patch can be > > simplified. Please find the updated v2 patch here. > > FAOD, it was me who suggested that `.set arch=mips2' was required, while > indeed existing `.set mips2' is just as good and does not have to be > changed. > > For some reason I misremembered that with GAS `.set mipsX' only overrides > the ISA level used for assembly and leaves the original CPU architecture > intact, while indeed both `.set mipsX' and `.set arch=mipsX' override both > settings and are equivalent to each other. Sorry to cause this confusion. > > This version is: > > Reviewed-by: Maciej W. Rozycki <macro@linux-mips.org> You should commit this, unless Fredrik has write access to commit it himself.
On Tue, 30 Oct 2018, Joseph Myers wrote: > > This version is: > > > > Reviewed-by: Maciej W. Rozycki <macro@linux-mips.org> > > You should commit this, unless Fredrik has write access to commit it > himself. We're consensus-driven and I don't want to give an impression of rushing things in. I hate it when some people do this. I think a settling period of 24-48 hours is appropriate in cases like this, in case someone wants to chime in. This was missing a ChangeLog entry, which I have now added, and committed this change. Thank you, Fredrik, for your contribution! Maciej
diff --git a/sysdeps/mips/sys/tas.h b/sysdeps/mips/sys/tas.h index d5ed013e28..22cee94f7a 100644 --- a/sysdeps/mips/sys/tas.h +++ b/sysdeps/mips/sys/tas.h @@ -38,10 +38,11 @@ __NTH (_test_and_set (int *__p, int __v)) { int __r, __t; + /* The R5900 reports itself as MIPS III but it does not have LL/SC. */ __asm__ __volatile__ ("/* Inline test and set */\n" ".set push\n\t" -#if _MIPS_SIM == _ABIO32 && __mips < 2 +#if _MIPS_SIM == _ABIO32 && (__mips < 2 || defined (_MIPS_ARCH_R5900)) ".set mips2\n\t" #endif "sync\n\t"
GAS treats the R5900 as MIPS III, with some modifications. The MIPS III designation means that the GNU C Library will try to assemble the LL and SC instructions, even though they are not implemented in the R5900. GAS will therefore produce the following errors: Error: opcode not supported on this processor: r5900 (mips3) `ll $2,0($4)' Error: opcode not supported on this processor: r5900 (mips3) `sc $6,0($4)' The MIPS II ISA override as used here enables the kernel to trap and emulate the LL and SC instructions, as required. This change has been tested by compiling the GNU C Library 2.27 with a GCC 8.2.0 cross-compiler for mipsr5900el-unknown-linux-gnu under Gentoo. --- Thank you very much for your reviews, Maciej and Joseph, It turns out that `.set arch=mips' isn't needed, so the patch can be simplified. Please find the updated v2 patch here. Changes in v2: - Discard `set arch=mips2' since this isn't needed - Add space before `(' as per GNU style - Amend note explaining R5900 exception --- sysdeps/mips/sys/tas.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)