[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/5] softmmu templates: optionally pass CPUState
From: |
malc |
Subject: |
Re: [Qemu-devel] [PATCH 2/5] softmmu templates: optionally pass CPUState to memory access functions |
Date: |
Fri, 24 Aug 2012 19:33:29 +0400 (MSK) |
User-agent: |
Alpine 2.00 (LNX 1167 2008-08-23) |
On Fri, 24 Aug 2012, Aurelien Jarno wrote:
> On Sun, Mar 11, 2012 at 10:24:03PM +0000, Blue Swirl wrote:
> > Optionally, make memory access helpers take a parameter for CPUState
> > instead of relying on global env.
> >
> > On most targets, perform simple moves to reorder registers. On i386,
> > switch from regparm(3) calling convention to standard stack-based
> > version.
> >
> > Signed-off-by: Blue Swirl <address@hidden>
> > ---
> > cpu-all.h | 9 +++++
> > exec-all.h | 2 +
> > exec.c | 4 ++
> > softmmu_defs.h | 28 ++++++++++++++++
> > softmmu_header.h | 60 ++++++++++++++++++++++++++--------
> > softmmu_template.h | 84
> > ++++++++++++++++++++++++++++++++---------------
> > tcg/arm/tcg-target.c | 53 ++++++++++++++++++++++++++++++
> > tcg/hppa/tcg-target.c | 44 +++++++++++++++++++++++++
> > tcg/i386/tcg-target.c | 57 ++++++++++++++++++++++++++++++++
> > tcg/ia64/tcg-target.c | 46 ++++++++++++++++++++++++++
> > tcg/mips/tcg-target.c | 44 +++++++++++++++++++++++++
> > tcg/ppc/tcg-target.c | 45 +++++++++++++++++++++++++
> > tcg/ppc64/tcg-target.c | 44 +++++++++++++++++++++++++
> > tcg/s390/tcg-target.c | 44 +++++++++++++++++++++++++
> > tcg/sparc/tcg-target.c | 50 +++++++++++++++++++++++++++--
> > tcg/tci/tcg-target.c | 6 +++
> > 16 files changed, 576 insertions(+), 44 deletions(-)
> >
>
> This commit completely broke arm and mips host support, not only for 64
> bit targets as written in the comments, but even for 32 bit targets as
> shifting arguments one by one doesn't work for qemu_st64 which needs 5
> values, while only 4 can be passed in registers.
>
> Moreover even on x86_64, this introduces some performance regressions by
> emitting 4 additional moves in the slow path and adding some more
> constraints on the registers that can be used for passing arguments to
> ld/st ops.
>
> While more and more targets needs AREG0 to be passed, I have started to
> work on fixing that. I came to the conclusion that passing AREG0 as the
> first argument, even if it is look the nice way to do it in C is
> probably not the best option:
> - On 32 bit hosts, which usually need register alignments for 64-bit
> values (at least on arm and mips), given AREG0 is a 32-bit value this
ditto ppc32
> makes the register usage very inefficient when the address or the value
> are 64 bits, in addition to making the code to handle quite complex. It
> would be better to place it close to mem_idx which is also 32 bits.
> - On at least ppc, ppc64, sparc64 and x86_64, this adds some more
> constraints to the ld/st ops arguments.
> - On x86_64 This also means the address loading of the first argument done
> in the TLB function can't be reused easily (it's not a problem right now
> due to registers shifting, but this problem appears when trying to clean
> the code).
> - Finally on all hosts, this make the AREG0 / nonAREG0 load/store
> different, and thus the load/store code much more complex (this is
> something that should disappear when all targets are using the AREG0
> case).
>
> That's why I would propose to move the env argument to the last
> argument. It's better to place it after mem_idx, as it is usually easier
> to store a register on the stack than an immediate value. It also means
> we don't need any register shifting, the code change for most hosts
> would be only a few lines to either copy a value from one register to
> another, or to store a register on the stack, that is without additional
> constraints (there is a call after that so the argument registers are
> already clobbered).
>
> What do you think of that? If that seems the way to go, I can start
> writing patches to do the changes and fix most hosts support.
>
> Aurelien
>
>
--
mailto:address@hidden
- Re: [Qemu-devel] [PATCH 2/5] softmmu templates: optionally pass CPUState to memory access functions, Aurelien Jarno, 2012/08/24
- Re: [Qemu-devel] [PATCH 2/5] softmmu templates: optionally pass CPUState to memory access functions,
malc <=
- Re: [Qemu-devel] [PATCH 2/5] softmmu templates: optionally pass CPUState to memory access functions, malc, 2012/08/24
- Re: [Qemu-devel] [PATCH 2/5] softmmu templates: optionally pass CPUState to memory access functions, Aurelien Jarno, 2012/08/24
- Re: [Qemu-devel] [PATCH 2/5] softmmu templates: optionally pass CPUState to memory access functions, Andreas Färber, 2012/08/24
- Re: [Qemu-devel] [PATCH 2/5] softmmu templates: optionally pass CPUState to memory access functions, Aurelien Jarno, 2012/08/24
- Re: [Qemu-devel] [PATCH 2/5] softmmu templates: optionally pass CPUState to memory access functions, Blue Swirl, 2012/08/25
- Re: [Qemu-devel] [PATCH 2/5] softmmu templates: optionally pass CPUState to memory access functions, Aurelien Jarno, 2012/08/25
- Re: [Qemu-devel] [PATCH 2/5] softmmu templates: optionally pass CPUState to memory access functions, Peter Maydell, 2012/08/24