Re: [PATCH 1/2 Version 6] Add recording support for the ISA 3.1 Powerpc instructions

[Joel Brobecker <brobecker@adacore.com>]

> Thanks for the additional comments.  Given that we are not under any
> timeline to get this patch done and the changes are very minor I think
> it is better to just get it right the first time rather than have to
> come back ad fix it up later.  The changes are all localized as well.

That's a great attitude to have.

For large patches, it can have a bit of a perverse effect, though,
which is related to how code review is done for GDB (email-based).
So, to review a new iteration of a patch I already reviewed before,
when the patch is large, I tend to go fish in the archives the
previous iteration, and do a diff between previous and new. Easy
enough, but it requires a bit of time to do so. If I trust a person
enough to follow up, or when things are very minor, I tend to let
the patch through, and ask that the corrections be done as a followup
patch. It's true that this means two patches instead of one, so this
could be seen as a real downside -- but the upside is that the minor
updates are sent for review on their own, and very often, reviewing
just that update patch will be trivial, and therefore the chances
of it being reviewed are much higher in my experience.

I was able to take a bit of time today to review the new version,
though, and this version looks good to me. So you can go ahead
and push to master.

Thanks again.

> I have made the changes to the header comment for the function
> ppc_record_ACC_fpscr() to clairfy what the function does as well as to
> use the capitalization to indicate the value.  The parameter "at" was
> also changed to "entry" to make it clearer we are selecting one of the
> possible AT entries to be recorded.  Finally, argument save_fpscr was
> changed to a boolean thus eliminating the #defines for RECORD_FPSCR and
> DO_NOT_RECORD_FPSCR.  Fortunately, the changes are all at the beginning
> of the patch making it easy to find.
> 
> As always, the patch was retested to make sure I didn't break anything.
> Please let me know if the additional changes are acceptable.  Thanks.
-- 
Joel