Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sat, 9 Apr 2016 20:48:19 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: 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

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.

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

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.

Rich

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.