Re: [Qemu-devel] [PATCH RFC 00/11] AREG0 elimination

[Aurelien Jarno]

On Sat, May 14, 2011 at 11:16:16PM +0200, Aurelien Jarno wrote:
> On Sat, May 14, 2011 at 10:35:20PM +0300, Blue Swirl wrote:
> > Here's a RFC series for eliminating AREG0.
> > 
> > Blue Swirl (11):
> >   Move user emulator stuff from cpu-exec.c to user-exec.c
> >   Delete unused tb_invalidate_page_range
> > 
> > The above should be OK to commit.
> > 
> >   cpu_loop_exit: avoid using AREG0
> >   Delegate setup of TCG temporaries to targets
> > 
> > These two are not, unless the overall plan is OK.
> > 
> >   TCG: fix negative frame offset calculations
> >   TCG/x86: use stack for TCG temps
> >   TCG/Sparc64: use stack for TCG temps
> > 
> > But these three should be OK. I've tested lightly x86_64 and Sparc64 hosts.
> > 
> >   Add CONFIG_TARGET_NEEDS_AREG0
> >   Don't compile legacy qemu_ld/st functions if target doesn't need them
> > 
> > Should be OK, though the latter patch only touches x86.
> > 
> >   Add new qemu_ld and qemu_st functions
> >   sparc: use new qemu_ld and qemu_st functions
> > 
> > The last two compile but QEMU segfaults. I just made a naive
> > conversion for getting comments.
> > 
> 
> What is the goal behing removing TCG_AREG0? If it is speed improvement,
> can you please provide some benchmarks?
> 
> The env register is used very often (basically for every load/store, but
> also a lot of helpers), so it makes sense to reserve a register for it.
> 
> For what I understand from your patch series, you prefer to pass this
> register explicitly to TCG functions. This basically means this TCG
> global will be loaded to host register as soon as it is used, but also
> regularly, as globals are saved back to their canonical location before
> an helper or a load/store.
> 
> So it seems that this patch series will just allowing the "env register"
> to change over time, though it will not spare one more register for the 
> TCG code, and it will emit longer TCG code to regularly reload the env
> global into a host register.
> 

One way to solve that would be to use a env register only at the TCG
level, but not at the GCC level. That means loading the value of env
into TCG_AREG0 in the prologue.

That way, all the TCG code will have direct access to env, and the GCC
generated code (which includes the helper) don't need to have this
register reserved. I doubt the latter will increase the performance, but
I understand the cleanliness argument.

This also means minimal code changes, especially as most (if not all) 
TCG targets seems to have TCG_AREG0 as a callee saved arguments.
Basically the path to do the changes could be:
- Make sure all targets have TCG_AREG0 as a callee saved register
- Load env into TCG_AREG0 (and rename it as a TCG_ENV?) in the prologue 
  (could be by passing it as argument to the prologue code)
- Change all helpers to not use env directly
- Change softmmu code to not use env directly
- Remove GCC hack to reserve a register for env.

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
address@hidden                 http://www.aurel32.net