Re: [PATCH] Fix pthread_arch.h for microMIPS

["dalias@libc.org" <dalias@libc.org>]

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