[PATCH] Fix atomic_arch.h for MIPS32 R6

[PATCH] Fix atomic_arch.h for MIPS32 R6

[Jaydeep Patil]

[-- Attachment #1: Type: text/plain, Size: 1447 bytes --]

Hi Rich,

The arch/mips/atomic_arch.h uses MIPS2 opcode for LL and SC instructions. Opcodes of these instructions differ on MIPSR6.

Refer to https://github.com/JaydeepIMG/musl-1/tree/fix_atomic_for_MIPS32_R6 for the patch.

From 63428cfc5dfa75d2771ba8205067c438942e1a60 Mon Sep 17 00:00:00 2001
From: Jaydeep Patil <jaydeep.patil@imgtec.com>
Date: Mon, 21 Mar 2016 06:00:39 +0000
Subject: [PATCH] Fix for opcodes of LL/SC instructions for MIPSR6

---
arch/mips/atomic_arch.h | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/arch/mips/atomic_arch.h b/arch/mips/atomic_arch.h
index ce2823b..16b1542 100644
--- a/arch/mips/atomic_arch.h
+++ b/arch/mips/atomic_arch.h
@@ -3,9 +3,13 @@ static inline int a_ll(volatile int *p)
{
        int v;
        __asm__ __volatile__ (
+#if __mips_isa_rev < 6
                ".set push ; .set mips2\n\t"
+#endif
                "ll %0, %1"
+#if __mips_isa_rev < 6
                "\n\t.set pop"
+#endif
                : "=r"(v) : "m"(*p));
        return v;
}
@@ -15,9 +19,13 @@ static inline int a_sc(volatile int *p, int v)
{
        int r;
        __asm__ __volatile__ (
+#if __mips_isa_rev < 6
                ".set push ; .set mips2\n\t"
+#endif
                "sc %0, %1"
+#if __mips_isa_rev < 6
                "\n\t.set pop"
+#endif
                : "=r"(r), "=m"(*p) : "0"(v) : "memory");
        return r;
}
--
2.1.4

Regards,
Jaydeep


[-- Attachment #2: Type: text/html, Size: 6123 bytes --]

Re: [PATCH] Fix atomic_arch.h for MIPS32 R6

[dalias]

On Mon, Mar 21, 2016 at 06:03:47AM +0000, Jaydeep Patil wrote:
> Hi Rich,
> 
> The arch/mips/atomic_arch.h uses MIPS2 opcode for LL and SC
> instructions. Opcodes of these instructions differ on MIPSR6.

Does this mean MIPSR6 is an incompatible ISA that can't run normal
MIPS binaries? If so that's a messy situation we need to find a way to
deal with; if the difference is just LLSC though then perhaps the
kernel's emulation handles it (albeit very slowly).

It would be helpful if you could provide a link to the documentation
of this issue (different opcodes).

> Refer to https://github.com/JaydeepIMG/musl-1/tree/fix_atomic_for_MIPS32_R6 for the patch.
> 
> >From 63428cfc5dfa75d2771ba8205067c438942e1a60 Mon Sep 17 00:00:00 2001
> From: Jaydeep Patil <jaydeep.patil@imgtec.com>
> Date: Mon, 21 Mar 2016 06:00:39 +0000
> Subject: [PATCH] Fix for opcodes of LL/SC instructions for MIPSR6
> 
> ---
> arch/mips/atomic_arch.h | 8 ++++++++
> 1 file changed, 8 insertions(+)
> 
> diff --git a/arch/mips/atomic_arch.h b/arch/mips/atomic_arch.h
> index ce2823b..16b1542 100644
> --- a/arch/mips/atomic_arch.h
> +++ b/arch/mips/atomic_arch.h
> @@ -3,9 +3,13 @@ static inline int a_ll(volatile int *p)
> {
>         int v;
>         __asm__ __volatile__ (
> +#if __mips_isa_rev < 6
>                 ".set push ; .set mips2\n\t"
> +#endif
>                 "ll %0, %1"
> +#if __mips_isa_rev < 6
>                 "\n\t.set pop"
> +#endif

I think just the .set mips2 could be inside #ifdef with the push/pop
always used; that produces less #ifdef clutter. But first we need to
figure out the above issues.

Rich

RE: [PATCH] Fix atomic_arch.h for MIPS32 R6

[Jaydeep Patil]

>-----Original Message-----
>From: Rich Felker [mailto:dalias@aerifal.cx] On Behalf Of dalias@libc.org
>Sent: 21 March 2016 PM 11:08
>To: musl@lists.openwall.com
>Subject: Re: [musl] [PATCH] Fix atomic_arch.h for MIPS32 R6
>
>On Mon, Mar 21, 2016 at 06:03:47AM +0000, Jaydeep Patil wrote:
>> Hi Rich,
>>
>> The arch/mips/atomic_arch.h uses MIPS2 opcode for LL and SC
>> instructions. Opcodes of these instructions differ on MIPSR6.
>
>Does this mean MIPSR6 is an incompatible ISA that can't run normal MIPS
>binaries? If so that's a messy situation we need to find a way to deal with; if
>the difference is just LLSC though then perhaps the kernel's emulation
>handles it (albeit very slowly).
>
>
>It would be helpful if you could provide a link to the documentation of this
>issue (different opcodes).

Refer to https://imagination-technologies-cloudfront-assets.s3.amazonaws.com/documentation/MD00086-2B-MIPS32BIS-AFP-06.04.pdf (Page 209) for details.

>> Refer to https://github.com/JaydeepIMG/musl-
>1/tree/fix_atomic_for_MIPS32_R6 for the patch.
>>
>> >From 63428cfc5dfa75d2771ba8205067c438942e1a60 Mon Sep 17 00:00:00
>> >2001
>> From: Jaydeep Patil <jaydeep.patil@imgtec.com>
>> Date: Mon, 21 Mar 2016 06:00:39 +0000
>> Subject: [PATCH] Fix for opcodes of LL/SC instructions for MIPSR6
>>
>> ---
>> arch/mips/atomic_arch.h | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/arch/mips/atomic_arch.h b/arch/mips/atomic_arch.h index
>> ce2823b..16b1542 100644
>> --- a/arch/mips/atomic_arch.h
>> +++ b/arch/mips/atomic_arch.h
>> @@ -3,9 +3,13 @@ static inline int a_ll(volatile int *p) {
>>         int v;
>>         __asm__ __volatile__ (
>> +#if __mips_isa_rev < 6
>>                 ".set push ; .set mips2\n\t"
>> +#endif
>>                 "ll %0, %1"
>> +#if __mips_isa_rev < 6
>>                 "\n\t.set pop"
>> +#endif
>
>I think just the .set mips2 could be inside #ifdef with the push/pop always
>used; that produces less #ifdef clutter. But first we need to figure out the
>above issues.
>

We don't need push/pop if we are not doing mips2

>Rich

Re: [PATCH] Fix atomic_arch.h for MIPS32 R6

[Rich Felker]

On Tue, Mar 22, 2016 at 04:58:51AM +0000, Jaydeep Patil wrote:
> >-----Original Message-----
> >From: Rich Felker [mailto:dalias@aerifal.cx] On Behalf Of dalias@libc.org
> >Sent: 21 March 2016 PM 11:08
> >To: musl@lists.openwall.com
> >Subject: Re: [musl] [PATCH] Fix atomic_arch.h for MIPS32 R6
> >
> >On Mon, Mar 21, 2016 at 06:03:47AM +0000, Jaydeep Patil wrote:
> >> Hi Rich,
> >>
> >> The arch/mips/atomic_arch.h uses MIPS2 opcode for LL and SC
> >> instructions. Opcodes of these instructions differ on MIPSR6.
> >
> >Does this mean MIPSR6 is an incompatible ISA that can't run normal MIPS
> >binaries? If so that's a messy situation we need to find a way to deal with; if
> >the difference is just LLSC though then perhaps the kernel's emulation
> >handles it (albeit very slowly).
> >
> >
> >It would be helpful if you could provide a link to the documentation of this
> >issue (different opcodes).
> 
> Refer to
> https://imagination-technologies-cloudfront-assets.s3.amazonaws.com/documentation/MD00086-2B-MIPS32BIS-AFP-06.04.pdf
> (Page 209) for details.

Page 454 contains the best info I could find, which seems to say that
MIPS R6 is essentially a MIPS-incompatible ISA (it can't reliably
execute pre-R6 code). Is this correct? If so that's really
unfortunate. Unfortunately there does not seem to be any info here
specific to LL/SC, and the page 209 you cited is really sparse.

If this is really the case then we probably need to consider whether
some kind of awful runtime-switching mechanism is needed for baseline
MIPS ISA levels, or whether users just have to consider R6 "incapable
of running pre-R6 binaries". But in the latter case we might want to
use a different dynamic linker name for R6.

> >> arch/mips/atomic_arch.h | 8 ++++++++
> >> 1 file changed, 8 insertions(+)
> >>
> >> diff --git a/arch/mips/atomic_arch.h b/arch/mips/atomic_arch.h index
> >> ce2823b..16b1542 100644
> >> --- a/arch/mips/atomic_arch.h
> >> +++ b/arch/mips/atomic_arch.h
> >> @@ -3,9 +3,13 @@ static inline int a_ll(volatile int *p) {
> >>         int v;
> >>         __asm__ __volatile__ (
> >> +#if __mips_isa_rev < 6
> >>                 ".set push ; .set mips2\n\t"
> >> +#endif
> >>                 "ll %0, %1"
> >> +#if __mips_isa_rev < 6
> >>                 "\n\t.set pop"
> >> +#endif
> >
> >I think just the .set mips2 could be inside #ifdef with the push/pop always
> >used; that produces less #ifdef clutter. But first we need to figure out the
> >above issues.
> 
> We don't need push/pop if we are not doing mips2

I was just saying it makes the code less cluttered to use them
spuriously even though we don't need to:

		".set push ; "
#if __mips_isa_rev < 6
		".set mips2 ; "
#endif
		"ll %0, %1 ; .set pop"

or similar.

It's also not clear to me whether the "m" constraint is valid anymore
for the R6 ll/sc instructions since they take a 9-bit offset now
instead of a 16-bit offset. The compiler could generate an address
expression whose offset part does not fit in 9 bits. In that case we
may need to #if the whole function (or at least the __asm__ statement)
separately rather than just skipping the .set mips2...

Rich

RE: [PATCH] Fix atomic_arch.h for MIPS32 R6

[Jaydeep Patil]

>-----Original Message-----
>From: Rich Felker [mailto:dalias@aerifal.cx] On Behalf Of Rich Felker
>Sent: 23 March 2016 AM 02:52
>To: musl@lists.openwall.com
>Subject: Re: [musl] [PATCH] Fix atomic_arch.h for MIPS32 R6
>
>On Tue, Mar 22, 2016 at 04:58:51AM +0000, Jaydeep Patil wrote:
>> >-----Original Message-----
>> >From: Rich Felker [mailto:dalias@aerifal.cx] On Behalf Of
>> >dalias@libc.org
>> >Sent: 21 March 2016 PM 11:08
>> >To: musl@lists.openwall.com
>> >Subject: Re: [musl] [PATCH] Fix atomic_arch.h for MIPS32 R6
>> >
>> >On Mon, Mar 21, 2016 at 06:03:47AM +0000, Jaydeep Patil wrote:
>> >> Hi Rich,
>> >>
>> >> The arch/mips/atomic_arch.h uses MIPS2 opcode for LL and SC
>> >> instructions. Opcodes of these instructions differ on MIPSR6.
>> >
>> >Does this mean MIPSR6 is an incompatible ISA that can't run normal
>> >MIPS binaries? If so that's a messy situation we need to find a way
>> >to deal with; if the difference is just LLSC though then perhaps the
>> >kernel's emulation handles it (albeit very slowly).
>> >
>> >
>> >It would be helpful if you could provide a link to the documentation
>> >of this issue (different opcodes).
>>
>> Refer to
>> https://imagination-technologies-cloudfront-assets.s3.amazonaws.com/do
>> cumentation/MD00086-2B-MIPS32BIS-AFP-06.04.pdf
>> (Page 209) for details.
>
>Page 454 contains the best info I could find, which seems to say that MIPS R6
>is essentially a MIPS-incompatible ISA (it can't reliably execute pre-R6 code). Is
>this correct? If so that's really unfortunate. Unfortunately there does not

R6 is not binary compatible with pre-R6.

>seem to be any info here specific to LL/SC, and the page 209 you cited is really
>sparse.

It is page 219 which talks about R6 and pre-R6 LL opcodes. 

>If this is really the case then we probably need to consider whether some kind
>of awful runtime-switching mechanism is needed for baseline MIPS ISA levels,
>or whether users just have to consider R6 "incapable of running pre-R6
>binaries". But in the latter case we might want to use a different dynamic
>linker name for R6.
>
>> >> arch/mips/atomic_arch.h | 8 ++++++++
>> >> 1 file changed, 8 insertions(+)
>> >>
>> >> diff --git a/arch/mips/atomic_arch.h b/arch/mips/atomic_arch.h
>> >> index
>> >> ce2823b..16b1542 100644
>> >> --- a/arch/mips/atomic_arch.h
>> >> +++ b/arch/mips/atomic_arch.h
>> >> @@ -3,9 +3,13 @@ static inline int a_ll(volatile int *p) {
>> >>         int v;
>> >>         __asm__ __volatile__ (
>> >> +#if __mips_isa_rev < 6
>> >>                 ".set push ; .set mips2\n\t"
>> >> +#endif
>> >>                 "ll %0, %1"
>> >> +#if __mips_isa_rev < 6
>> >>                 "\n\t.set pop"
>> >> +#endif
>> >
>> >I think just the .set mips2 could be inside #ifdef with the push/pop
>> >always used; that produces less #ifdef clutter. But first we need to
>> >figure out the above issues.
>>
>> We don't need push/pop if we are not doing mips2
>
>I was just saying it makes the code less cluttered to use them spuriously even
>though we don't need to:
>
>		".set push ; "
>#if __mips_isa_rev < 6
>		".set mips2 ; "
>#endif
>		"ll %0, %1 ; .set pop"
>
>or similar.
>
>It's also not clear to me whether the "m" constraint is valid anymore for the R6
>ll/sc instructions since they take a 9-bit offset now instead of a 16-bit offset.
>The compiler could generate an address expression whose offset part does
>not fit in 9 bits. In that case we may need to #if the whole function (or at least
>the __asm__ statement) separately rather than just skipping the .set mips2...
>

The "m" constrain is still valid here, as the offset will be 0 in this case.

>Rich

Re: [PATCH] Fix atomic_arch.h for MIPS32 R6

[Rich Felker]

On Wed, Mar 23, 2016 at 06:37:41AM +0000, Jaydeep Patil wrote:
> >-----Original Message-----
> >From: Rich Felker [mailto:dalias@aerifal.cx] On Behalf Of Rich Felker
> >Sent: 23 March 2016 AM 02:52
> >To: musl@lists.openwall.com
> >Subject: Re: [musl] [PATCH] Fix atomic_arch.h for MIPS32 R6
> >
> >On Tue, Mar 22, 2016 at 04:58:51AM +0000, Jaydeep Patil wrote:
> >> >-----Original Message-----
> >> >From: Rich Felker [mailto:dalias@aerifal.cx] On Behalf Of
> >> >dalias@libc.org
> >> >Sent: 21 March 2016 PM 11:08
> >> >To: musl@lists.openwall.com
> >> >Subject: Re: [musl] [PATCH] Fix atomic_arch.h for MIPS32 R6
> >> >
> >> >On Mon, Mar 21, 2016 at 06:03:47AM +0000, Jaydeep Patil wrote:
> >> >> Hi Rich,
> >> >>
> >> >> The arch/mips/atomic_arch.h uses MIPS2 opcode for LL and SC
> >> >> instructions. Opcodes of these instructions differ on MIPSR6.
> >> >
> >> >Does this mean MIPSR6 is an incompatible ISA that can't run normal
> >> >MIPS binaries? If so that's a messy situation we need to find a way
> >> >to deal with; if the difference is just LLSC though then perhaps the
> >> >kernel's emulation handles it (albeit very slowly).
> >> >
> >> >
> >> >It would be helpful if you could provide a link to the documentation
> >> >of this issue (different opcodes).
> >>
> >> Refer to
> >> https://imagination-technologies-cloudfront-assets.s3.amazonaws.com/do
> >> cumentation/MD00086-2B-MIPS32BIS-AFP-06.04.pdf
> >> (Page 209) for details.
> >
> >Page 454 contains the best info I could find, which seems to say that MIPS R6
> >is essentially a MIPS-incompatible ISA (it can't reliably execute pre-R6 code). Is
> >this correct? If so that's really unfortunate. Unfortunately there does not
> 
> R6 is not binary compatible with pre-R6.

If R6 doesn't even try to be compatible with pre-R6 then I think we
should treat it as a separate subarch and add "r6" in the dynamic
linker name, either before or after the optional "el" component. Is
there some general documentation of this incompatibility? How do GCC
and binutils handle it? Are the ELF files flagged incompatible
somehow?

> >I was just saying it makes the code less cluttered to use them spuriously even
> >though we don't need to:
> >
> >		".set push ; "
> >#if __mips_isa_rev < 6
> >		".set mips2 ; "
> >#endif
> >		"ll %0, %1 ; .set pop"
> >
> >or similar.
> >
> >It's also not clear to me whether the "m" constraint is valid anymore for the R6
> >ll/sc instructions since they take a 9-bit offset now instead of a 16-bit offset.
> >The compiler could generate an address expression whose offset part does
> >not fit in 9 bits. In that case we may need to #if the whole function (or at least
> >the __asm__ statement) separately rather than just skipping the .set mips2....
> >
> 
> The "m" constrain is still valid here, as the offset will be 0 in this case..

How can you assume the offset will be 0? It's the compiler's choice
what to use. For instance, a_cas(&foo->bar, t, s) is likely to have an
offset equal to offsetof(__typeof__(foo),bar). AFAIK this happens in
practice with small offsets in mutex structures, etc. so the bug may
be unlikely to be hit, but I think it's still an incorrect-constraint
bug.

Rich

RE: [PATCH] Fix atomic_arch.h for MIPS32 R6

[Jaydeep Patil]

>-----Original Message-----
>From: Rich Felker [mailto:dalias@aerifal.cx] On Behalf Of Rich Felker
>Sent: 23 March 2016 PM 08:33
>To: Jaydeep Patil
>Cc: musl@lists.openwall.com
>Subject: Re: [musl] [PATCH] Fix atomic_arch.h for MIPS32 R6
>
>On Wed, Mar 23, 2016 at 06:37:41AM +0000, Jaydeep Patil wrote:
>> >-----Original Message-----
>> >From: Rich Felker [mailto:dalias@aerifal.cx] On Behalf Of Rich Felker
>> >Sent: 23 March 2016 AM 02:52
>> >To: musl@lists.openwall.com
>> >Subject: Re: [musl] [PATCH] Fix atomic_arch.h for MIPS32 R6
>> >
>> >On Tue, Mar 22, 2016 at 04:58:51AM +0000, Jaydeep Patil wrote:
>> >> >-----Original Message-----
>> >> >From: Rich Felker [mailto:dalias@aerifal.cx] On Behalf Of
>> >> >dalias@libc.org
>> >> >Sent: 21 March 2016 PM 11:08
>> >> >To: musl@lists.openwall.com
>> >> >Subject: Re: [musl] [PATCH] Fix atomic_arch.h for MIPS32 R6
>> >> >
>> >> >On Mon, Mar 21, 2016 at 06:03:47AM +0000, Jaydeep Patil wrote:
>> >> >> Hi Rich,
>> >> >>
>> >> >> The arch/mips/atomic_arch.h uses MIPS2 opcode for LL and SC
>> >> >> instructions. Opcodes of these instructions differ on MIPSR6.
>> >> >
>> >> >Does this mean MIPSR6 is an incompatible ISA that can't run normal
>> >> >MIPS binaries? If so that's a messy situation we need to find a
>> >> >way to deal with; if the difference is just LLSC though then
>> >> >perhaps the kernel's emulation handles it (albeit very slowly).
>> >> >
>> >> >
>> >> >It would be helpful if you could provide a link to the
>> >> >documentation of this issue (different opcodes).
>> >>
>> >> Refer to
>> >> https://imagination-technologies-cloudfront-assets.s3.amazonaws.com
>> >> /do cumentation/MD00086-2B-MIPS32BIS-AFP-06.04.pdf
>> >> (Page 209) for details.
>> >
>> >Page 454 contains the best info I could find, which seems to say that
>> >MIPS R6 is essentially a MIPS-incompatible ISA (it can't reliably
>> >execute pre-R6 code). Is this correct? If so that's really
>> >unfortunate. Unfortunately there does not
>>
>> R6 is not binary compatible with pre-R6.
>
>If R6 doesn't even try to be compatible with pre-R6 then I think we should
>treat it as a separate subarch and add "r6" in the dynamic linker name, either
>before or after the optional "el" component. Is there some general
>documentation of this incompatibility? How do GCC and binutils handle it? Are
>the ELF files flagged incompatible somehow?

There is no document which talks about this incompatibility particularly.
The e_flags member of the ELF header has two new flags (E_MIPS_ARCH_32R6 and E_MIPS_ARCH_64R6) added for R6.
We can choose to add "r6" in the dynamic linker's name.

>> >I was just saying it makes the code less cluttered to use them
>> >spuriously even though we don't need to:
>> >
>> >		".set push ; "
>> >#if __mips_isa_rev < 6
>> >		".set mips2 ; "
>> >#endif
>> >		"ll %0, %1 ; .set pop"
>> >
>> >or similar.
>> >
>> >It's also not clear to me whether the "m" constraint is valid anymore
>> >for the R6 ll/sc instructions since they take a 9-bit offset now instead of a
>16-bit offset.
>> >The compiler could generate an address expression whose offset part
>> >does not fit in 9 bits. In that case we may need to #if the whole
>> >function (or at least the __asm__ statement) separately rather than just
>skipping the .set mips2....
>> >
>>
>> The "m" constrain is still valid here, as the offset will be 0 in this case..
>
>How can you assume the offset will be 0? It's the compiler's choice what to
>use. For instance, a_cas(&foo->bar, t, s) is likely to have an offset equal to
>offsetof(__typeof__(foo),bar). AFAIK this happens in practice with small
>offsets in mutex structures, etc. so the bug may be unlikely to be hit, but I
>think it's still an incorrect-constraint bug.

Compiler generates appropriate LL/SC based on the offset. 
Compiler adds the offset to the base register if it does not fit 9bits.

>Rich

Re: [PATCH] Fix atomic_arch.h for MIPS32 R6

[Rich Felker]

On Mon, Mar 28, 2016 at 05:07:39AM +0000, Jaydeep Patil wrote:
> >> >I was just saying it makes the code less cluttered to use them
> >> >spuriously even though we don't need to:
> >> >
> >> >		".set push ; "
> >> >#if __mips_isa_rev < 6
> >> >		".set mips2 ; "
> >> >#endif
> >> >		"ll %0, %1 ; .set pop"
> >> >
> >> >or similar.
> >> >
> >> >It's also not clear to me whether the "m" constraint is valid anymore
> >> >for the R6 ll/sc instructions since they take a 9-bit offset now instead of a
> >16-bit offset.
> >> >The compiler could generate an address expression whose offset part
> >> >does not fit in 9 bits. In that case we may need to #if the whole
> >> >function (or at least the __asm__ statement) separately rather than just
> >skipping the .set mips2....
> >> >
> >>
> >> The "m" constrain is still valid here, as the offset will be 0 in this case..
> >
> >How can you assume the offset will be 0? It's the compiler's choice what to
> >use. For instance, a_cas(&foo->bar, t, s) is likely to have an offset equal to
> >offsetof(__typeof__(foo),bar). AFAIK this happens in practice with small
> >offsets in mutex structures, etc. so the bug may be unlikely to be hit, but I
> >think it's still an incorrect-constraint bug.
> 
> Compiler generates appropriate LL/SC based on the offset. 
> Compiler adds the offset to the base register if it does not fit 9bits.

The compiler has no way of knowing that the operand will be used with
ll with the 9-bit offset restriction; as far as it knows, it will be
used in a normal context where a 16-bit offset is valid. I don't have
a toolchain that will target r6, but you can try the following program
which produces an offset of 4096 for loading p[1024]:

unsigned ll1k(volatile unsigned *p)
{
	unsigned val;
	__asm__ __volatile__ ("ll %0, %1" : "=r"(val) : "m"(p[1024]) : "memory" );
	return val;
}

I would expect this to produce errors at assembly time on r6.

Rich

Re: [PATCH] Fix atomic_arch.h for MIPS32 R6

[Rich Felker]

[-- Attachment #1: Type: text/plain, Size: 2473 bytes --]

On Mon, Mar 28, 2016 at 09:04:51AM -0400, Rich Felker wrote:
> On Mon, Mar 28, 2016 at 05:07:39AM +0000, Jaydeep Patil wrote:
> > >> >I was just saying it makes the code less cluttered to use them
> > >> >spuriously even though we don't need to:
> > >> >
> > >> >		".set push ; "
> > >> >#if __mips_isa_rev < 6
> > >> >		".set mips2 ; "
> > >> >#endif
> > >> >		"ll %0, %1 ; .set pop"
> > >> >
> > >> >or similar.
> > >> >
> > >> >It's also not clear to me whether the "m" constraint is valid anymore
> > >> >for the R6 ll/sc instructions since they take a 9-bit offset now instead of a
> > >16-bit offset.
> > >> >The compiler could generate an address expression whose offset part
> > >> >does not fit in 9 bits. In that case we may need to #if the whole
> > >> >function (or at least the __asm__ statement) separately rather than just
> > >skipping the .set mips2....
> > >> >
> > >>
> > >> The "m" constrain is still valid here, as the offset will be 0 in this case..
> > >
> > >How can you assume the offset will be 0? It's the compiler's choice what to
> > >use. For instance, a_cas(&foo->bar, t, s) is likely to have an offset equal to
> > >offsetof(__typeof__(foo),bar). AFAIK this happens in practice with small
> > >offsets in mutex structures, etc. so the bug may be unlikely to be hit, but I
> > >think it's still an incorrect-constraint bug.
> > 
> > Compiler generates appropriate LL/SC based on the offset. 
> > Compiler adds the offset to the base register if it does not fit 9bits.
> 
> The compiler has no way of knowing that the operand will be used with
> ll with the 9-bit offset restriction; as far as it knows, it will be
> used in a normal context where a 16-bit offset is valid. I don't have
> a toolchain that will target r6, but you can try the following program
> which produces an offset of 4096 for loading p[1024]:
> 
> unsigned ll1k(volatile unsigned *p)
> {
> 	unsigned val;
> 	__asm__ __volatile__ ("ll %0, %1" : "=r"(val) : "m"(p[1024]) : "memory" );
> 	return val;
> }
> 
> I would expect this to produce errors at assembly time on r6.

Indeed, the ZC constraint seems to have been added to address this;
see:

https://gcc.gnu.org/onlinedocs/gcc/Machine-Constraints.html

It's not available on old gcc versions (pre-mipsr6) so I think the
attached patch would be appropriate. Let me know if it works for you.

We still need to add r6 detection in configure and the dynamic linker
name in reloc.h, and fix the pthread_arch.h issue.

Rich

[-- Attachment #2: mipsr6-atomic.diff --]
[-- Type: text/plain, Size: 1279 bytes --]

diff --git a/arch/mips/atomic_arch.h b/arch/mips/atomic_arch.h
index ce2823b..ad534f2 100644
--- a/arch/mips/atomic_arch.h
+++ b/arch/mips/atomic_arch.h
@@ -2,11 +2,17 @@
 static inline int a_ll(volatile int *p)
 {
 	int v;
+#if __mips_isa_rev < 6
 	__asm__ __volatile__ (
 		".set push ; .set mips2\n\t"
 		"ll %0, %1"
 		"\n\t.set pop"
 		: "=r"(v) : "m"(*p));
+#else
+	__asm__ __volatile__ (
+		"ll %0, %1"
+		: "=r"(v) : "ZC"(*p));
+#endif
 	return v;
 }
 
@@ -14,24 +20,29 @@ static inline int a_ll(volatile int *p)
 static inline int a_sc(volatile int *p, int v)
 {
 	int r;
+#if __mips_isa_rev < 6
 	__asm__ __volatile__ (
 		".set push ; .set mips2\n\t"
 		"sc %0, %1"
 		"\n\t.set pop"
 		: "=r"(r), "=m"(*p) : "0"(v) : "memory");
+#else
+	__asm__ __volatile__ (
+		"sc %0, %1"
+		: "=r"(r), "=ZC"(*p) : "0"(v) : "memory");
+#endif
 	return r;
 }
 
 #define a_barrier a_barrier
 static inline void a_barrier()
 {
+#if __mips_isa_rev < 2
 	/* mips2 sync, but using too many directives causes
 	 * gcc not to inline it, so encode with .long instead. */
 	__asm__ __volatile__ (".long 0xf" : : : "memory");
-#if 0
-	__asm__ __volatile__ (
-		".set push ; .set mips2 ; sync ; .set pop"
-		: : : "memory");
+#else
+	__asm__ __volatile__ ("sync" : : : "memory");
 #endif
 }
 

RE: [PATCH] Fix atomic_arch.h for MIPS32 R6

[Jaydeep Patil]

>-----Original Message-----
>From: Rich Felker [mailto:dalias@aerifal.cx] On Behalf Of Rich Felker
>Sent: 28 March 2016 PM 06:35
>To: Jaydeep Patil
>Cc: musl@lists.openwall.com
>Subject: Re: [musl] [PATCH] Fix atomic_arch.h for MIPS32 R6
>
>On Mon, Mar 28, 2016 at 05:07:39AM +0000, Jaydeep Patil wrote:
>> >> >I was just saying it makes the code less cluttered to use them
>> >> >spuriously even though we don't need to:
>> >> >
>> >> >		".set push ; "
>> >> >#if __mips_isa_rev < 6
>> >> >		".set mips2 ; "
>> >> >#endif
>> >> >		"ll %0, %1 ; .set pop"
>> >> >
>> >> >or similar.
>> >> >
>> >> >It's also not clear to me whether the "m" constraint is valid
>> >> >anymore for the R6 ll/sc instructions since they take a 9-bit
>> >> >offset now instead of a
>> >16-bit offset.
>> >> >The compiler could generate an address expression whose offset
>> >> >part does not fit in 9 bits. In that case we may need to #if the
>> >> >whole function (or at least the __asm__ statement) separately
>> >> >rather than just
>> >skipping the .set mips2....
>> >> >
>> >>
>> >> The "m" constrain is still valid here, as the offset will be 0 in this case..
>> >
>> >How can you assume the offset will be 0? It's the compiler's choice
>> >what to use. For instance, a_cas(&foo->bar, t, s) is likely to have
>> >an offset equal to offsetof(__typeof__(foo),bar). AFAIK this happens
>> >in practice with small offsets in mutex structures, etc. so the bug
>> >may be unlikely to be hit, but I think it's still an incorrect-constraint bug.
>>
>> Compiler generates appropriate LL/SC based on the offset.
>> Compiler adds the offset to the base register if it does not fit 9bits.
>
>The compiler has no way of knowing that the operand will be used with ll with
>the 9-bit offset restriction; as far as it knows, it will be used in a normal
>context where a 16-bit offset is valid. I don't have a toolchain that will target
>r6, but you can try the following program which produces an offset of 4096 for
>loading p[1024]:
>
>unsigned ll1k(volatile unsigned *p)
>{
>	unsigned val;
>	__asm__ __volatile__ ("ll %0, %1" : "=r"(val) : "m"(p[1024]) :
>"memory" );
>	return val;
>}
>
>I would expect this to produce errors at assembly time on r6.
>Rich

This is what compiler has generated for above function:

$ gcc -c -o main.o main.c -O3 -mips32r6 -mabi=32

Objdump:

00000000 <ll1k>:
   0:   24821000        addiu   v0,a0,4096
   4:   7c420036        ll      v0,0(v0)
   8:   d81f0000        jrc     ra
   c:   00000000        nop

Regards,
Jaydeep

RE: [PATCH] Fix atomic_arch.h for MIPS32 R6

[Jaydeep Patil]

>-----Original Message-----
>From: Rich Felker [mailto:dalias@aerifal.cx] On Behalf Of Rich Felker
>Sent: 28 March 2016 PM 06:35
>To: Jaydeep Patil
>Cc: musl@lists.openwall.com
>Subject: Re: [musl] [PATCH] Fix atomic_arch.h for MIPS32 R6
>
>On Mon, Mar 28, 2016 at 05:07:39AM +0000, Jaydeep Patil wrote:
>> >> >I was just saying it makes the code less cluttered to use them
>> >> >spuriously even though we don't need to:
>> >> >
>> >> >		".set push ; "
>> >> >#if __mips_isa_rev < 6
>> >> >		".set mips2 ; "
>> >> >#endif
>> >> >		"ll %0, %1 ; .set pop"
>> >> >
>> >> >or similar.
>> >> >
>> >> >It's also not clear to me whether the "m" constraint is valid
>> >> >anymore for the R6 ll/sc instructions since they take a 9-bit
>> >> >offset now instead of a
>> >16-bit offset.
>> >> >The compiler could generate an address expression whose offset
>> >> >part does not fit in 9 bits. In that case we may need to #if the
>> >> >whole function (or at least the __asm__ statement) separately
>> >> >rather than just
>> >skipping the .set mips2....
>> >> >
>> >>
>> >> The "m" constrain is still valid here, as the offset will be 0 in this case..
>> >
>> >How can you assume the offset will be 0? It's the compiler's choice
>> >what to use. For instance, a_cas(&foo->bar, t, s) is likely to have
>> >an offset equal to offsetof(__typeof__(foo),bar). AFAIK this happens
>> >in practice with small offsets in mutex structures, etc. so the bug
>> >may be unlikely to be hit, but I think it's still an incorrect-constraint bug.
>>
>> Compiler generates appropriate LL/SC based on the offset.
>> Compiler adds the offset to the base register if it does not fit 9bits.
>
>The compiler has no way of knowing that the operand will be used with ll with
>the 9-bit offset restriction; as far as it knows, it will be used in a normal
>context where a 16-bit offset is valid. I don't have a toolchain that will target
>r6, but you can try the following program which produces an offset of 4096 for
>loading p[1024]:
>
>unsigned ll1k(volatile unsigned *p)
>{
>	unsigned val;
>	__asm__ __volatile__ ("ll %0, %1" : "=r"(val) : "m"(p[1024]) :
>"memory" );
>	return val;
>}
>
>I would expect this to produce errors at assembly time on r6.
>
>Rich

With -O0 optimization level:

00000000 <ll1k>:
   0:   27bdffe8        addiu   sp,sp,-24
   4:   afbe0014        sw      s8,20(sp)
   8:   03a0f025        move    s8,sp
   c:   afc40018        sw      a0,24(s8)
  10:   8fc20018        lw      v0,24(s8)
  14:   24421000        addiu   v0,v0,4096
  18:   7c420036        ll      v0,0(v0)
  1c:   afc20008        sw      v0,8(s8)
  20:   8fc20008        lw      v0,8(s8)
  24:   03c0e825        move    sp,s8
  28:   8fbe0014        lw      s8,20(sp)
  2c:   27bd0018        addiu   sp,sp,24
  30:   d81f0000        jrc     ra

Re: [PATCH] Fix atomic_arch.h for MIPS32 R6

[Rich Felker]

On Tue, Mar 29, 2016 at 03:54:02AM +0000, Jaydeep Patil wrote:
> >-----Original Message-----
> >From: Rich Felker [mailto:dalias@aerifal.cx] On Behalf Of Rich Felker
> >Sent: 28 March 2016 PM 06:35
> >To: Jaydeep Patil
> >Cc: musl@lists.openwall.com
> >Subject: Re: [musl] [PATCH] Fix atomic_arch.h for MIPS32 R6
> >
> >On Mon, Mar 28, 2016 at 05:07:39AM +0000, Jaydeep Patil wrote:
> >> >> >I was just saying it makes the code less cluttered to use them
> >> >> >spuriously even though we don't need to:
> >> >> >
> >> >> >		".set push ; "
> >> >> >#if __mips_isa_rev < 6
> >> >> >		".set mips2 ; "
> >> >> >#endif
> >> >> >		"ll %0, %1 ; .set pop"
> >> >> >
> >> >> >or similar.
> >> >> >
> >> >> >It's also not clear to me whether the "m" constraint is valid
> >> >> >anymore for the R6 ll/sc instructions since they take a 9-bit
> >> >> >offset now instead of a
> >> >16-bit offset.
> >> >> >The compiler could generate an address expression whose offset
> >> >> >part does not fit in 9 bits. In that case we may need to #if the
> >> >> >whole function (or at least the __asm__ statement) separately
> >> >> >rather than just
> >> >skipping the .set mips2....
> >> >> >
> >> >>
> >> >> The "m" constrain is still valid here, as the offset will be 0 in this case..
> >> >
> >> >How can you assume the offset will be 0? It's the compiler's choice
> >> >what to use. For instance, a_cas(&foo->bar, t, s) is likely to have
> >> >an offset equal to offsetof(__typeof__(foo),bar). AFAIK this happens
> >> >in practice with small offsets in mutex structures, etc. so the bug
> >> >may be unlikely to be hit, but I think it's still an incorrect-constraint bug.
> >>
> >> Compiler generates appropriate LL/SC based on the offset.
> >> Compiler adds the offset to the base register if it does not fit 9bits.
> >
> >The compiler has no way of knowing that the operand will be used with ll with
> >the 9-bit offset restriction; as far as it knows, it will be used in a normal
> >context where a 16-bit offset is valid. I don't have a toolchain that will target
> >r6, but you can try the following program which produces an offset of 4096 for
> >loading p[1024]:
> >
> >unsigned ll1k(volatile unsigned *p)
> >{
> >	unsigned val;
> >	__asm__ __volatile__ ("ll %0, %1" : "=r"(val) : "m"(p[1024]) :
> >"memory" );
> >	return val;
> >}
> >
> >I would expect this to produce errors at assembly time on r6.
> >Rich
> 
> This is what compiler has generated for above function:
> 
> $ gcc -c -o main.o main.c -O3 -mips32r6 -mabi=32
> 
> Objdump:
> 
> 00000000 <ll1k>:
>    0:   24821000        addiu   v0,a0,4096
>    4:   7c420036        ll      v0,0(v0)
>    8:   d81f0000        jrc     ra
>    c:   00000000        nop

Can you try gcc -S instead of -c (still at -O3) to produce asm output
without assembling it?

Rich

RE: [PATCH] Fix atomic_arch.h for MIPS32 R6

[Jaydeep Patil]

>-----Original Message-----
>From: Rich Felker [mailto:dalias@aerifal.cx] On Behalf Of Rich Felker
>Sent: 29 March 2016 AM 09:41
>To: Jaydeep Patil
>Cc: musl@lists.openwall.com
>Subject: Re: [musl] [PATCH] Fix atomic_arch.h for MIPS32 R6
>
>On Tue, Mar 29, 2016 at 03:54:02AM +0000, Jaydeep Patil wrote:
>> >-----Original Message-----
>> >From: Rich Felker [mailto:dalias@aerifal.cx] On Behalf Of Rich Felker
>> >Sent: 28 March 2016 PM 06:35
>> >To: Jaydeep Patil
>> >Cc: musl@lists.openwall.com
>> >Subject: Re: [musl] [PATCH] Fix atomic_arch.h for MIPS32 R6
>> >
>> >On Mon, Mar 28, 2016 at 05:07:39AM +0000, Jaydeep Patil wrote:
>> >> >> >I was just saying it makes the code less cluttered to use them
>> >> >> >spuriously even though we don't need to:
>> >> >> >
>> >> >> >		".set push ; "
>> >> >> >#if __mips_isa_rev < 6
>> >> >> >		".set mips2 ; "
>> >> >> >#endif
>> >> >> >		"ll %0, %1 ; .set pop"
>> >> >> >
>> >> >> >or similar.
>> >> >> >
>> >> >> >It's also not clear to me whether the "m" constraint is valid
>> >> >> >anymore for the R6 ll/sc instructions since they take a 9-bit
>> >> >> >offset now instead of a
>> >> >16-bit offset.
>> >> >> >The compiler could generate an address expression whose offset
>> >> >> >part does not fit in 9 bits. In that case we may need to #if
>> >> >> >the whole function (or at least the __asm__ statement)
>> >> >> >separately rather than just
>> >> >skipping the .set mips2....
>> >> >> >
>> >> >>
>> >> >> The "m" constrain is still valid here, as the offset will be 0 in this case..
>> >> >
>> >> >How can you assume the offset will be 0? It's the compiler's
>> >> >choice what to use. For instance, a_cas(&foo->bar, t, s) is likely
>> >> >to have an offset equal to offsetof(__typeof__(foo),bar). AFAIK
>> >> >this happens in practice with small offsets in mutex structures,
>> >> >etc. so the bug may be unlikely to be hit, but I think it's still an incorrect-
>constraint bug.
>> >>
>> >> Compiler generates appropriate LL/SC based on the offset.
>> >> Compiler adds the offset to the base register if it does not fit 9bits.
>> >
>> >The compiler has no way of knowing that the operand will be used with
>> >ll with the 9-bit offset restriction; as far as it knows, it will be
>> >used in a normal context where a 16-bit offset is valid. I don't have
>> >a toolchain that will target r6, but you can try the following
>> >program which produces an offset of 4096 for loading p[1024]:
>> >
>> >unsigned ll1k(volatile unsigned *p)
>> >{
>> >	unsigned val;
>> >	__asm__ __volatile__ ("ll %0, %1" : "=r"(val) : "m"(p[1024]) :
>> >"memory" );
>> >	return val;
>> >}
>> >
>> >I would expect this to produce errors at assembly time on r6.
>> >Rich
>>
>> This is what compiler has generated for above function:
>>
>> $ gcc -c -o main.o main.c -O3 -mips32r6 -mabi=32
>>
>> Objdump:
>>
>> 00000000 <ll1k>:
>>    0:   24821000        addiu   v0,a0,4096
>>    4:   7c420036        ll      v0,0(v0)
>>    8:   d81f0000        jrc     ra
>>    c:   00000000        nop
>
>Can you try gcc -S instead of -c (still at -O3) to produce asm output without
>assembling it?

Generated asssembly:

#APP
 # 4 "test.c" 1
        ll $2, 4096($4)
 # 0 "" 2
#NO_APP
        jrc     $31

Even if we set "noreorder" before LL, assembler generates addiu+ll:

00000000 <ll1k>:
   0:   24821000        addiu   v0,a0,4096
   4:   7c420036        ll      v0,0(v0)
   8:   d81f0000        jrc     ra
   c:   00000000        nop

>Rich

Re: [PATCH] Fix atomic_arch.h for MIPS32 R6

[Rich Felker]

On Tue, Mar 29, 2016 at 07:16:46AM +0000, Jaydeep Patil wrote:
> >-----Original Message-----
> >From: Rich Felker [mailto:dalias@aerifal.cx] On Behalf Of Rich Felker
> >Sent: 29 March 2016 AM 09:41
> >To: Jaydeep Patil
> >Cc: musl@lists.openwall.com
> >Subject: Re: [musl] [PATCH] Fix atomic_arch.h for MIPS32 R6
> >
> >On Tue, Mar 29, 2016 at 03:54:02AM +0000, Jaydeep Patil wrote:
> >> >-----Original Message-----
> >> >From: Rich Felker [mailto:dalias@aerifal.cx] On Behalf Of Rich Felker
> >> >Sent: 28 March 2016 PM 06:35
> >> >To: Jaydeep Patil
> >> >Cc: musl@lists.openwall.com
> >> >Subject: Re: [musl] [PATCH] Fix atomic_arch.h for MIPS32 R6
> >> >
> >> >On Mon, Mar 28, 2016 at 05:07:39AM +0000, Jaydeep Patil wrote:
> >> >> >> >I was just saying it makes the code less cluttered to use them
> >> >> >> >spuriously even though we don't need to:
> >> >> >> >
> >> >> >> >		".set push ; "
> >> >> >> >#if __mips_isa_rev < 6
> >> >> >> >		".set mips2 ; "
> >> >> >> >#endif
> >> >> >> >		"ll %0, %1 ; .set pop"
> >> >> >> >
> >> >> >> >or similar.
> >> >> >> >
> >> >> >> >It's also not clear to me whether the "m" constraint is valid
> >> >> >> >anymore for the R6 ll/sc instructions since they take a 9-bit
> >> >> >> >offset now instead of a
> >> >> >16-bit offset.
> >> >> >> >The compiler could generate an address expression whose offset
> >> >> >> >part does not fit in 9 bits. In that case we may need to #if
> >> >> >> >the whole function (or at least the __asm__ statement)
> >> >> >> >separately rather than just
> >> >> >skipping the .set mips2....
> >> >> >> >
> >> >> >>
> >> >> >> The "m" constrain is still valid here, as the offset will be 0 in this case..
> >> >> >
> >> >> >How can you assume the offset will be 0? It's the compiler's
> >> >> >choice what to use. For instance, a_cas(&foo->bar, t, s) is likely
> >> >> >to have an offset equal to offsetof(__typeof__(foo),bar). AFAIK
> >> >> >this happens in practice with small offsets in mutex structures,
> >> >> >etc. so the bug may be unlikely to be hit, but I think it's still an incorrect-
> >constraint bug.
> >> >>
> >> >> Compiler generates appropriate LL/SC based on the offset.
> >> >> Compiler adds the offset to the base register if it does not fit 9bits.
> >> >
> >> >The compiler has no way of knowing that the operand will be used with
> >> >ll with the 9-bit offset restriction; as far as it knows, it will be
> >> >used in a normal context where a 16-bit offset is valid. I don't have
> >> >a toolchain that will target r6, but you can try the following
> >> >program which produces an offset of 4096 for loading p[1024]:
> >> >
> >> >unsigned ll1k(volatile unsigned *p)
> >> >{
> >> >	unsigned val;
> >> >	__asm__ __volatile__ ("ll %0, %1" : "=r"(val) : "m"(p[1024]) :
> >> >"memory" );
> >> >	return val;
> >> >}
> >> >
> >> >I would expect this to produce errors at assembly time on r6.
> >> >Rich
> >>
> >> This is what compiler has generated for above function:
> >>
> >> $ gcc -c -o main.o main.c -O3 -mips32r6 -mabi=32
> >>
> >> Objdump:
> >>
> >> 00000000 <ll1k>:
> >>    0:   24821000        addiu   v0,a0,4096
> >>    4:   7c420036        ll      v0,0(v0)
> >>    8:   d81f0000        jrc     ra
> >>    c:   00000000        nop
> >
> >Can you try gcc -S instead of -c (still at -O3) to produce asm output without
> >assembling it?
> 
> Generated asssembly:
> 
> #APP
>  # 4 "test.c" 1
>         ll $2, 4096($4)
>  # 0 "" 2
> #NO_APP
>         jrc     $31
> 
> Even if we set "noreorder" before LL, assembler generates addiu+ll:
> 
> 00000000 <ll1k>:
>    0:   24821000        addiu   v0,a0,4096
>    4:   7c420036        ll      v0,0(v0)
>    8:   d81f0000        jrc     ra
>    c:   00000000        nop

I see. I suspected the assembler was doing it. "noat", not
"noreorder", is the way to suppress things like this but I doubt even
"noat" does it since a separate temp register ("at") is not needed in
this case.

If all assembers that support R6 support this rewriting, then the ZC
constraint in gcc is really just an optimization, not strictly
necessary. We should probably check (1) whether clang's internal
assembler can do the rewriting, and (2) whether clang supports the ZC
constraint. I would prefer using ZC but I want to do whatever is more
compatible; I don't think the codegen efficiency matters a lot either
way.

Rich

RE: [PATCH] Fix atomic_arch.h for MIPS32 R6

[Jaydeep Patil]

>-----Original Message-----
>From: Rich Felker [mailto:dalias@aerifal.cx] On Behalf Of Rich Felker
>Sent: 29 March 2016 PM 07:03
>To: Jaydeep Patil
>Cc: musl@lists.openwall.com
>Subject: Re: [musl] [PATCH] Fix atomic_arch.h for MIPS32 R6
>
>On Tue, Mar 29, 2016 at 07:16:46AM +0000, Jaydeep Patil wrote:
>> >-----Original Message-----
>> >From: Rich Felker [mailto:dalias@aerifal.cx] On Behalf Of Rich Felker
>> >Sent: 29 March 2016 AM 09:41
>> >To: Jaydeep Patil
>> >Cc: musl@lists.openwall.com
>> >Subject: Re: [musl] [PATCH] Fix atomic_arch.h for MIPS32 R6
>> >
>> >On Tue, Mar 29, 2016 at 03:54:02AM +0000, Jaydeep Patil wrote:
>> >> >-----Original Message-----
>> >> >From: Rich Felker [mailto:dalias@aerifal.cx] On Behalf Of Rich
>> >> >Felker
>> >> >Sent: 28 March 2016 PM 06:35
>> >> >To: Jaydeep Patil
>> >> >Cc: musl@lists.openwall.com
>> >> >Subject: Re: [musl] [PATCH] Fix atomic_arch.h for MIPS32 R6
>> >> >
>> >> >On Mon, Mar 28, 2016 at 05:07:39AM +0000, Jaydeep Patil wrote:
>> >> >> >> >I was just saying it makes the code less cluttered to use
>> >> >> >> >them spuriously even though we don't need to:
>> >> >> >> >
>> >> >> >> >		".set push ; "
>> >> >> >> >#if __mips_isa_rev < 6
>> >> >> >> >		".set mips2 ; "
>> >> >> >> >#endif
>> >> >> >> >		"ll %0, %1 ; .set pop"
>> >> >> >> >
>> >> >> >> >or similar.
>> >> >> >> >
>> >> >> >> >It's also not clear to me whether the "m" constraint is
>> >> >> >> >valid anymore for the R6 ll/sc instructions since they take
>> >> >> >> >a 9-bit offset now instead of a
>> >> >> >16-bit offset.
>> >> >> >> >The compiler could generate an address expression whose
>> >> >> >> >offset part does not fit in 9 bits. In that case we may need
>> >> >> >> >to #if the whole function (or at least the __asm__
>> >> >> >> >statement) separately rather than just
>> >> >> >skipping the .set mips2....
>> >> >> >> >
>> >> >> >>
>> >> >> >> The "m" constrain is still valid here, as the offset will be 0 in this
>case..
>> >> >> >
>> >> >> >How can you assume the offset will be 0? It's the compiler's
>> >> >> >choice what to use. For instance, a_cas(&foo->bar, t, s) is
>> >> >> >likely to have an offset equal to
>> >> >> >offsetof(__typeof__(foo),bar). AFAIK this happens in practice
>> >> >> >with small offsets in mutex structures, etc. so the bug may be
>> >> >> >unlikely to be hit, but I think it's still an incorrect-
>> >constraint bug.
>> >> >>
>> >> >> Compiler generates appropriate LL/SC based on the offset.
>> >> >> Compiler adds the offset to the base register if it does not fit 9bits.
>> >> >
>> >> >The compiler has no way of knowing that the operand will be used
>> >> >with ll with the 9-bit offset restriction; as far as it knows, it
>> >> >will be used in a normal context where a 16-bit offset is valid. I
>> >> >don't have a toolchain that will target r6, but you can try the
>> >> >following program which produces an offset of 4096 for loading p[1024]:
>> >> >
>> >> >unsigned ll1k(volatile unsigned *p) {
>> >> >	unsigned val;
>> >> >	__asm__ __volatile__ ("ll %0, %1" : "=r"(val) : "m"(p[1024]) :
>> >> >"memory" );
>> >> >	return val;
>> >> >}
>> >> >
>> >> >I would expect this to produce errors at assembly time on r6.
>> >> >Rich
>> >>
>> >> This is what compiler has generated for above function:
>> >>
>> >> $ gcc -c -o main.o main.c -O3 -mips32r6 -mabi=32
>> >>
>> >> Objdump:
>> >>
>> >> 00000000 <ll1k>:
>> >>    0:   24821000        addiu   v0,a0,4096
>> >>    4:   7c420036        ll      v0,0(v0)
>> >>    8:   d81f0000        jrc     ra
>> >>    c:   00000000        nop
>> >
>> >Can you try gcc -S instead of -c (still at -O3) to produce asm output
>> >without assembling it?
>>
>> Generated asssembly:
>>
>> #APP
>>  # 4 "test.c" 1
>>         ll $2, 4096($4)
>>  # 0 "" 2
>> #NO_APP
>>         jrc     $31
>>
>> Even if we set "noreorder" before LL, assembler generates addiu+ll:
>>
>> 00000000 <ll1k>:
>>    0:   24821000        addiu   v0,a0,4096
>>    4:   7c420036        ll      v0,0(v0)
>>    8:   d81f0000        jrc     ra
>>    c:   00000000        nop
>
>I see. I suspected the assembler was doing it. "noat", not "noreorder", is the
>way to suppress things like this but I doubt even "noat" does it since a
>separate temp register ("at") is not needed in this case.
>
>If all assembers that support R6 support this rewriting, then the ZC constraint
>in gcc is really just an optimization, not strictly necessary. We should probably
>check (1) whether clang's internal assembler can do the rewriting, and (2)
>whether clang supports the ZC constraint. I would prefer using ZC but I want
>to do whatever is more compatible; I don't think the codegen efficiency
>matters a lot either way.
>Rich

Clang's integrated assembler does not support this rewriting. However ZC is supported.
I have modified both atomic_arch.h and pthread_arch.h to reflect this. 
Please refer to https://github.com/JaydeepIMG/musl-1/tree/fix_inline_asm_for_R6 for the patch (also listed below).
I have also added R6 as subarch.



From 20054ee55643d9e81163ca58ac63cc38b5080969 Mon Sep 17 00:00:00 2001
From: Jaydeep Patil <jaydeep.patil@imgtec.com>
Date: Wed, 30 Mar 2016 10:37:30 +0100
Subject: [PATCH] [MIPS] Update inline asm for R6 and add R6 as subtarget

---
 arch/mips/atomic_arch.h    | 17 +++--------------
 arch/mips/pthread_arch.h   |  8 +-------
 arch/mips64/atomic_arch.h  | 12 +++++-------
 arch/mips64/pthread_arch.h |  7 +------
 configure                  |  2 ++
 5 files changed, 12 insertions(+), 34 deletions(-)

diff --git a/arch/mips/atomic_arch.h b/arch/mips/atomic_arch.h
index ce2823b..4dbe4bb 100644
--- a/arch/mips/atomic_arch.h
+++ b/arch/mips/atomic_arch.h
@@ -3,10 +3,8 @@ static inline int a_ll(volatile int *p)
 {
 	int v;
 	__asm__ __volatile__ (
-		".set push ; .set mips2\n\t"
 		"ll %0, %1"
-		"\n\t.set pop"
-		: "=r"(v) : "m"(*p));
+		: "=r"(v) : "ZC"(*p));
 	return v;
 }
 
@@ -15,24 +13,15 @@ static inline int a_sc(volatile int *p, int v)
 {
 	int r;
 	__asm__ __volatile__ (
-		".set push ; .set mips2\n\t"
 		"sc %0, %1"
-		"\n\t.set pop"
-		: "=r"(r), "=m"(*p) : "0"(v) : "memory");
+		: "=r"(r), "=ZC"(*p) : "0"(v) : "memory");
 	return r;
 }
 
 #define a_barrier a_barrier
 static inline void a_barrier()
 {
-	/* mips2 sync, but using too many directives causes
-	 * gcc not to inline it, so encode with .long instead. */
-	__asm__ __volatile__ (".long 0xf" : : : "memory");
-#if 0
-	__asm__ __volatile__ (
-		".set push ; .set mips2 ; sync ; .set pop"
-		: : : "memory");
-#endif
+	__asm__ __volatile__ ("sync" : : : "memory");
 }
 
 #define a_pre_llsc a_barrier
diff --git a/arch/mips/pthread_arch.h b/arch/mips/pthread_arch.h
index 8a49965..d8b6955 100644
--- a/arch/mips/pthread_arch.h
+++ b/arch/mips/pthread_arch.h
@@ -1,13 +1,7 @@
 static inline struct pthread *__pthread_self()
 {
-#ifdef __clang__
-	char *tp;
-	__asm__ __volatile__ (".word 0x7c03e83b ; move %0, $3" : "=r" (tp) : : "$3" );
-#else
 	register char *tp __asm__("$3");
-	/* rdhwr $3,$29 */
-	__asm__ __volatile__ (".word 0x7c03e83b" : "=r" (tp) );
-#endif
+	__asm__ __volatile__ ("rdhwr %0,$29" : "=r" (tp));
 	return (pthread_t)(tp - 0x7000 - sizeof(struct pthread));
 }
 
diff --git a/arch/mips64/atomic_arch.h b/arch/mips64/atomic_arch.h
index b468fd9..ac92891 100644
--- a/arch/mips64/atomic_arch.h
+++ b/arch/mips64/atomic_arch.h
@@ -4,7 +4,7 @@ static inline int a_ll(volatile int *p)
 	int v;
 	__asm__ __volatile__ (
 		"ll %0, %1"
-		: "=r"(v) : "m"(*p));
+		: "=r"(v) : "ZC"(*p));
 	return v;
 }
 
@@ -14,7 +14,7 @@ static inline int a_sc(volatile int *p, int v)
 	int r;
 	__asm__ __volatile__ (
 		"sc %0, %1"
-		: "=r"(r), "=m"(*p) : "0"(v) : "memory");
+		: "=r"(r), "=ZC"(*p) : "0"(v) : "memory");
 	return r;
 }
 
@@ -24,7 +24,7 @@ static inline void *a_ll_p(volatile void *p)
 	void *v;
 	__asm__ __volatile__ (
 		"lld %0, %1"
-		: "=r"(v) : "m"(*(void *volatile *)p));
+		: "=r"(v) : "ZC"(*(void *volatile *)p));
 	return v;
 }
 
@@ -34,16 +34,14 @@ static inline int a_sc_p(volatile void *p, void *v)
 	long r;
 	__asm__ __volatile__ (
 		"scd %0, %1"
-		: "=r"(r), "=m"(*(void *volatile *)p) : "0"(v) : "memory");
+		: "=r"(r), "=ZC"(*(void *volatile *)p) : "0"(v) : "memory");
 	return r;
 }
 
 #define a_barrier a_barrier
 static inline void a_barrier()
 {
-	/* mips2 sync, but using too many directives causes
-	 * gcc not to inline it, so encode with .long instead. */
-	__asm__ __volatile__ (".long 0xf" : : : "memory");
+	__asm__ __volatile__ ("sync" : : : "memory");
 }
 
 #define a_pre_llsc a_barrier
diff --git a/arch/mips64/pthread_arch.h b/arch/mips64/pthread_arch.h
index b42edbe..d8b6955 100644
--- a/arch/mips64/pthread_arch.h
+++ b/arch/mips64/pthread_arch.h
@@ -1,12 +1,7 @@
 static inline struct pthread *__pthread_self()
 {
-#ifdef __clang__
-	char *tp;
-	__asm__ __volatile__ (".word 0x7c03e83b ; move %0, $3" : "=r" (tp) : : "$3" );
-#else
 	register char *tp __asm__("$3");
-	__asm__ __volatile__ (".word 0x7c03e83b" : "=r" (tp) );
-#endif
+	__asm__ __volatile__ ("rdhwr %0,$29" : "=r" (tp));
 	return (pthread_t)(tp - 0x7000 - sizeof(struct pthread));
 }
 
diff --git a/configure b/configure
index 213a825..969671d 100755
--- a/configure
+++ b/configure
@@ -612,11 +612,13 @@ trycppif __AARCH64EB__ "$t" && SUBARCH=${SUBARCH}_be
 fi
 
 if test "$ARCH" = "mips" ; then
+trycppif "__mips_isa_rev >= 6" "$t" && SUBARCH=${SUBARCH}r6
 trycppif "_MIPSEL || __MIPSEL || __MIPSEL__" "$t" && SUBARCH=${SUBARCH}el
 trycppif __mips_soft_float "$t" && SUBARCH=${SUBARCH}-sf
 fi
 
 if test "$ARCH" = "mips64" ; then
+trycppif "__mips_isa_rev >= 6" "$t" && SUBARCH=${SUBARCH}r6
 trycppif "_MIPSEL || __MIPSEL || __MIPSEL__" "$t" && SUBARCH=${SUBARCH}el
 trycppif __mips_soft_float "$t" && SUBARCH=${SUBARCH}-sf
 fi
-- 
2.1.4


Thanks,
Jaydeep

Re: [PATCH] Fix atomic_arch.h for MIPS32 R6

[Rich Felker]

On Wed, Mar 30, 2016 at 09:45:59AM +0000, Jaydeep Patil wrote:
> >> >> >The compiler has no way of knowing that the operand will be used
> >> >> >with ll with the 9-bit offset restriction; as far as it knows, it
> >> >> >will be used in a normal context where a 16-bit offset is valid. I
> >> >> >don't have a toolchain that will target r6, but you can try the
> >> >> >following program which produces an offset of 4096 for loading p[1024]:
> >> >> >
> >> >> >unsigned ll1k(volatile unsigned *p) {
> >> >> >	unsigned val;
> >> >> >	__asm__ __volatile__ ("ll %0, %1" : "=r"(val) : "m"(p[1024]) :
> >> >> >"memory" );
> >> >> >	return val;
> >> >> >}
> >> >> >
> >> >> >I would expect this to produce errors at assembly time on r6.
> >> >> >Rich
> >> >>
> >> >> This is what compiler has generated for above function:
> >> >>
> >> >> $ gcc -c -o main.o main.c -O3 -mips32r6 -mabi=32
> >> >>
> >> >> Objdump:
> >> >>
> >> >> 00000000 <ll1k>:
> >> >>    0:   24821000        addiu   v0,a0,4096
> >> >>    4:   7c420036        ll      v0,0(v0)
> >> >>    8:   d81f0000        jrc     ra
> >> >>    c:   00000000        nop
> >> >
> >> >Can you try gcc -S instead of -c (still at -O3) to produce asm output
> >> >without assembling it?
> >>
> >> Generated asssembly:
> >>
> >> #APP
> >>  # 4 "test.c" 1
> >>         ll $2, 4096($4)
> >>  # 0 "" 2
> >> #NO_APP
> >>         jrc     $31
> >>
> >> Even if we set "noreorder" before LL, assembler generates addiu+ll:
> >>
> >> 00000000 <ll1k>:
> >>    0:   24821000        addiu   v0,a0,4096
> >>    4:   7c420036        ll      v0,0(v0)
> >>    8:   d81f0000        jrc     ra
> >>    c:   00000000        nop
> >
> >I see. I suspected the assembler was doing it. "noat", not "noreorder", is the
> >way to suppress things like this but I doubt even "noat" does it since a
> >separate temp register ("at") is not needed in this case.
> >
> >If all assembers that support R6 support this rewriting, then the ZC constraint
> >in gcc is really just an optimization, not strictly necessary. We should probably
> >check (1) whether clang's internal assembler can do the rewriting, and (2)
> >whether clang supports the ZC constraint. I would prefer using ZC but I want
> >to do whatever is more compatible; I don't think the codegen efficiency
> >matters a lot either way.
> >Rich
> 
> Clang's integrated assembler does not support this rewriting. However ZC is supported.
> I have modified both atomic_arch.h and pthread_arch.h to reflect this. 
> Please refer to https://github.com/JaydeepIMG/musl-1/tree/fix_inline_asm_for_R6 for the patch (also listed below).
> I have also added R6 as subarch.
> 
> 
> 
> >From 20054ee55643d9e81163ca58ac63cc38b5080969 Mon Sep 17 00:00:00 2001
> From: Jaydeep Patil <jaydeep.patil@imgtec.com>
> Date: Wed, 30 Mar 2016 10:37:30 +0100
> Subject: [PATCH] [MIPS] Update inline asm for R6 and add R6 as subtarget
> 
> ---
>  arch/mips/atomic_arch.h    | 17 +++--------------
>  arch/mips/pthread_arch.h   |  8 +-------
>  arch/mips64/atomic_arch.h  | 12 +++++-------
>  arch/mips64/pthread_arch.h |  7 +------
>  configure                  |  2 ++
>  5 files changed, 12 insertions(+), 34 deletions(-)
> 
> diff --git a/arch/mips/atomic_arch.h b/arch/mips/atomic_arch.h
> index ce2823b..4dbe4bb 100644
> --- a/arch/mips/atomic_arch.h
> +++ b/arch/mips/atomic_arch.h
> @@ -3,10 +3,8 @@ static inline int a_ll(volatile int *p)
>  {
>  	int v;
>  	__asm__ __volatile__ (
> -		".set push ; .set mips2\n\t"
>  		"ll %0, %1"
> -		"\n\t.set pop"
> -		: "=r"(v) : "m"(*p));
> +		: "=r"(v) : "ZC"(*p));
>  	return v;
>  }

Did you see the proposed patch I sent? Your version does not work
because it removes support for pre-mips2. It also removed support for
gcc versions prior to 4.9, since "ZC" was not added until r6 support
was added (or maybe later). My version of the patch addresses both of
these issues by conditioning on __mips_isa_rev>=6.

>  #define a_pre_llsc a_barrier
> diff --git a/arch/mips/pthread_arch.h b/arch/mips/pthread_arch.h
> index 8a49965..d8b6955 100644
> --- a/arch/mips/pthread_arch.h
> +++ b/arch/mips/pthread_arch.h
> @@ -1,13 +1,7 @@
>  static inline struct pthread *__pthread_self()
>  {
> -#ifdef __clang__
> -	char *tp;
> -	__asm__ __volatile__ (".word 0x7c03e83b ; move %0, $3" : "=r" (tp) : : "$3" );
> -#else
>  	register char *tp __asm__("$3");
> -	/* rdhwr $3,$29 */
> -	__asm__ __volatile__ (".word 0x7c03e83b" : "=r" (tp) );
> -#endif
> +	__asm__ __volatile__ ("rdhwr %0,$29" : "=r" (tp));
>  	return (pthread_t)(tp - 0x7000 - sizeof(struct pthread));
>  }

This also needs to either be conditioned on __mips_isa_rev>=2 or
similar, or else the .set push/pop stuff needs to be used for
__mips_isa_rev<6.

>  #define a_barrier a_barrier
>  static inline void a_barrier()
>  {
> -	/* mips2 sync, but using too many directives causes
> -	 * gcc not to inline it, so encode with .long instead. */
> -	__asm__ __volatile__ (".long 0xf" : : : "memory");
> +	__asm__ __volatile__ ("sync" : : : "memory");
>  }

I think this should be okay, since all mips64 ISA levels have this
insn as far as I know.

> diff --git a/configure b/configure
> index 213a825..969671d 100755
> --- a/configure
> +++ b/configure
> @@ -612,11 +612,13 @@ trycppif __AARCH64EB__ "$t" && SUBARCH=${SUBARCH}_be
>  fi
>  
>  if test "$ARCH" = "mips" ; then
> +trycppif "__mips_isa_rev >= 6" "$t" && SUBARCH=${SUBARCH}r6
>  trycppif "_MIPSEL || __MIPSEL || __MIPSEL__" "$t" && SUBARCH=${SUBARCH}el
>  trycppif __mips_soft_float "$t" && SUBARCH=${SUBARCH}-sf
>  fi
>  
>  if test "$ARCH" = "mips64" ; then
> +trycppif "__mips_isa_rev >= 6" "$t" && SUBARCH=${SUBARCH}r6
>  trycppif "_MIPSEL || __MIPSEL || __MIPSEL__" "$t" && SUBARCH=${SUBARCH}el
>  trycppif __mips_soft_float "$t" && SUBARCH=${SUBARCH}-sf
>  fi

This looks ok, but a similar condition in arch/mips*/reloc.h is also
needed.

Since I've done most of the thinking about the above issues already
and have a patch for some of them, let me prepare a complete patch and
send it to the list for you to check and make sure it meets your needs
for r6. I should be able to prepare it very quickly. Then we can look
at applying the same changes to the n32 port and reviewing it.

Rich

Re: [PATCH] Fix atomic_arch.h for MIPS32 R6

[Rich Felker]

[-- Attachment #1: Type: text/plain, Size: 784 bytes --]

On Wed, Mar 30, 2016 at 10:29:26AM -0400, Rich Felker wrote:
> Since I've done most of the thinking about the above issues already
> and have a patch for some of them, let me prepare a complete patch and
> send it to the list for you to check and make sure it meets your needs
> for r6. I should be able to prepare it very quickly. Then we can look
> at applying the same changes to the n32 port and reviewing it.

Can you see if the attached patch works for you? It not only adds r6
support but improves support for non-baseline (i.e. > mips1) ISA
levels by optimizing out the unnecessary .set's (which hurt gcc's
inlining, because gcc is dumb about them) and lifts the $3 register
restriction on rdhwr for ISA levels where the instructions are known
to be available natively.

Rich

[-- Attachment #2: mipsr6.diff --]
[-- Type: text/plain, Size: 5927 bytes --]

diff --git a/arch/mips/atomic_arch.h b/arch/mips/atomic_arch.h
index ce2823b..1248d17 100644
--- a/arch/mips/atomic_arch.h
+++ b/arch/mips/atomic_arch.h
@@ -1,12 +1,24 @@
+#if __mips_isa_rev < 6
+#define LLSC_M "m"
+#else
+#define LLSC_M "ZC"
+#endif
+
 #define a_ll a_ll
 static inline int a_ll(volatile int *p)
 {
 	int v;
+#if __mips < 2
 	__asm__ __volatile__ (
 		".set push ; .set mips2\n\t"
 		"ll %0, %1"
 		"\n\t.set pop"
 		: "=r"(v) : "m"(*p));
+#else
+	__asm__ __volatile__ (
+		"ll %0, %1"
+		: "=r"(v) : LLSC_M(*p));
+#endif
 	return v;
 }
 
@@ -14,26 +26,33 @@ static inline int a_ll(volatile int *p)
 static inline int a_sc(volatile int *p, int v)
 {
 	int r;
+#if __mips < 2
 	__asm__ __volatile__ (
 		".set push ; .set mips2\n\t"
 		"sc %0, %1"
 		"\n\t.set pop"
 		: "=r"(r), "=m"(*p) : "0"(v) : "memory");
+#else
+	__asm__ __volatile__ (
+		"sc %0, %1"
+		: "=r"(r), "="LLSC_M(*p) : "0"(v) : "memory");
+#endif
 	return r;
 }
 
 #define a_barrier a_barrier
 static inline void a_barrier()
 {
+#if __mips < 2
 	/* mips2 sync, but using too many directives causes
 	 * gcc not to inline it, so encode with .long instead. */
 	__asm__ __volatile__ (".long 0xf" : : : "memory");
-#if 0
-	__asm__ __volatile__ (
-		".set push ; .set mips2 ; sync ; .set pop"
-		: : : "memory");
+#else
+	__asm__ __volatile__ ("sync" : : : "memory");
 #endif
 }
 
 #define a_pre_llsc a_barrier
 #define a_post_llsc a_barrier
+
+#undef LLSC_M
diff --git a/arch/mips/pthread_arch.h b/arch/mips/pthread_arch.h
index 8a49965..e581265 100644
--- a/arch/mips/pthread_arch.h
+++ b/arch/mips/pthread_arch.h
@@ -1,12 +1,11 @@
 static inline struct pthread *__pthread_self()
 {
-#ifdef __clang__
-	char *tp;
-	__asm__ __volatile__ (".word 0x7c03e83b ; move %0, $3" : "=r" (tp) : : "$3" );
-#else
+#if __mips_isa_rev < 2
 	register char *tp __asm__("$3");
-	/* rdhwr $3,$29 */
 	__asm__ __volatile__ (".word 0x7c03e83b" : "=r" (tp) );
+#else
+	char *tp;
+	__asm__ __volatile__ ("rdhwr %0, $29" : "=r" (tp) );
 #endif
 	return (pthread_t)(tp - 0x7000 - sizeof(struct pthread));
 }
diff --git a/arch/mips/reloc.h b/arch/mips/reloc.h
index 8c52df0..b3d59a4 100644
--- a/arch/mips/reloc.h
+++ b/arch/mips/reloc.h
@@ -1,5 +1,11 @@
 #include <endian.h>
 
+#if __mips_isa_rev >= 6
+#define ISA_SUFFIX "r6"
+#else
+#define ISA_SUFFIX ""
+#endif
+
 #if __BYTE_ORDER == __LITTLE_ENDIAN
 #define ENDIAN_SUFFIX "el"
 #else
@@ -12,7 +18,7 @@
 #define FP_SUFFIX ""
 #endif
 
-#define LDSO_ARCH "mips" ENDIAN_SUFFIX FP_SUFFIX
+#define LDSO_ARCH "mips" ISA_SUFFIX ENDIAN_SUFFIX FP_SUFFIX
 
 #define TPOFF_K (-0x7000)
 
diff --git a/arch/mips64/atomic_arch.h b/arch/mips64/atomic_arch.h
index b468fd9..d0f8b4a 100644
--- a/arch/mips64/atomic_arch.h
+++ b/arch/mips64/atomic_arch.h
@@ -1,10 +1,16 @@
+#if __mips_isa_rev < 6
+#define LLSC_M "m"
+#else
+#define LLSC_M "ZC"
+#endif
+
 #define a_ll a_ll
 static inline int a_ll(volatile int *p)
 {
 	int v;
 	__asm__ __volatile__ (
 		"ll %0, %1"
-		: "=r"(v) : "m"(*p));
+		: "=r"(v) : LLSC_M(*p));
 	return v;
 }
 
@@ -14,7 +20,7 @@ static inline int a_sc(volatile int *p, int v)
 	int r;
 	__asm__ __volatile__ (
 		"sc %0, %1"
-		: "=r"(r), "=m"(*p) : "0"(v) : "memory");
+		: "=r"(r), "="LLSC_M(*p) : "0"(v) : "memory");
 	return r;
 }
 
@@ -24,7 +30,7 @@ static inline void *a_ll_p(volatile void *p)
 	void *v;
 	__asm__ __volatile__ (
 		"lld %0, %1"
-		: "=r"(v) : "m"(*(void *volatile *)p));
+		: "=r"(v) : LLSC_M(*(void *volatile *)p));
 	return v;
 }
 
@@ -34,17 +40,17 @@ static inline int a_sc_p(volatile void *p, void *v)
 	long r;
 	__asm__ __volatile__ (
 		"scd %0, %1"
-		: "=r"(r), "=m"(*(void *volatile *)p) : "0"(v) : "memory");
+		: "=r"(r), "="LLSC_M(*(void *volatile *)p) : "0"(v) : "memory");
 	return r;
 }
 
 #define a_barrier a_barrier
 static inline void a_barrier()
 {
-	/* mips2 sync, but using too many directives causes
-	 * gcc not to inline it, so encode with .long instead. */
-	__asm__ __volatile__ (".long 0xf" : : : "memory");
+	__asm__ __volatile__ ("sync" : : : "memory");
 }
 
 #define a_pre_llsc a_barrier
 #define a_post_llsc a_barrier
+
+#undef LLSC_M
diff --git a/arch/mips64/pthread_arch.h b/arch/mips64/pthread_arch.h
index b42edbe..e581265 100644
--- a/arch/mips64/pthread_arch.h
+++ b/arch/mips64/pthread_arch.h
@@ -1,11 +1,11 @@
 static inline struct pthread *__pthread_self()
 {
-#ifdef __clang__
-	char *tp;
-	__asm__ __volatile__ (".word 0x7c03e83b ; move %0, $3" : "=r" (tp) : : "$3" );
-#else
+#if __mips_isa_rev < 2
 	register char *tp __asm__("$3");
 	__asm__ __volatile__ (".word 0x7c03e83b" : "=r" (tp) );
+#else
+	char *tp;
+	__asm__ __volatile__ ("rdhwr %0, $29" : "=r" (tp) );
 #endif
 	return (pthread_t)(tp - 0x7000 - sizeof(struct pthread));
 }
diff --git a/arch/mips64/reloc.h b/arch/mips64/reloc.h
index 5933147..bbd9bd9 100644
--- a/arch/mips64/reloc.h
+++ b/arch/mips64/reloc.h
@@ -4,6 +4,12 @@
 #define _GNU_SOURCE
 #include <endian.h>
 
+#if __mips_isa_rev >= 6
+#define ISA_SUFFIX "r6"
+#else
+#define ISA_SUFFIX ""
+#endif
+
 #if __BYTE_ORDER == __LITTLE_ENDIAN
 #define ENDIAN_SUFFIX "el"
 #else
@@ -16,7 +22,7 @@
 #define FP_SUFFIX ""
 #endif
 
-#define LDSO_ARCH "mips64" ENDIAN_SUFFIX FP_SUFFIX
+#define LDSO_ARCH "mips64" ISA_SUFFIX ENDIAN_SUFFIX FP_SUFFIX
 
 #define TPOFF_K (-0x7000)
 
diff --git a/configure b/configure
index 213a825..969671d 100755
--- a/configure
+++ b/configure
@@ -612,11 +612,13 @@ trycppif __AARCH64EB__ "$t" && SUBARCH=${SUBARCH}_be
 fi
 
 if test "$ARCH" = "mips" ; then
+trycppif "__mips_isa_rev >= 6" "$t" && SUBARCH=${SUBARCH}r6
 trycppif "_MIPSEL || __MIPSEL || __MIPSEL__" "$t" && SUBARCH=${SUBARCH}el
 trycppif __mips_soft_float "$t" && SUBARCH=${SUBARCH}-sf
 fi
 
 if test "$ARCH" = "mips64" ; then
+trycppif "__mips_isa_rev >= 6" "$t" && SUBARCH=${SUBARCH}r6
 trycppif "_MIPSEL || __MIPSEL || __MIPSEL__" "$t" && SUBARCH=${SUBARCH}el
 trycppif __mips_soft_float "$t" && SUBARCH=${SUBARCH}-sf
 fi

RE: [PATCH] Fix atomic_arch.h for MIPS32 R6

[Jaydeep Patil]

>-----Original Message-----
>From: Rich Felker [mailto:dalias@aerifal.cx] On Behalf Of Rich Felker
>Sent: 30 March 2016 PM 08:58
>To: Jaydeep Patil
>Cc: musl@lists.openwall.com
>Subject: Re: [musl] [PATCH] Fix atomic_arch.h for MIPS32 R6
>
>On Wed, Mar 30, 2016 at 10:29:26AM -0400, Rich Felker wrote:
>> Since I've done most of the thinking about the above issues already
>> and have a patch for some of them, let me prepare a complete patch and
>> send it to the list for you to check and make sure it meets your needs
>> for r6. I should be able to prepare it very quickly. Then we can look
>> at applying the same changes to the n32 port and reviewing it.
>
>Can you see if the attached patch works for you? It not only adds r6 support
>but improves support for non-baseline (i.e. > mips1) ISA levels by optimizing
>out the unnecessary .set's (which hurt gcc's inlining, because gcc is dumb
>about them) and lifts the $3 register restriction on rdhwr for ISA levels where
>the instructions are known to be available natively.

The patch works fine. I have tried it for mips32 r2-r6, mips64 r2-r6, micromips32 r2-r5.
I have tried it with both GCC and Clang 3.9.0 compilers.

>Rich

Thanks,
Jaydeep