Re: [PATCH, NDS32] patch for nds32 port

[Pei-Shiang Hung <pshung807 at gmail dot com>]

Dear Corinna,

Thanks the time for review.
I have revised the patches and changelog.

For your questions,
1. Several system calls whose error states are not defined, thus
ignore calling SYS_geterr. Like, _gettimeofday.
2. !!WATCH OUT!!, originally, this comment is used to comment that
register $r2 have been set to zero so that we don't have to initial
$r2 in L_call_main.
I have removed this comment.

Thank you.



2014-12-12 0:37 GMT+08:00 Corinna Vinschen <vinschen@redhat.com>:
> Hi Pei-Shiang Hung,
>
>
> On Dec 11 18:30, Pei-Shiang Hung wrote:
>> Dear all,
>>
>> Total are 3 patches.
>> The first patch doesn't include the generated files.
>
> Thanks.  I scanned the patches and patch 1 and 3 look good to me.
> In patch 2 I see a few (minor) issues.
>
>>  * libgloss/nds32/_exit.S : NDS virtual hosting support
>>  * libgloss/nds32/_getpid.S : NDS virtual hosting support
>>  * libgloss/nds32/_gettimeofday.S : NDS virtual hosting support
>>  * libgloss/nds32/_isatty.S : NDS virtual hosting support
>>  * libgloss/nds32/_kill.S : NDS virtual hosting support
>>  * libgloss/nds32/_link.S : NDS virtual hosting support
>>  * libgloss/nds32/_times.S : NDS virtual hosting support
>>  * libgloss/nds32/_unlink.S : NDS virtual hosting support
>
> I think this entry is not right.  Your unlink.S patch fixes a copy/paste
> bug only AFAICS.
>
>> +     .size   _getpid, .-_getpid
>> diff --git a/libgloss/nds32/_gettimeofday.S b/libgloss/nds32/_gettimeofday.S
>> index adc5f68..e78bd5e 100644
>> --- a/libgloss/nds32/_gettimeofday.S
>> +++ b/libgloss/nds32/_gettimeofday.S
>> @@ -30,13 +30,19 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>>  #ifdef __NDS32_VH__
>>
>>  #include "vh.h"
>> -.extern _impure_ptr
>>  TYPE3 _gettimeofday, VH_GETTIMEOFDAY
>>
>>  #else        /* not __NDS32_VH__ */
>>
>>  #include "../syscall.h"
>>  #include "syscall_extra.h"
>> -SYS_WRAPPER _gettimeofday, SYS_gettimeofday
>> +     .text
>> +     .global _gettimeofday
>> +     .type   _gettimeofday, @function
>> +     .align  2
>> +_gettimeofday:
>> +     syscall SYS_gettimeofday
>> +     ret
>> +     .size   _gettimeofday, .-_gettimeofday
>
> So, what's the difference to the old implementation?  AFAICS, the
> new implementation neglects to call __syscall_error_handler, but
> otherwise it's the same.  What's the advantage?  In how far does
> this change add "NDS virtual hosting support"?  Care to explain?
>
>> @@ -97,22 +128,54 @@ _start:
>>        */
>>       la      $r0, _edata
>>       la      $r1, _end
>> +     /* !!! WATCH OUT !!!  */
>
> Watch out?  Yes, ok, but... what for?  Either this comment should go away,
> or it would help to extend the comment to explain what for and why.
>
> Under the premise that I'm not at all familiar with the CPU, the rest
> looks ok to me.
>
>
> Thanks,
> Corinna
>
> --
> Corinna Vinschen
> Cygwin Maintainer
> Red Hat

Attachment: Changelog1.txt
Description: Text document

Attachment: 0001-PATCH-NDS32-libm-nds32.patch
Description: Binary data

Attachment: 0002-PATCH-NDS32-NDS32-libgross.patch
Description: Binary data

Attachment: 0003-patch-3-3-FPU-support-for-setjmp.patch
Description: Binary data

Attachment: Changelog2.txt
Description: Text document

Attachment: Changelog3.txt
Description: Text document