On Wed, Oct 19, 2011 at 17:25, Richard Henderson <r...@twiddle.net> wrote:
> On 10/09/2011 12:20 PM, Blue Swirl wrote:
>>> I didn't bother to attach the patches, if someone wants to try, the
>>> patch set can be found here:
>>> git://repo.or.cz/qemu/blueswirl.git
>>> http://repo.or.cz/r/qemu/blueswirl.git
>>
>> I pushed the patch set to repo.or.cz so if someone wants to comment or
>> test, they are there.
>>
>> It's mostly the same stuff as before, though I split int_helper.c to
>> int32_helper.c and int64_helper.c since they have nothing in common
>> and extracted TCG iargs/oargs changes.
>>
>>> Blue Swirl (26):
>>> Document softmmu templates
>>> softmmu_header: pass CPUState to tlb_fill
>>> Move GETPC from dyngen-exec.h to exec-all.h
>
> I don't see these three in the repo.
Because I applied them earlier...
>>> Sparc: fix coding style
>
> Reviewed-by: Richard Henderson <r...@twiddle.net>
>
>>> Sparc: split helper.c
>
> Reviewed-by: Richard Henderson <r...@twiddle.net>
>
>>> Sparc: move trivial functions from op_helper.c
>
> Reviewed-by: Richard Henderson <r...@twiddle.net>
>
>>> Sparc: avoid AREG0 for raise_exception and helper_debug
>
> Reviewed-by: Richard Henderson <r...@twiddle.net>
>
>>> Sparc: fix coding style
>
> Reviewed-by: Richard Henderson <r...@twiddle.net>
>
>>> Sparc: split FPU and VIS op helpers
>
> Reviewed-by: Richard Henderson <r...@twiddle.net>
>
>>> Sparc: avoid AREG0 for float and VIS ops
>
> Reviewed-by: Richard Henderson <r...@twiddle.net>
>
>>> Sparc: split lazy condition code handling op helpers
>
> Reviewed-by: Richard Henderson <r...@twiddle.net>
>
>>> Sparc: avoid AREG0 for lazy condition code helpers
>
> Reviewed-by: Richard Henderson <r...@twiddle.net>
>
>>> Sparc: split CWP and PSTATE op helpers
>
> Reviewed-by: Richard Henderson <r...@twiddle.net>
>
>>> Sparc: avoid AREG0 for CWP and PSTATE helpers
>
> Reviewed-by: Richard Henderson <r...@twiddle.net>
> An especially nice cleanup too, avoiding all of the saved_env frobbing.
>
>>> Sparc: avoid AREG0 for softint op helpers and Leon cache control
>
> This one loses do_modify_softint in the move. Which gets re-instated
> in your following "convert int_helper to trace framework" patch. But
> in the meantime the series is non-bisectable.
Nice catch, will fix. I think I'll apply the patches before this one
now to shorten the series a bit.
>>> Sparc: avoid AREG0 for division op helpers
>
> Reviewed-by: Richard Henderson <r...@twiddle.net>
>
>>> Sparc: fix coding style in helper.c
>
> Reviewed-by: Richard Henderson <r...@twiddle.net>
>
>>> Sparc: split MMU helpers
>
> Reviewed-by: Richard Henderson <r...@twiddle.net>
>
>>> Sparc: convert mmu_helper to trace framework
>
> Reviewed-by: Richard Henderson <r...@twiddle.net>
>
>>> Sparc: convert int_helper to trace framework
>
> Reviewed-by: Richard Henderson <r...@twiddle.net>
>
>>> Sparc: convert win_helper to trace framework
>
> Reviewed-by: Richard Henderson <r...@twiddle.net>
>
>>> Sparc: split load and store op helpers
>
> Reviewed-by: Richard Henderson <r...@twiddle.net>
>
>>> TCG: add 5 arg helpers to def-helper.h
>
> Reviewed-by: Richard Henderson <r...@twiddle.net>
>
>>> Sparc: avoid AREG0 for memory access helpers
>
>> +#define WRAP_LD(rettype, fn) \
>> + rettype cpu_ ## fn (CPUState *env1, target_ulong addr) \
>> + { \
>> + CPUState *saved_env; \
>> + rettype ret; \
>> + \
>> + saved_env = env; \
>> + env = env1; \
>> + ret = fn(addr); \
>> + env = saved_env; \
>> + return ret; \
>> + }
>
> I don't think this actually works in the fault case. In particular GETPC
> will return an incorrect address. OTOH, I suppose we already have this
> problem from the other ldst helpers, e.g. ld_asi, which is where these new
> routines are going to be called from. So this doesn't really change the
> state of affairs much. I have no good ideas for solving this problem.
OK, maybe it's better to leave this and the 5 arg patch from 1.0.
> Reviewed-by: Richard Henderson <r...@twiddle.net>
>
>>> softmmu templates: optionally pass CPUState to memory access
>>> functions
>>> Sparc: avoid AREG0 wrappers for memory access helpers
>
> Both look ok. I'm certainly not fond of the intermediate state. If we
> convert target-sparc and tcg-i386, we should convert all of them, and
> not leave that intermediate state for long.
>
> Something that's clearly not going to happen for the 1.0 release.
Fully agree. Thanks for the review.
