[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <BD7773622145634B952E5B54ACA8E349AA24DA07@PUMAIL01.pu.imgtec.org>
Date: Mon, 11 Apr 2016 03:40:03 +0000
From: Jaydeep Patil <Jaydeep.Patil@...tec.com>
To: "musl@...ts.openwall.com" <musl@...ts.openwall.com>
Subject: RE: MIPS64 N32 port
>-----Original Message-----
>From: Rich Felker [mailto:dalias@...ifal.cx] On Behalf Of Rich Felker
>Sent: 10 April 2016 AM 06:18
>To: musl@...ts.openwall.com
>Subject: Re: [musl] [MUSL] MIPS64 N32 port
>
>On Tue, Apr 05, 2016 at 05:57:26AM +0000, Jaydeep Patil wrote:
>> Please refer to
>> https://github.com/JaydeepIMG/musl-1/tree/mipsn32port_v1 for rebased
>> MIPS64 N32 port (also attached).
>
>I tested this on my flight back and it seems to be mostly working,
>except for configure failing to detect n32. Some comments inline:
>
>> +#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");
>> +#else
>> + __asm__ __volatile__ ("sync" : : : "memory");
>> +#endif
>> +}
>
>I think __mips<2 is mutually exclusive with 64-bit so it should be
>okay to omit this.
>
>> diff --git a/configure b/configure
>> index 969671d..dbc0198 100755
>> --- a/configure
>> +++ b/configure
>> @@ -307,6 +307,7 @@ i?86*) ARCH=i386 ;;
>> x86_64-x32*|x32*|x86_64*x32) ARCH=x32 ;;
>> x86_64-nt64*) ARCH=nt64 ;;
>> x86_64*) ARCH=x86_64 ;;
>> +mipsn32) ARCH=mipsn32 ;;
>> mips64*) ARCH=mips64 ;;
>> mips*) ARCH=mips ;;
>> microblaze*) ARCH=microblaze ;;
>
>This pattern does not seem to match any tuple actually used; it
>doesn't even have a glob * in it. On my first try, I got a heap of
>warnings and a broken build because musl detected the n32 compiler as
>mips64.
>
>It doesn't seem to be possible to detect from the tuple that a
>compiler is n32, so I think the right fix is something like:
>
>@@ -618,6 +619,7 @@ trycppif __mips_soft_float "$t" &&
>SUBARCH=${SUBARCH}-sf
> fi
>
> if test "$ARCH" = "mips64" ; then
>+trycppif "_MIPS_SIM != _ABI64" "$t" && ARCH=mipsn32
> 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
Ok
>Alternatively, instead of depending on the predefined macros we could
>compile a test program that errors out depending on sizeof(void*)
>being 4 vs 8. If we take the latter option, a new trystaticassert
>function should probably be added to configure to make it easy to
>reuse the logic. But I think the preprocessor version is okay for now.
>
>I'd really like to transition the configure script to only using the
>-dumpmachine tuple as a hint, and only when it's available, and
>checking predefined macros and such to verify the arch.
>
>> @@ -611,6 +612,12 @@ if test "$ARCH" = "aarch64" ; then
>> trycppif __AARCH64EB__ "$t" && SUBARCH=${SUBARCH}_be
>> fi
>>
>> +if test "$ARCH" = "mipsn32" ; 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 hunk would become unnecessary then.
Yes
>> diff --git a/src/thread/mipsn32/__unmapself.s
>b/src/thread/mipsn32/__unmapself.s
>> new file mode 100644
>> index 0000000..771f8b3
>> --- /dev/null
>> +++ b/src/thread/mipsn32/__unmapself.s
>> @@ -0,0 +1,10 @@
>> +.set noreorder
>> +.global __unmapself
>> +.type __unmapself,@function
>> +__unmapself:
>> + move $sp, $25
>> + li $2, 6011
>> + syscall
>> + li $4, 0
>> + li $2, 6058
>> + syscall
>
>I don't think the move $sp, $25 is needed here. If you read the git
>history for why it was added, it way to work around an o32 kernel bug
>that does not affect other ABIs. The git log message references the
>relevant kernel commit and I checked the kernel git history and
>verified that the affected code is o32-only.
Ok.
>Otherwise, it looks very good. Thanks for working on this and for your
>patience. If the above suggested changes look okay with you I can make
>them and commit it.
Thanks for the review.
Could you please commit it?
>Rich
Thanks,
Jaydeep
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
