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 <blauwir...@gmail.com> > --- > 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 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 -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
