All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 00/26] AREG0 conversion
@ 2011-09-24 18:14 Blue Swirl
  2011-10-09 19:20 ` Blue Swirl
  0 siblings, 1 reply; 4+ messages in thread
From: Blue Swirl @ 2011-09-24 18:14 UTC (permalink / raw)
  To: qemu-devel

In this version, target-sparc/op_helper.c is completely eliminated
after the last commit!

For some reason, sparc-softmmu crashes after first qemu_st op and it
does not compile on non-x86. Other targets still seem to work, as does
sparc-softmmu until the last patch. I haven't tested i386 host either,
5 arg helpers could trigger some new bugs and the i386 TCG part is
bogus.

Patches 1 to 3 should be applied, for others it should be nice to get
the last patch working. Any review would be helpful.

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

Blue Swirl (26):
  Document softmmu templates
  softmmu_header: pass CPUState to tlb_fill
  Move GETPC from dyngen-exec.h to exec-all.h
  Sparc: fix coding style
  Sparc: split helper.c
  Sparc: move trivial functions from op_helper.c
  Sparc: avoid AREG0 for raise_exception and helper_debug
  Sparc: fix coding style
  Sparc: split FPU and VIS op helpers
  Sparc: avoid AREG0 for float and VIS ops
  Sparc: split lazy condition code handling op helpers
  Sparc: avoid AREG0 for lazy condition code helpers
  Sparc: split CWP and PSTATE op helpers
  Sparc: avoid AREG0 for CWP and PSTATE helpers
  Sparc: avoid AREG0 for softint op helpers and Leon cache control
  Sparc: avoid AREG0 for division op helpers
  Sparc: fix coding style in helper.c
  Sparc: split MMU helpers
  Sparc: convert mmu_helper to trace framework
  Sparc: convert int_helper to trace framework
  Sparc: convert win_helper to trace framework
  Sparc: split load and store op helpers
  TCG: add 5 arg helpers to def-helper.h
  Sparc: avoid AREG0 for memory access helpers
  softmmu templates: optionally pass CPUState to memory access
    functions
  Sparc: avoid AREG0 wrappers for memory access helpers

 Makefile.target               |   17 +-
 configure                     |    7 +
 cpu-all.h                     |    9 +
 def-helper.h                  |   26 +
 dyngen-exec.h                 |   12 -
 exec-all.h                    |   20 +-
 exec.c                        |    1 +
 softmmu_defs.h                |   28 +
 softmmu_exec.h                |   12 +-
 softmmu_header.h              |   69 +-
 softmmu_template.h            |   95 +-
 target-alpha/op_helper.c      |    7 +-
 target-arm/op_helper.c        |    6 +-
 target-cris/op_helper.c       |    7 +-
 target-i386/op_helper.c       |    7 +-
 target-lm32/op_helper.c       |    7 +-
 target-m68k/op_helper.c       |    7 +-
 target-microblaze/op_helper.c |    7 +-
 target-mips/op_helper.c       |    7 +-
 target-ppc/op_helper.c        |    7 +-
 target-s390x/op_helper.c      |    7 +-
 target-sh4/op_helper.c        |    7 +-
 target-sparc/cc_helper.c      |  485 +++++
 target-sparc/cpu.h            |   39 +-
 target-sparc/cpu_init.c       |  848 ++++++++
 target-sparc/fop_helper.c     |  394 ++++
 target-sparc/helper.c         | 1929 +------------------
 target-sparc/helper.h         |  250 ++--
 target-sparc/int_helper.c     |  345 ++++
 target-sparc/ldst_helper.c    | 2477 +++++++++++++++++++++++
 target-sparc/mmu_helper.c     |  853 ++++++++
 target-sparc/op_helper.c      | 4365 -----------------------------------------
 target-sparc/translate.c      |  432 +++--
 target-sparc/vis_helper.c     |  406 ++++
 target-sparc/win_helper.c     |  393 ++++
 target-xtensa/op_helper.c     |    5 +-
 tcg/i386/tcg-target.c         |   85 +-
 trace-events                  |   25 +
 38 files changed, 7009 insertions(+), 6694 deletions(-)
 create mode 100644 target-sparc/cc_helper.c
 create mode 100644 target-sparc/cpu_init.c
 create mode 100644 target-sparc/fop_helper.c
 create mode 100644 target-sparc/int_helper.c
 create mode 100644 target-sparc/ldst_helper.c
 create mode 100644 target-sparc/mmu_helper.c
 delete mode 100644 target-sparc/op_helper.c
 create mode 100644 target-sparc/vis_helper.c
 create mode 100644 target-sparc/win_helper.c

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH 00/26] AREG0 conversion
  2011-09-24 18:14 [Qemu-devel] [PATCH 00/26] AREG0 conversion Blue Swirl
@ 2011-10-09 19:20 ` Blue Swirl
  2011-10-19 17:25   ` Richard Henderson
  0 siblings, 1 reply; 4+ messages in thread
From: Blue Swirl @ 2011-10-09 19:20 UTC (permalink / raw)
  To: qemu-devel

On Sat, Sep 24, 2011 at 6:14 PM, Blue Swirl <blauwirbel@gmail.com> wrote:
> In this version, target-sparc/op_helper.c is completely eliminated
> after the last commit!
>
> For some reason, sparc-softmmu crashes after first qemu_st op and it
> does not compile on non-x86. Other targets still seem to work, as does
> sparc-softmmu until the last patch. I haven't tested i386 host either,
> 5 arg helpers could trigger some new bugs and the i386 TCG part is
> bogus.

Now Sparc targets work also after the final commit after I fixed bugs
with TCG patches.

I was able to make some performance tests with Byte nbench, but
unfortunately the variance between each run is greater (1.5%) than
variance between patched and unpatched versions (0.6%). Probably the
speed of the host CPU varies or something.

For some reason, this breaks build on i386 or mingw32. It's not
possible to compile any Sparc targets until other TCG back ends are
converted, now only amd64 works.

> Patches 1 to 3 should be applied, for others it should be nice to get
> the last patch working. Any review would be helpful.
>
> 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
>  Sparc: fix coding style
>  Sparc: split helper.c
>  Sparc: move trivial functions from op_helper.c
>  Sparc: avoid AREG0 for raise_exception and helper_debug
>  Sparc: fix coding style
>  Sparc: split FPU and VIS op helpers
>  Sparc: avoid AREG0 for float and VIS ops
>  Sparc: split lazy condition code handling op helpers
>  Sparc: avoid AREG0 for lazy condition code helpers
>  Sparc: split CWP and PSTATE op helpers
>  Sparc: avoid AREG0 for CWP and PSTATE helpers
>  Sparc: avoid AREG0 for softint op helpers and Leon cache control
>  Sparc: avoid AREG0 for division op helpers
>  Sparc: fix coding style in helper.c
>  Sparc: split MMU helpers
>  Sparc: convert mmu_helper to trace framework
>  Sparc: convert int_helper to trace framework
>  Sparc: convert win_helper to trace framework
>  Sparc: split load and store op helpers
>  TCG: add 5 arg helpers to def-helper.h
>  Sparc: avoid AREG0 for memory access helpers
>  softmmu templates: optionally pass CPUState to memory access
>    functions
>  Sparc: avoid AREG0 wrappers for memory access helpers
>
>  Makefile.target               |   17 +-
>  configure                     |    7 +
>  cpu-all.h                     |    9 +
>  def-helper.h                  |   26 +
>  dyngen-exec.h                 |   12 -
>  exec-all.h                    |   20 +-
>  exec.c                        |    1 +
>  softmmu_defs.h                |   28 +
>  softmmu_exec.h                |   12 +-
>  softmmu_header.h              |   69 +-
>  softmmu_template.h            |   95 +-
>  target-alpha/op_helper.c      |    7 +-
>  target-arm/op_helper.c        |    6 +-
>  target-cris/op_helper.c       |    7 +-
>  target-i386/op_helper.c       |    7 +-
>  target-lm32/op_helper.c       |    7 +-
>  target-m68k/op_helper.c       |    7 +-
>  target-microblaze/op_helper.c |    7 +-
>  target-mips/op_helper.c       |    7 +-
>  target-ppc/op_helper.c        |    7 +-
>  target-s390x/op_helper.c      |    7 +-
>  target-sh4/op_helper.c        |    7 +-
>  target-sparc/cc_helper.c      |  485 +++++
>  target-sparc/cpu.h            |   39 +-
>  target-sparc/cpu_init.c       |  848 ++++++++
>  target-sparc/fop_helper.c     |  394 ++++
>  target-sparc/helper.c         | 1929 +------------------
>  target-sparc/helper.h         |  250 ++--
>  target-sparc/int_helper.c     |  345 ++++
>  target-sparc/ldst_helper.c    | 2477 +++++++++++++++++++++++
>  target-sparc/mmu_helper.c     |  853 ++++++++
>  target-sparc/op_helper.c      | 4365 -----------------------------------------
>  target-sparc/translate.c      |  432 +++--
>  target-sparc/vis_helper.c     |  406 ++++
>  target-sparc/win_helper.c     |  393 ++++
>  target-xtensa/op_helper.c     |    5 +-
>  tcg/i386/tcg-target.c         |   85 +-
>  trace-events                  |   25 +
>  38 files changed, 7009 insertions(+), 6694 deletions(-)
>  create mode 100644 target-sparc/cc_helper.c
>  create mode 100644 target-sparc/cpu_init.c
>  create mode 100644 target-sparc/fop_helper.c
>  create mode 100644 target-sparc/int_helper.c
>  create mode 100644 target-sparc/ldst_helper.c
>  create mode 100644 target-sparc/mmu_helper.c
>  delete mode 100644 target-sparc/op_helper.c
>  create mode 100644 target-sparc/vis_helper.c
>  create mode 100644 target-sparc/win_helper.c
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH 00/26] AREG0 conversion
  2011-10-09 19:20 ` Blue Swirl
@ 2011-10-19 17:25   ` Richard Henderson
  2011-10-23 12:14     ` Blue Swirl
  0 siblings, 1 reply; 4+ messages in thread
From: Richard Henderson @ 2011-10-19 17:25 UTC (permalink / raw)
  To: Blue Swirl; +Cc: qemu-devel

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~

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH 00/26] AREG0 conversion
  2011-10-19 17:25   ` Richard Henderson
@ 2011-10-23 12:14     ` Blue Swirl
  0 siblings, 0 replies; 4+ messages in thread
From: Blue Swirl @ 2011-10-23 12:14 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-devel

On Wed, Oct 19, 2011 at 17:25, Richard Henderson <rth@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 <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.

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 <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.

OK, maybe it's better to leave this and the 5 arg patch from 1.0.

> 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.

Fully agree. Thanks for the review.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2011-10-23 12:14 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-09-24 18:14 [Qemu-devel] [PATCH 00/26] AREG0 conversion Blue Swirl
2011-10-09 19:20 ` Blue Swirl
2011-10-19 17:25   ` Richard Henderson
2011-10-23 12:14     ` Blue Swirl

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.