[PATCH] Fix pthread_arch.h for microMIPS

[PATCH] Fix pthread_arch.h for microMIPS

[Jaydeep Patil]

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

Hi Rich,

The patch fixes a link time error when compiled for microMIPS. The pthread_self() function has been modified to use rdhwr instruction instead of .word directive.
The change has been done for both clang and gcc. Functions containing .word are not compiled for microMIPS.

Please refer to https://github.com/JaydeepIMG/musl-1/tree/fix_rdhwr_for_umips for details.



From 09e4e395d9f1538edb548ffaa02db74e8e11701e Mon Sep 17 00:00:00 2001
From: Jaydeep Patil <jaydeep.patil@imgtec.com>
Date: Mon, 21 Mar 2016 09:53:37 +0000
Subject: [PATCH] Use rdhwr insn instead of .word for microMIPS

---
arch/mips/pthread_arch.h   | 10 ++--------
arch/mips64/pthread_arch.h |  9 ++-------
2 files changed, 4 insertions(+), 15 deletions(-)

diff --git a/arch/mips/pthread_arch.h b/arch/mips/pthread_arch.h
index 8a49965..30e2394 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
+       register char *tp;
+       __asm__ __volatile__ ("rdhwr %0,$29" : "=r" (tp));
        return (pthread_t)(tp - 0x7000 - sizeof(struct pthread));
}

diff --git a/arch/mips64/pthread_arch.h b/arch/mips64/pthread_arch.h
index b42edbe..30e2394 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
+       register char *tp;
+       __asm__ __volatile__ ("rdhwr %0,$29" : "=r" (tp));
        return (pthread_t)(tp - 0x7000 - sizeof(struct pthread));
}

--
2.1.4

Thanks,
Jaydeep


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

Re: [PATCH] Fix pthread_arch.h for microMIPS

[dalias]

On Mon, Mar 21, 2016 at 10:01:02AM +0000, Jaydeep Patil wrote:
> Hi Rich,
> 
> The patch fixes a link time error when compiled for microMIPS. The
> pthread_self() function has been modified to use rdhwr instruction
> instead of .word directive.
> The change has been done for both clang and gcc. Functions
> containing .word are not compiled for microMIPS.
> 
> Please refer to https://github.com/JaydeepIMG/musl-1/tree/fix_rdhwr_for_umips for details.
> 
> 
> 
> >From 09e4e395d9f1538edb548ffaa02db74e8e11701e Mon Sep 17 00:00:00 2001
> From: Jaydeep Patil <jaydeep.patil@imgtec.com>
> Date: Mon, 21 Mar 2016 09:53:37 +0000
> Subject: [PATCH] Use rdhwr insn instead of .word for microMIPS
> 
> ---
> arch/mips/pthread_arch.h   | 10 ++--------
> arch/mips64/pthread_arch.h |  9 ++-------
> 2 files changed, 4 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/mips/pthread_arch.h b/arch/mips/pthread_arch.h
> index 8a49965..30e2394 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
> +       register char *tp;
> +       __asm__ __volatile__ ("rdhwr %0,$29" : "=r" (tp));
>         return (pthread_t)(tp - 0x7000 - sizeof(struct pthread));
> }

You can't remove the register constraint to use $3 here; the reason
for the constraint is not that the opcode is hard-coded, but that the
kernel's fast-path emulation for MIPS-I, MIPS-II, and MIPS32r1 cpus
that lack support for this hardware register only works when $3 is
used as the destination register. Otherwise a very slow path for
emulation is taken. (On our part, this probably should be documented
in a comment -- sorry it's not.)

There are probably other reasons we're using .word instead of the
mnemonic here too; I suspect it fails to assemble without .set to a
proper ISA level or sufficient -march. This needs to be checked. Is
there a reason the .word doesn't work on microMIPS? I thought the
32-bit opcodes were the same but maybe I'm mistaken.

Rich

RE: [PATCH] Fix pthread_arch.h for microMIPS

[Jaydeep Patil]

>-----Original Message-----
>From: Rich Felker [mailto:dalias@aerifal.cx] On Behalf Of dalias@libc.org
>Sent: 21 March 2016 PM 11:13
>To: musl@lists.openwall.com
>Subject: Re: [musl] [PATCH] Fix pthread_arch.h for microMIPS
>
>On Mon, Mar 21, 2016 at 10:01:02AM +0000, Jaydeep Patil wrote:
>> Hi Rich,
>>
>> The patch fixes a link time error when compiled for microMIPS. The
>> pthread_self() function has been modified to use rdhwr instruction
>> instead of .word directive.
>> The change has been done for both clang and gcc. Functions containing
>> .word are not compiled for microMIPS.
>>
>> Please refer to https://github.com/JaydeepIMG/musl-
>1/tree/fix_rdhwr_for_umips for details.
>>
>>
>>
>> >From 09e4e395d9f1538edb548ffaa02db74e8e11701e Mon Sep 17 00:00:00
>> >2001
>> From: Jaydeep Patil <jaydeep.patil@imgtec.com>
>> Date: Mon, 21 Mar 2016 09:53:37 +0000
>> Subject: [PATCH] Use rdhwr insn instead of .word for microMIPS
>>
>> ---
>> arch/mips/pthread_arch.h   | 10 ++--------
>> arch/mips64/pthread_arch.h |  9 ++-------
>> 2 files changed, 4 insertions(+), 15 deletions(-)
>>
>> diff --git a/arch/mips/pthread_arch.h b/arch/mips/pthread_arch.h index
>> 8a49965..30e2394 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
>> +       register char *tp;
>> +       __asm__ __volatile__ ("rdhwr %0,$29" : "=r" (tp));
>>         return (pthread_t)(tp - 0x7000 - sizeof(struct pthread)); }
>
>You can't remove the register constraint to use $3 here; the reason for the
>constraint is not that the opcode is hard-coded, but that the kernel's fast-path
>emulation for MIPS-I, MIPS-II, and MIPS32r1 cpus that lack support for this
>hardware register only works when $3 is used as the destination register.
>Otherwise a very slow path for emulation is taken. (On our part, this probably
>should be documented in a comment -- sorry it's not.)

Yes, $3 must be used

>
>There are probably other reasons we're using .word instead of the mnemonic
>here too; I suspect it fails to assemble without .set to a proper ISA level or
>sufficient -march. This needs to be checked. Is there a reason the .word
>doesn't work on microMIPS? I thought the 32-bit opcodes were the same but
>maybe I'm mistaken.
>

Assembler fails to compile this function for microMIPS when it sees a .word. 
Opcodes also differ for microMIPS. 
Refer to https://imagination-technologies-cloudfront-assets.s3.amazonaws.com/documentation/MD00086-2B-MIPS32BIS-AFP-06.04.pdf (Page 320) for details.

>Rich

Re: [PATCH] Fix pthread_arch.h for microMIPS

[dalias]

On Tue, Mar 22, 2016 at 05:09:39AM +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:13
> >To: musl@lists.openwall.com
> >Subject: Re: [musl] [PATCH] Fix pthread_arch.h for microMIPS
> >
> >On Mon, Mar 21, 2016 at 10:01:02AM +0000, Jaydeep Patil wrote:
> >> Hi Rich,
> >>
> >> The patch fixes a link time error when compiled for microMIPS. The
> >> pthread_self() function has been modified to use rdhwr instruction
> >> instead of .word directive.
> >> The change has been done for both clang and gcc. Functions containing
> >> .word are not compiled for microMIPS.
> >>
> >> Please refer to https://github.com/JaydeepIMG/musl-
> >1/tree/fix_rdhwr_for_umips for details.
> >>
> >>
> >>
> >> >From 09e4e395d9f1538edb548ffaa02db74e8e11701e Mon Sep 17 00:00:00
> >> >2001
> >> From: Jaydeep Patil <jaydeep.patil@imgtec.com>
> >> Date: Mon, 21 Mar 2016 09:53:37 +0000
> >> Subject: [PATCH] Use rdhwr insn instead of .word for microMIPS
> >>
> >> ---
> >> arch/mips/pthread_arch.h   | 10 ++--------
> >> arch/mips64/pthread_arch.h |  9 ++-------
> >> 2 files changed, 4 insertions(+), 15 deletions(-)
> >>
> >> diff --git a/arch/mips/pthread_arch.h b/arch/mips/pthread_arch.h index
> >> 8a49965..30e2394 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
> >> +       register char *tp;
> >> +       __asm__ __volatile__ ("rdhwr %0,$29" : "=r" (tp));
> >>         return (pthread_t)(tp - 0x7000 - sizeof(struct pthread)); }
> >
> >You can't remove the register constraint to use $3 here; the reason for the
> >constraint is not that the opcode is hard-coded, but that the kernel's fast-path
> >emulation for MIPS-I, MIPS-II, and MIPS32r1 cpus that lack support for this
> >hardware register only works when $3 is used as the destination register.
> >Otherwise a very slow path for emulation is taken. (On our part, this probably
> >should be documented in a comment -- sorry it's not.)
> 
> Yes, $3 must be used
> 
> >
> >There are probably other reasons we're using .word instead of the mnemonic
> >here too; I suspect it fails to assemble without .set to a proper ISA level or
> >sufficient -march. This needs to be checked. Is there a reason the .word
> >doesn't work on microMIPS? I thought the 32-bit opcodes were the same but
> >maybe I'm mistaken.
> >
> 
> Assembler fails to compile this function for microMIPS when it sees a .word.. 
> Opcodes also differ for microMIPS. 
> Refer to https://imagination-technologies-cloudfront-assets.s3.amazonaws.com/documentation/MD00086-2B-MIPS32BIS-AFP-06.04.pdf (Page 320) for details.

That document does not seem to have microMIPS instruction encodings,
but I found it on page 485 here:

https://imagination-technologies-cloudfront-assets.s3.amazonaws.com/documentation/MD00594-2B-microMIPS64-AFP-05.04.pdf

I think the clean solution here is probably just to use the mnemonic
if __mips_isa_rev >= 2 (or whatever the right level is) and only
hard-code the opcode (or maybe a .set approach like for ll/sc) for
__mips_isa_rev < 2. Does this sound okay?

Also, does anyone know if the __clang__ hack for failure to support
named register constraints is still needed? It can be preserved if
needed but I'd rather drop it to declutter and get better codegen on
clang.

Rich

RE: [PATCH] Fix pthread_arch.h for microMIPS

[Jaydeep Patil]

>-----Original Message-----
>From: Rich Felker [mailto:dalias@aerifal.cx] On Behalf Of dalias@libc.org
>Sent: 23 March 2016 AM 03:08
>To: musl@lists.openwall.com
>Subject: Re: [musl] [PATCH] Fix pthread_arch.h for microMIPS
>
>On Tue, Mar 22, 2016 at 05:09:39AM +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:13
>> >To: musl@lists.openwall.com
>> >Subject: Re: [musl] [PATCH] Fix pthread_arch.h for microMIPS
>> >
>> >On Mon, Mar 21, 2016 at 10:01:02AM +0000, Jaydeep Patil wrote:
>> >> Hi Rich,
>> >>
>> >> The patch fixes a link time error when compiled for microMIPS. The
>> >> pthread_self() function has been modified to use rdhwr instruction
>> >> instead of .word directive.
>> >> The change has been done for both clang and gcc. Functions
>> >> containing .word are not compiled for microMIPS.
>> >>
>> >> Please refer to https://github.com/JaydeepIMG/musl-
>> >1/tree/fix_rdhwr_for_umips for details.
>> >>
>> >>
>> >>
>> >> >From 09e4e395d9f1538edb548ffaa02db74e8e11701e Mon Sep 17
>00:00:00
>> >> >2001
>> >> From: Jaydeep Patil <jaydeep.patil@imgtec.com>
>> >> Date: Mon, 21 Mar 2016 09:53:37 +0000
>> >> Subject: [PATCH] Use rdhwr insn instead of .word for microMIPS
>> >>
>> >> ---
>> >> arch/mips/pthread_arch.h   | 10 ++--------
>> >> arch/mips64/pthread_arch.h |  9 ++-------
>> >> 2 files changed, 4 insertions(+), 15 deletions(-)
>> >>
>> >> diff --git a/arch/mips/pthread_arch.h b/arch/mips/pthread_arch.h
>> >> index
>> >> 8a49965..30e2394 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
>> >> +       register char *tp;
>> >> +       __asm__ __volatile__ ("rdhwr %0,$29" : "=r" (tp));
>> >>         return (pthread_t)(tp - 0x7000 - sizeof(struct pthread)); }
>> >
>> >You can't remove the register constraint to use $3 here; the reason
>> >for the constraint is not that the opcode is hard-coded, but that the
>> >kernel's fast-path emulation for MIPS-I, MIPS-II, and MIPS32r1 cpus
>> >that lack support for this hardware register only works when $3 is used as
>the destination register.
>> >Otherwise a very slow path for emulation is taken. (On our part, this
>> >probably should be documented in a comment -- sorry it's not.)
>>
>> Yes, $3 must be used
>>
>> >
>> >There are probably other reasons we're using .word instead of the
>> >mnemonic here too; I suspect it fails to assemble without .set to a
>> >proper ISA level or sufficient -march. This needs to be checked. Is
>> >there a reason the .word doesn't work on microMIPS? I thought the
>> >32-bit opcodes were the same but maybe I'm mistaken.
>> >
>>
>> Assembler fails to compile this function for microMIPS when it sees a .word..
>> Opcodes also differ for microMIPS.
>> Refer to https://imagination-technologies-cloudfront-
>assets.s3.amazonaws.com/documentation/MD00086-2B-MIPS32BIS-AFP-
>06.04.pdf (Page 320) for details.
>
>That document does not seem to have microMIPS instruction encodings, but I
>found it on page 485 here:
>
>https://imagination-technologies-cloudfront-
>assets.s3.amazonaws.com/documentation/MD00594-2B-microMIPS64-AFP-
>05.04.pdf
>
>I think the clean solution here is probably just to use the mnemonic if
>__mips_isa_rev >= 2 (or whatever the right level is) and only hard-code the
>opcode (or maybe a .set approach like for ll/sc) for __mips_isa_rev < 2. Does
>this sound okay?

Okay

>Also, does anyone know if the __clang__ hack for failure to support named
>register constraints is still needed? It can be preserved if needed but I'd rather
>drop it to declutter and get better codegen on clang.

Named register constrains are supported with Clang. We can use __asm__ ("$3") here.

>Rich