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

[Aurelien Jarno]

On Sun, May 15, 2011 at 02:12:27PM +0300, Blue Swirl wrote:
> On Sun, May 15, 2011 at 1:19 PM, Aurelien Jarno <address@hidden> wrote:
> > I still don't get where are this list of possible changes? Did I miss
> > something in another thread?
> 
> I'm referring to the patches I sent.

Ok, patches 1, 2 and 4 to 7 looks ok at a first glance, though I think
patches 6 and 7 should be done for all hosts or none of them.

> > On the TCG generated code, the env structure is used almost for every
> > op, so it really makes sense to keep it in a register instead of having to
> > reload the address of env regularly from memory. Given it only affects
> > TCG generated code, I don't see the point of portability here.
> 
> For example, maybe the bugs in Sparc glibc could be avoided by using
> one of %i set of registers (not accessible from helpers) for AREG0
> within generated code instead of %g registers which seem to be
> fragile.

First of all, but it's a different subject, I am not sure there are
sparc glibc bugs, I'd rather says QEMU mis-uses some register. For
example the following code is probably wrong:

/* Note: must be synced with dyngen-exec.h */
#ifdef CONFIG_SOLARIS
#define TCG_AREG0 TCG_REG_G2
#elif defined(__sparc_v9__)
#define TCG_AREG0 TCG_REG_G5
#else
#define TCG_AREG0 TCG_REG_G6
#endif

__sparc_v9__ can set on the 32-bit ABI, when the compiler targets V8+, 
so the condition is probably wrong there. Secondly the SPARC ABI [1] on
page 23 that %g5 to %g7 are reserved for system. I don't think QEMU has
the right to use this registers.

Anyway, I don't see why keeping TCG_AREG0 inside the TCG generated code
would prevent you to use a register from the %i set for it.

> >> > On the other hand, I have objections to remove uses of TCG_AREG0 from
> >> > the TCG part. This register is part of the TCG design and thus used very
> >> > often. In any case we have to keep it for accesses to the globals, so we
> >> > can keep it for softmmu load/stores too.
> >>
> >> Right, any changes would need to be backed by performance figures.
> >>
> >> > If we go for the removal of
> >> > TCG_AREG0 from the GCC side, it probably means loading it in the prologue
> >> > instead.
> >>
> >> Do you mean cpu-exec.c side with this? I think this is a very similar
> >> register tradeoff case to helpers.
> >
> > I mean calling tcg_qemu_tb_exec(tb_ptr, env) instead of
> > tcg_qemu_tb_exec(tb_ptr), and modification in the prologue to save the
> > env pointer into the TCG_AREG0 register (which is kept internal to TCG
> > generated code).
> 
> Maybe it would be easy to test and benchmark this change, it doesn't
> affect generated code or helpers.
> 

Yes it's something that can be benchmarked. From experience it won't 
make any significant performance change, it's basically two register
moves (one in the caller, one in the prologue), for each TB which is
not chained. In any case, it will introduce a slight overhead that has
to be compensated by allowing the helpers to use an additional 
register.

[1] http://www.sparc.com/standards/psABI3rd.pdf 

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