[Qemu-devel] [PATCH 0/6] AREG0 patches v4

[Qemu-devel] [PATCH 0/6] AREG0 patches v4

[Blue Swirl]

In this version, I made basic AREG0 free load/store implementations
for all targets. Only x86-64 is tested, others have probably problems,
especially 64 bit guest (Sparc64 in this case) on 32 bit hosts.

I think this should be committed as a starting point if there are no
major objections.

Blue Swirl (6):
  TCG/i386: use iargs or oargs instead of register names
  TCG/sparc: use iargs or oargs instead of register names etc
  TCG: add 5 arg helpers to def-helper.h
  Sparc: avoid AREG0 for memory access helpers
  softmmu templates: optionally pass CPUState to memory access
    functions
  Sparc: avoid AREG0 wrappers for memory access helpers

 Makefile.target            |   12 +-
 configure                  |    7 +
 cpu-all.h                  |    9 +
 def-helper.h               |   26 +++
 exec-all.h                 |    2 +
 exec.c                     |    4 +
 softmmu_defs.h             |   20 ++
 softmmu_header.h           |   60 +++++--
 softmmu_template.h         |   82 ++++++---
 target-sparc/cpu.h         |    3 +-
 target-sparc/helper.h      |   20 +-
 target-sparc/ldst_helper.c |  415 ++++++++++++++++++++++++--------------------
 target-sparc/op_helper.c   |   74 --------
 target-sparc/translate.c   |   62 ++++---
 tcg/arm/tcg-target.c       |   53 ++++++
 tcg/hppa/tcg-target.c      |   44 +++++
 tcg/i386/tcg-target.c      |  187 ++++++++++++++++-----
 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     |   84 +++++++---
 tcg/tci/tcg-target.c       |    6 +
 24 files changed, 987 insertions(+), 406 deletions(-)
 delete mode 100644 target-sparc/op_helper.c

Re: [Qemu-devel] [PATCH 0/6] AREG0 patches v4

[Aurelien Jarno]

On Sat, Jan 07, 2012 at 10:24:09PM +0000, Blue Swirl wrote:
> In this version, I made basic AREG0 free load/store implementations
> for all targets. Only x86-64 is tested, others have probably problems,
> especially 64 bit guest (Sparc64 in this case) on 32 bit hosts.
> 
> I think this should be committed as a starting point if there are no
> major objections.
> 

If it is known to be broken on 32-bit hosts, I am not sure it's really
a good idea to commit these patches. It's just going to prevent all
people developing on x86 to continue testing or developing QEMU.

Also I have to say I still don't get the goal of these patch series. I
have just tried it on an x86-64 guest with a 32-bit sparc guest. It
works well, but the generated TB are slightly bigger. I have done a
quick and dirty performance test by compiling some code inside the
guests, and I get a slow down of about 0.6%, though for this kind of
performance changes, it should probably done with other methods.

Is there another goal behind this patch series beside performance?

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

Re: [Qemu-devel] [PATCH 0/6] AREG0 patches v4

[Blue Swirl]

On Sun, Jan 8, 2012 at 00:52, Aurelien Jarno <aurelien@aurel32.net> wrote:
> On Sat, Jan 07, 2012 at 10:24:09PM +0000, Blue Swirl wrote:
>> In this version, I made basic AREG0 free load/store implementations
>> for all targets. Only x86-64 is tested, others have probably problems,
>> especially 64 bit guest (Sparc64 in this case) on 32 bit hosts.
>>
>> I think this should be committed as a starting point if there are no
>> major objections.
>>
>
> If it is known to be broken on 32-bit hosts, I am not sure it's really
> a good idea to commit these patches. It's just going to prevent all
> people developing on x86 to continue testing or developing QEMU.

No, the brokenness is limited to 32 bit hosts with 64 bit AREG0 free
targets, which currently means Sparc64 on i386, Sparc64 on ARM etc.
Sparc32 should run unless there are other problems. Other targets
which are not AREG0 free (like x86) should be unchanged on any host.

I'm not familiar with various host ABIs, fixing all of the targets is too much.

> Also I have to say I still don't get the goal of these patch series. I
> have just tried it on an x86-64 guest with a 32-bit sparc guest. It
> works well, but the generated TB are slightly bigger. I have done a
> quick and dirty performance test by compiling some code inside the
> guests, and I get a slow down of about 0.6%, though for this kind of
> performance changes, it should probably done with other methods.

Thank you for testing. I also made some tests earlier which were near
that figure but there performance increased. But my test setup adds a
lot of noise (CPU throttling etc.) so I didn't trust the figures much.

The slow down could come from changed compiler flags. IIRC looking at
disassembled functions, stack protector kicks in.

> Is there another goal behind this patch series beside performance?

Yes, we eliminate the global register variable, which has given us a
lot of trouble. Those are not supported by for example LLVM or sparse.
The semantics of calling code which has not been compiled with
register variables from code which has been compiled so is not
defined, but it happens to work most of the time by saving the
variable by hand. On Sparc hosts, global register handling is a mess.

After the change has been made, all QEMU code is just plain C. With
TCI it should even be portable to any host.

When AREG0 is only used internally to TCG generated code, we could
even put the CPUState structure to stack and use stack pointer to
access them, freeing a global variable there too.

> --
> Aurelien Jarno                          GPG: 1024D/F1BCDB73
> aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [PATCH 0/6] AREG0 patches v4

[Aurelien Jarno]

On Sun, Jan 08, 2012 at 12:27:39PM +0000, Blue Swirl wrote:
> On Sun, Jan 8, 2012 at 00:52, Aurelien Jarno <aurelien@aurel32.net> wrote:
> > On Sat, Jan 07, 2012 at 10:24:09PM +0000, Blue Swirl wrote:
> >> In this version, I made basic AREG0 free load/store implementations
> >> for all targets. Only x86-64 is tested, others have probably problems,
> >> especially 64 bit guest (Sparc64 in this case) on 32 bit hosts.
> >>
> >> I think this should be committed as a starting point if there are no
> >> major objections.
> >>
> >
> > If it is known to be broken on 32-bit hosts, I am not sure it's really
> > a good idea to commit these patches. It's just going to prevent all
> > people developing on x86 to continue testing or developing QEMU.
> 
> No, the brokenness is limited to 32 bit hosts with 64 bit AREG0 free
> targets, which currently means Sparc64 on i386, Sparc64 on ARM etc.
> Sparc32 should run unless there are other problems. Other targets
> which are not AREG0 free (like x86) should be unchanged on any host.
> 
> I'm not familiar with various host ABIs, fixing all of the targets is too much.

Yes, but I guess your goal is to have more qemu targets than only
sparc, so it's going to become important at some point. I think it 
should not be committed until we have working x86 and x86-64 host
support.

BTW, right now I have just got a quick overview of the patch series, I
have mostly tested them. I am planning to review them more in details in
the next days.


> > Also I have to say I still don't get the goal of these patch series. I
> > have just tried it on an x86-64 guest with a 32-bit sparc guest. It
> > works well, but the generated TB are slightly bigger. I have done a
> > quick and dirty performance test by compiling some code inside the
> > guests, and I get a slow down of about 0.6%, though for this kind of
> > performance changes, it should probably done with other methods.
> 
> Thank you for testing. I also made some tests earlier which were near
> that figure but there performance increased. But my test setup adds a
> lot of noise (CPU throttling etc.) so I didn't trust the figures much.
> 
> The slow down could come from changed compiler flags. IIRC looking at
> disassembled functions, stack protector kicks in.

Given all the translation blocks are bigger (there is one more mov
instruction per memory load/store and also for some helper), I don't
think it really work it at compiler flags or things like that. The code
to execute is longer, which also has side effects, like not playing very
well wrt instructions cache, or even qemu TB cache.

That said 0.8% is not that big, we can probably accept that, if we have
a good reason for that. And here come the next point...


> > Is there another goal behind this patch series beside performance?
> 
> Yes, we eliminate the global register variable, which has given us a
> lot of trouble. Those are not supported by for example LLVM or sparse.
> The semantics of calling code which has not been compiled with
> register variables from code which has been compiled so is not
> defined, but it happens to work most of the time by saving the
> variable by hand. On Sparc hosts, global register handling is a mess.

Even if I personally don't really have this use case, I understand it's
something that might be important for others. 

My main concern there is how to reach this goal for all hosts and all
targets in a reasonable time frame. I remember the difficult moments 
with both dyngen and TCG, with both softfloat-native and softfloat or 
even now with qdev and non-qdev devices.

That's why I think we should start with at least x86 and x86-64 on the
host side. On the target side, we should have at least two targets
converted, and if possible with different word size and different 
endianness. That way maintainers of the TCG host code can tests all the
possible cases before committing it (experience has shown that it's 
difficult to have all cases right at the first time). MIPS can be a good
candidate, as it can do both 32 and 64 bits and both big and little 
endian.

Ideally at least all hosts should be converted before the next release,
so we don't ship QEMU with support for half of the hosts only.

Do you have any comments or plans about that? I guess it's actually the
critical part in the changes you would like to see.


> After the change has been made, all QEMU code is just plain C. With
> TCI it should even be portable to any host.

Isn't that already the case with TCI? As far as I know TCI doesn't use
host registers, so it should already be fine (beside the doc says it's
not really tested in cross-endianness mode).


> When AREG0 is only used internally to TCG generated code, we could
> even put the CPUState structure to stack and use stack pointer to
> access them, freeing a global variable there too.

That might be a good idea on architectures with very few registers like
i386, not sure it'll have a big impact on others.

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

Re: [Qemu-devel] [PATCH 0/6] AREG0 patches v4

[Blue Swirl]

On Mon, Jan 9, 2012 at 23:04, Aurelien Jarno <aurelien@aurel32.net> wrote:
> On Sun, Jan 08, 2012 at 12:27:39PM +0000, Blue Swirl wrote:
>> On Sun, Jan 8, 2012 at 00:52, Aurelien Jarno <aurelien@aurel32.net> wrote:
>> > On Sat, Jan 07, 2012 at 10:24:09PM +0000, Blue Swirl wrote:
>> >> In this version, I made basic AREG0 free load/store implementations
>> >> for all targets. Only x86-64 is tested, others have probably problems,
>> >> especially 64 bit guest (Sparc64 in this case) on 32 bit hosts.
>> >>
>> >> I think this should be committed as a starting point if there are no
>> >> major objections.
>> >>
>> >
>> > If it is known to be broken on 32-bit hosts, I am not sure it's really
>> > a good idea to commit these patches. It's just going to prevent all
>> > people developing on x86 to continue testing or developing QEMU.
>>
>> No, the brokenness is limited to 32 bit hosts with 64 bit AREG0 free
>> targets, which currently means Sparc64 on i386, Sparc64 on ARM etc.
>> Sparc32 should run unless there are other problems. Other targets
>> which are not AREG0 free (like x86) should be unchanged on any host.
>>
>> I'm not familiar with various host ABIs, fixing all of the targets is too much.
>
> Yes, but I guess your goal is to have more qemu targets than only
> sparc, so it's going to become important at some point. I think it
> should not be committed until we have working x86 and x86-64 host
> support.

OK. The problem with 64 bit guest on i386 host is that arguments need
to be pushed to stack since the registers will not be enough. This
happens even before my patches in some cases, but the code that
handles this is not so clean.

> BTW, right now I have just got a quick overview of the patch series, I
> have mostly tested them. I am planning to review them more in details in
> the next days.
>
>
>> > Also I have to say I still don't get the goal of these patch series. I
>> > have just tried it on an x86-64 guest with a 32-bit sparc guest. It
>> > works well, but the generated TB are slightly bigger. I have done a
>> > quick and dirty performance test by compiling some code inside the
>> > guests, and I get a slow down of about 0.6%, though for this kind of
>> > performance changes, it should probably done with other methods.
>>
>> Thank you for testing. I also made some tests earlier which were near
>> that figure but there performance increased. But my test setup adds a
>> lot of noise (CPU throttling etc.) so I didn't trust the figures much.
>>
>> The slow down could come from changed compiler flags. IIRC looking at
>> disassembled functions, stack protector kicks in.
>
> Given all the translation blocks are bigger (there is one more mov
> instruction per memory load/store and also for some helper), I don't
> think it really work it at compiler flags or things like that. The code
> to execute is longer, which also has side effects, like not playing very
> well wrt instructions cache, or even qemu TB cache.
>
> That said 0.8% is not that big, we can probably accept that, if we have
> a good reason for that. And here come the next point...
>
>
>> > Is there another goal behind this patch series beside performance?
>>
>> Yes, we eliminate the global register variable, which has given us a
>> lot of trouble. Those are not supported by for example LLVM or sparse.
>> The semantics of calling code which has not been compiled with
>> register variables from code which has been compiled so is not
>> defined, but it happens to work most of the time by saving the
>> variable by hand. On Sparc hosts, global register handling is a mess.
>
> Even if I personally don't really have this use case, I understand it's
> something that might be important for others.
>
> My main concern there is how to reach this goal for all hosts and all
> targets in a reasonable time frame. I remember the difficult moments
> with both dyngen and TCG, with both softfloat-native and softfloat or
> even now with qdev and non-qdev devices.
>
> That's why I think we should start with at least x86 and x86-64 on the
> host side. On the target side, we should have at least two targets
> converted, and if possible with different word size and different
> endianness. That way maintainers of the TCG host code can tests all the
> possible cases before committing it (experience has shown that it's
> difficult to have all cases right at the first time). MIPS can be a good
> candidate, as it can do both 32 and 64 bits and both big and little
> endian.
>
> Ideally at least all hosts should be converted before the next release,
> so we don't ship QEMU with support for half of the hosts only.
>
> Do you have any comments or plans about that? I guess it's actually the
> critical part in the changes you would like to see.

I think the MIPS part should work except for 64 on 32, though it is by
no means optimal. If one day all QEMU targets are AREG0 free, the TCG
target maintainers probably want to optimize. It should be easy to
avoid most if not all extra register moves in the TLB slow path.

>> After the change has been made, all QEMU code is just plain C. With
>> TCI it should even be portable to any host.
>
> Isn't that already the case with TCI? As far as I know TCI doesn't use
> host registers, so it should already be fine (beside the doc says it's
> not really tested in cross-endianness mode).

I haven't tested. There could be also GCC specific hacks elsewhere
which are not supported by for example LLVM.

>> When AREG0 is only used internally to TCG generated code, we could
>> even put the CPUState structure to stack and use stack pointer to
>> access them, freeing a global variable there too.
>
> That might be a good idea on architectures with very few registers like
> i386, not sure it'll have a big impact on others.
>
> --
> Aurelien Jarno                          GPG: 1024D/F1BCDB73
> aurelien@aurel32.net                 http://www.aurel32.net