Re: [Qemu-devel] [PATCH 00/26] AREG0 conversion

[Richard Henderson <rth@twiddle.net>]

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.

>>  Sparc: fix coding style

Reviewed-by: Richard Henderson <rth@twiddle.net>

>>  Sparc: split helper.c

Reviewed-by: Richard Henderson <rth@twiddle.net>

>>  Sparc: move trivial functions from op_helper.c

Reviewed-by: Richard Henderson <rth@twiddle.net>

>>  Sparc: avoid AREG0 for raise_exception and helper_debug

Reviewed-by: Richard Henderson <rth@twiddle.net>

>>  Sparc: fix coding style

Reviewed-by: Richard Henderson <rth@twiddle.net>

>>  Sparc: split FPU and VIS op helpers

Reviewed-by: Richard Henderson <rth@twiddle.net>

>>  Sparc: avoid AREG0 for float and VIS ops

Reviewed-by: Richard Henderson <rth@twiddle.net>

>>  Sparc: split lazy condition code handling op helpers

Reviewed-by: Richard Henderson <rth@twiddle.net>

>>  Sparc: avoid AREG0 for lazy condition code helpers

Reviewed-by: Richard Henderson <rth@twiddle.net>

>>  Sparc: split CWP and PSTATE op helpers

Reviewed-by: Richard Henderson <rth@twiddle.net>

>>  Sparc: avoid AREG0 for CWP and PSTATE helpers

Reviewed-by: Richard Henderson <rth@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.

>>  Sparc: avoid AREG0 for division op helpers

Reviewed-by: Richard Henderson <rth@twiddle.net>

>>  Sparc: fix coding style in helper.c

Reviewed-by: Richard Henderson <rth@twiddle.net>

>>  Sparc: split MMU helpers

Reviewed-by: Richard Henderson <rth@twiddle.net>

>>  Sparc: convert mmu_helper to trace framework

Reviewed-by: Richard Henderson <rth@twiddle.net>

>>  Sparc: convert int_helper to trace framework

Reviewed-by: Richard Henderson <rth@twiddle.net>

>>  Sparc: convert win_helper to trace framework

Reviewed-by: Richard Henderson <rth@twiddle.net>

>>  Sparc: split load and store op helpers

Reviewed-by: Richard Henderson <rth@twiddle.net>

>>  TCG: add 5 arg helpers to def-helper.h

Reviewed-by: Richard Henderson <rth@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.

Reviewed-by: Richard Henderson <rth@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.


r~