Re: [PATCH 0/24] MIPS: Add support for reconfigurable FPR size, MIPS MSA and MIPSR6,support

Re: [PATCH 0/24] MIPS: Add support for reconfigurable FPR size, MIPS MSA and MIPSR6,support

[Maciej W. Rozycki]

Bhushan,

 First of all thank you for your submission, and also apologies for the 
long RTT.

 Now a couple of general questions/notes:

1. Can you please reformat the e-mails with patches such that they can be 
   readily fed to Git?  Using `git format-patch' would be the usual way,
   although of course feel free to do it however you like.  In the form 
   you posted they have leading blanks across descriptions which do not
   work well with `git am' (see also 
   <https://sourceware.org/gdb/wiki/ContributionChecklist>).

2. You haven't told us how you verified these changes.  Have you run any 
   regression testing at all?  If so, the which targets have you tested?
   Also please make sure that you at least build changes which affect
   generic files with `--enable-targets=all'.

3. Some changes touch files in the include/ and opcodes/ subdirectories.  
   These (except for include/gdb/) are maintained as a part of the 
   binutils project and need to be posted to the relevant mailing list, 
   which is <binutils@sourceware.org>.  I'll review the relevant parts in 
   this round with an appropriate `Cc' added, but please pay attention to
   have the binutils mailing list included where appropriate in the 
   submission.  In fact especially changes to opcodes/ and include/opcode/ 
   appear self-contained and largely independent from the GDB part of the
   containing patch, so they may best be split off and submitted to the
   binutils project first.  More on this in replies to individual patches.

>     1. Handles run-time reconfigurable FPR size. MIPS revision 2 onwards 
> MIPS32
> 
>     and MIPS64 both can have either 32-bit or 64-bit FPRs. This is decided
> 
>     based on the values of CP1.FIR.F64, CP0.SR.FR and CP0.Config5.FRE bits.
> 
>     Patch 1 to patch 12 in the series handles this property. This work is
> based
> 
>     on the patch submitted earlier by Maciej W. Rozycki. Please refer to
> 
>     https://sourceware.org/ml/gdb-patches/2012-06/msg00201.html for details.
> So
> 
>     this series includes his patch (updated to GDB 7.11 release version) as
> the
> 
>     [patch 1] of the series.

 For the record please note that the availability of CP0.Status.FR goes 
back to the MIPS III ISA and therefore all 64-bit MIPS processors have it.
So if you need to determine the presence of this bit, then the algorithm 
conceptually is (isa_is_64bit || CP1.FIR.F64).

 This introduction is not going to end up in our Git repository, but for 
the sake of people's confusion avoidance please update it in patch 
resubmission.

 Finally please note that I cannot approve changes outside the MIPS 
target, but to speed up the process I'll check them regardless to see if 
there are any obvious issues.  You'll still have to seek a global 
maintainer's approval for those changes though.

  Maciej

Re: [PATCH 0/24] MIPS: Add support for reconfigurable FPR size, MIPS MSA and MIPSR6,support

[Pedro Alves]

On 07/25/2016 02:58 PM, Maciej W. Rozycki wrote:

>  Finally please note that I cannot approve changes outside the MIPS 
> target, but to speed up the process I'll check them regardless to see if 
> there are any obvious issues.  You'll still have to seek a global 
> maintainer's approval for those changes though.
> 

I can't speak for others, but at least I was waiting for a
repost and for Maciej to validate the overall idea.

Also, for some reason, neither I nor the mailing list archives
seem to have received the whole patch set:

  https://sourceware.org/ml/gdb-patches/2016-06/msg00510.html

FYI, in case a global-maintainer review was the blocker here.

Thanks,
Pedro Alves

Re: [PATCH 0/24] MIPS: Add support for reconfigurable FPR size, MIPS MSA and MIPSR6,support

[Maciej W. Rozycki]

On Tue, 4 Oct 2016, Pedro Alves wrote:

> >  Finally please note that I cannot approve changes outside the MIPS 
> > target, but to speed up the process I'll check them regardless to see if 
> > there are any obvious issues.  You'll still have to seek a global 
> > maintainer's approval for those changes though.
> 
> I can't speak for others, but at least I was waiting for a
> repost and for Maciej to validate the overall idea.

 After numerous hiccups and distractions I am about to get back to this 
review.

 For the record -- I have come across an inconsistency in FCR handling, 
coming from a regression introduced with XML description support for the 
MIPS target, affecting this patch series.  I am about to commit a fix, 
after the final testing I'm still working on; I'd like to see it there in 
7.12 so that we have a fixed proper release before MSA goes in.  Code in 
the patch set will then have to be adjusted accordingly.

 Parts of some patches apply to binutils rather than GDB and need to be 
split, so I'll go through the series as they are so that there is no need 
to proliferate reposts unnecessarily.

> Also, for some reason, neither I nor the mailing list archives
> seem to have received the whole patch set:
> 
>   https://sourceware.org/ml/gdb-patches/2016-06/msg00510.html

 There has been an issue with this submission indeed in that it arrived as 
two separate threads, but the remaining patches do have made it there:

https://sourceware.org/ml/gdb-patches/2016-06/msg00425.html

> FYI, in case a global-maintainer review was the blocker here.

 I don't think so, not at this stage.  We need to sort out the general 
structure of the patch set first.  Please feel free, or indeed anyone 
else, to chime in if you like, of course.

  Maciej