Re: [MUSL] microMIPS32R2 O32 port

[Rich Felker <dalias@libc.org>]

On Thu, Apr 13, 2017 at 10:37:05AM +0000, Jaydeep Patil wrote:
> Hi Szabolcs,
> 
> Please find the attached patch.

I still don't follow the motivation of all the changes in this patch.
See comments below:

> [...]
> diff --git a/arch/mips/crt_arch.h b/arch/mips/crt_arch.h
> index 9fc50d7..78832b0 100644
> --- a/arch/mips/crt_arch.h
> +++ b/arch/mips/crt_arch.h
> @@ -1,6 +1,7 @@
>  __asm__(
>  ".set push\n"
>  ".set noreorder\n"
> +".set nomicromips\n"
>  ".text \n"
>  ".global _" START "\n"
>  ".global " START "\n"

Is there a need for crt1.o (or other users of this file like
dlstart/rcrt1) to be built as plain mips rather than micromips? If so,
please explain. Arbitrary changes like this without an explanation of
why they're being made (what was broken before, and why this is the
correct fix) are not acceptable.

> diff --git a/arch/mips/reloc.h b/arch/mips/reloc.h
> index b3d59a4..772b3aa 100644
> --- a/arch/mips/reloc.h
> +++ b/arch/mips/reloc.h
> @@ -36,15 +36,23 @@
>  #define CRTJMP(pc,sp) __asm__ __volatile__( \
>  	"move $sp,%1 ; jr %0" : : "r"(pc), "r"(sp) : "memory" )
>  
> +/*
> + * When compiled for microMIPS, .align makes sure that .gpword
> + * is placed at word boundary. $ra must point to first .gpword.
> + * ISA bit of $ra must be cleared for microMIPS before using it
> + * as a base address. For MIPS, ISA bit is always zero.
> +*/
>  #define GETFUNCSYM(fp, sym, got) __asm__ ( \
>  	".hidden " #sym "\n" \
>  	".set push \n" \
>  	".set noreorder \n" \
> +	"	.align 2 \n" \
>  	"	bal 1f \n" \
>  	"	 nop \n" \
>  	"	.gpword . \n" \
>  	"	.gpword " #sym " \n" \
> -	"1:	lw %0, ($ra) \n" \
> +	"1:	ins $ra, $0, 0, 1 \n" \
> +	"	lw %0, ($ra) \n" \
>  	"	subu %0, $ra, %0 \n" \
>  	"	lw $ra, 4($ra) \n" \
>  	"	addu %0, %0, $ra \n" \

By this, do you mean that .gpword is producing a value that's relative
to the actual byte position of the directive rather than the logical
(ISA bit set) value of "." at the point of .gpword?

> diff --git a/src/thread/mips/syscall_cp.s b/src/thread/mips/syscall_cp.s
> index d284626..9c5f55e 100644
> --- a/src/thread/mips/syscall_cp.s
> +++ b/src/thread/mips/syscall_cp.s
> @@ -1,5 +1,5 @@
>  .set    noreorder
> -
> +.set    nomicromips
>  .global __cp_begin
>  .hidden __cp_begin
>  .type   __cp_begin,@function

I'm also unclear on the motivation of this one. Before (v1) you had a
lot of changes to replace .s files with something micromips-compatible
(removing branch delay slots); now (v2) those changes are not
included. So are .s files even being built as micromips at all? If
not, why is the above needed? If so, how do the files with delay slots
work?

Rich