[v2] MIPS: Use `.set mips2' to emulate LL/SC for the R5900 too
Commit Message
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(-)
Comments
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
@@ -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"