On 3 September 2012 01:01, Peter Maydell <peter.mayd...@linaro.org> wrote: > On 2 September 2012 18:33, Blue Swirl <blauwir...@gmail.com> wrote: >> Add an explicit CPUState parameter instead of relying on AREG0 >> and switch to AREG0 free mode. >> >> Signed-off-by: Blue Swirl <blauwir...@gmail.com> >> --- >> 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