RE: [MUSL] microMIPS32R2 O32 port

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

>-----Original Message-----
>From: Rich Felker [mailto:dalias@aerifal.cx] On Behalf Of Rich Felker
>Sent: 21 April 2017 PM 07:03
>To: Jaydeep Patil
>Cc: Szabolcs Nagy; musl@lists.openwall.com; Andre McCurdy
>Subject: Re: [musl] [MUSL] microMIPS32R2 O32 port
>
>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.

The crt1.o and __cp_begin are built as pure MIPS functions rather than microMIPS as they are reading data using $ra register (with ISA bit set).
We can clear the ISA bit (using INS) and choose to compile these functions as microMIPS.


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

ISA bit is set for all microMIPS symbols (including the dot symbol). The GETFUNCSYM macro cannot be compiled as pure MIPS code as it might be expanded in a microMIPS function.
We need to clear the ISA bit of $ra before using it as a base address in load/store instructions.


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

Branch delay slots are removed (called as compact instructions) in the newer MIPS/microMIPS cores (in development). 
The MIPS/microMIPS R2-R6 still support instructions with delay slot.
Assembler takes care of converting a BRANCH + NOP to its appropriate compact instruction (BEQ + NOP to BEQC). 
With the v1 branch I was trying to create generic hand-written assembly which can be used for newer cores with the compact instructions.
However I realized that it would appropriate to create a new arch instead of creating generic assembly.
Thus in v2 branch I modified only those functions which would create issues when compiled with interlinking on.


>Rich

Thanks,
Jaydeep