[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: |
Blue Swirl |
Subject: |
Re: [Qemu-devel] [PATCH 2/5] softmmu templates: optionally pass CPUState to memory access functions |
Date: |
Sat, 25 Aug 2012 09:05:38 +0000 |
On Fri, Aug 24, 2012 at 3:05 PM, Aurelien Jarno <address@hidden> 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.
IIRC this was an earlier version, at least regparm() stuff was separated.
>
> 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
> 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.
For 1.2, fixing host support would be most important. It's a good idea
to change the order, but I'd postpone it to 1.3.
>
> Aurelien
>
> --
> Aurelien Jarno GPG: 1024D/F1BCDB73
> address@hidden http://www.aurel32.net
- Re: [Qemu-devel] [PATCH 2/5] softmmu templates: optionally pass CPUState to memory access functions, (continued)
- 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
- 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/25
- Re: [Qemu-devel] [PATCH 2/5] softmmu templates: optionally pass CPUState to memory access functions, Peter Maydell, 2012/08/25
Re: [Qemu-devel] [PATCH 2/5] softmmu templates: optionally pass CPUState to memory access functions,
Blue Swirl <=