Re: [Qemu-devel] [PATCH 16/21] target-arm: switch to AREG0 free mode

[Peter Maydell]

On 3 September 2012 01:01, Peter Maydell <address@hidden> wrote:
> On 2 September 2012 18:33, Blue Swirl <address@hidden> wrote:
>> Add an explicit CPUState parameter instead of relying on AREG0
>> and switch to AREG0 free mode.
>>
>> Signed-off-by: Blue Swirl <address@hidden>
>> ---
>>  configure                |    2 +-
>>  target-arm/Makefile.objs |    2 -
>>  target-arm/cpu.h         |   10 ++-
>>  target-arm/helper.c      |    8 +-
>>  target-arm/helper.h      |   60 +++++++++---------
>>  target-arm/op_helper.c   |   92 +++++++++++++---------------
>>  target-arm/translate.c   |  148 
>> +++++++++++++++++++++++-----------------------
>>  7 files changed, 158 insertions(+), 164 deletions(-)
>
> This is too big to easily review -- it's making a change to a lot
> of helpers, and in each case that change affects three places
> (callers, declaration, implementation). That'

Sorry, finger slip meant I sent that half finished. To continue...

That's quite hard to cross-reference when the patch is this big.
I think it would be helpful if you could split it up into patches
touching smaller groups of helpers at once rather than having a
single patch that does them all at once.

thanks
-- PMM