RE: [PATCH] Fix pthread_arch.h for microMIPS

[Jaydeep Patil <Jaydeep.Patil@imgtec.com>]

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