[Qemu-devel] [PATCH v3 00/29] tcg-aarch64 improvements

[Qemu-devel] [PATCH v3 00/29] tcg-aarch64 improvements

[Richard Henderson]

I'm not sure if I posted v2 or not, but my branch is named -3,
therefore this is v3.  ;-)

The jumbo "fixme" patch from v1 has been split up.  This has been
updated for the changes in the tlb helpers over the past few weeks.
For the benefit of trivial conflict resolution, it's relative to a
tree that contains basically all of my patches.

See git://github.com/rth7680/qemu.git tcg-aarch-3 for the tree, if
you find yourself missing any of the dependencies.


r~


Richard Henderson (29):
  tcg-aarch64: Set ext based on TCG_OPF_64BIT
  tcg-aarch64: Change all ext variables to bool
  tcg-aarch64: Don't handle mov/movi in tcg_out_op
  tcg-aarch64: Hoist common argument loads in tcg_out_op
  tcg-aarch64: Change enum aarch64_arith_opc to AArch64Insn
  tcg-aarch64: Merge enum aarch64_srr_opc with AArch64Insn
  tcg-aarch64: Introduce tcg_fmt_* functions
  tcg-aarch64: Introduce tcg_fmt_Rdn_aimm
  tcg-aarch64: Implement mov with tcg_fmt_* functions
  tcg-aarch64: Handle constant operands to add, sub, and compare
  tcg-aarch64: Handle constant operands to and, or, xor
  tcg-aarch64: Support andc, orc, eqv, not
  tcg-aarch64: Handle zero as first argument to sub
  tcg-aarch64: Support movcond
  tcg-aarch64: Support deposit
  tcg-aarch64: Support add2, sub2
  tcg-aarch64: Support muluh, mulsh
  tcg-aarch64: Support div, rem
  tcg-aarch64: Introduce tcg_fmt_Rd_uimm_s
  tcg-aarch64: Improve tcg_out_movi
  tcg-aarch64: Avoid add with zero in tlb load
  tcg-aarch64: Use adrp in tcg_out_movi
  tcg-aarch64: Pass return address to load/store helpers directly.
  tcg-aarch64: Use tcg_out_call for qemu_ld/st
  tcg-aarch64: Use symbolic names for branches
  tcg-aarch64: Implement tcg_register_jit
  tcg-aarch64: Reuse FP and LR in translated code
  tcg-aarch64: Introduce tcg_out_ldst_pair
  tcg-aarch64: Remove redundant CPU_TLB_ENTRY_BITS check

 include/exec/exec-all.h  |   18 -
 tcg/aarch64/tcg-target.c | 1276 ++++++++++++++++++++++++++++++----------------
 tcg/aarch64/tcg-target.h |   76 +--
 3 files changed, 867 insertions(+), 503 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH v3 00/29] tcg-aarch64 improvements

[Richard Henderson]

I'm not sure if I posted v2 or not, but my branch is named -3,
therefore this is v3.  ;-)

The jumbo "fixme" patch from v1 has been split up.  This has been
updated for the changes in the tlb helpers over the past few weeks.
For the benefit of trivial conflict resolution, it's relative to a
tree that contains basically all of my patches.

See git://github.com/rth7680/qemu.git tcg-aarch-3 for the tree, if
you find yourself missing any of the dependencies.


r~


Richard Henderson (29):
  tcg-aarch64: Set ext based on TCG_OPF_64BIT
  tcg-aarch64: Change all ext variables to bool
  tcg-aarch64: Don't handle mov/movi in tcg_out_op
  tcg-aarch64: Hoist common argument loads in tcg_out_op
  tcg-aarch64: Change enum aarch64_arith_opc to AArch64Insn
  tcg-aarch64: Merge enum aarch64_srr_opc with AArch64Insn
  tcg-aarch64: Introduce tcg_fmt_* functions
  tcg-aarch64: Introduce tcg_fmt_Rdn_aimm
  tcg-aarch64: Implement mov with tcg_fmt_* functions
  tcg-aarch64: Handle constant operands to add, sub, and compare
  tcg-aarch64: Handle constant operands to and, or, xor
  tcg-aarch64: Support andc, orc, eqv, not
  tcg-aarch64: Handle zero as first argument to sub
  tcg-aarch64: Support movcond
  tcg-aarch64: Support deposit
  tcg-aarch64: Support add2, sub2
  tcg-aarch64: Support muluh, mulsh
  tcg-aarch64: Support div, rem
  tcg-aarch64: Introduce tcg_fmt_Rd_uimm_s
  tcg-aarch64: Improve tcg_out_movi
  tcg-aarch64: Avoid add with zero in tlb load
  tcg-aarch64: Use adrp in tcg_out_movi
  tcg-aarch64: Pass return address to load/store helpers directly.
  tcg-aarch64: Use tcg_out_call for qemu_ld/st
  tcg-aarch64: Use symbolic names for branches
  tcg-aarch64: Implement tcg_register_jit
  tcg-aarch64: Reuse FP and LR in translated code
  tcg-aarch64: Introduce tcg_out_ldst_pair
  tcg-aarch64: Remove redundant CPU_TLB_ENTRY_BITS check

 include/exec/exec-all.h  |   18 -
 tcg/aarch64/tcg-target.c | 1276 ++++++++++++++++++++++++++++++----------------
 tcg/aarch64/tcg-target.h |   76 +--
 3 files changed, 867 insertions(+), 503 deletions(-)

-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v3 00/29] tcg-aarch64 improvements

[Richard W.M. Jones]

On Mon, Sep 02, 2013 at 10:54:34AM -0700, Richard Henderson wrote:
> I'm not sure if I posted v2 or not, but my branch is named -3,
> therefore this is v3.  ;-)
> 
> The jumbo "fixme" patch from v1 has been split up.  This has been
> updated for the changes in the tlb helpers over the past few weeks.
> For the benefit of trivial conflict resolution, it's relative to a
> tree that contains basically all of my patches.
> 
> See git://github.com/rth7680/qemu.git tcg-aarch-3 for the tree, if
> you find yourself missing any of the dependencies.

Is there a way yet to compile and run a 'qemu-system-aarch64'? [on a
regular x86-64 host]

I tried your git branch above and Peter's v5 patch posted a while back
(which doesn't cleanly apply), but I don't seem to have the right
combination of bits to make a working binary.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/

Re: [Qemu-devel] [PATCH v3 00/29] tcg-aarch64 improvements

[Laurent Desnogues]

On Tue, Sep 3, 2013 at 9:37 AM, Richard W.M. Jones <rjones@redhat.com> wrote:
> On Mon, Sep 02, 2013 at 10:54:34AM -0700, Richard Henderson wrote:
>> I'm not sure if I posted v2 or not, but my branch is named -3,
>> therefore this is v3.  ;-)
>>
>> The jumbo "fixme" patch from v1 has been split up.  This has been
>> updated for the changes in the tlb helpers over the past few weeks.
>> For the benefit of trivial conflict resolution, it's relative to a
>> tree that contains basically all of my patches.
>>
>> See git://github.com/rth7680/qemu.git tcg-aarch-3 for the tree, if
>> you find yourself missing any of the dependencies.
>
> Is there a way yet to compile and run a 'qemu-system-aarch64'? [on a
> regular x86-64 host]

The current public work is only to run QEMU on Aarch64 host, not
Aarch64 on other hosts ;-)

> I tried your git branch above and Peter's v5 patch posted a while back
> (which doesn't cleanly apply), but I don't seem to have the right
> combination of bits to make a working binary.

You'll need a cross-compiler or ARM foundation model.


Laurent

Re: [Qemu-devel] [PATCH v3 00/29] tcg-aarch64 improvements

[Peter Maydell]

On 3 September 2013 08:37, Richard W.M. Jones <rjones@redhat.com> wrote:
> Is there a way yet to compile and run a 'qemu-system-aarch64'? [on a
> regular x86-64 host]

The code for this has not yet been written :-)
The patchset I posted will build a qemu-system-aarch64 but
with no actual 64 bit CPUs (you can run all the 32 bit CPUs
if you like). It's foundational work for doing the system emulation
on and also for the linux-user 64 bit emulation which Alex is doing.

As Laurent says, don't confuse this with the tcg-aarch64 code
in tree, which is for emulating MIPS/x86/etc on aarch64 hosts.

> I tried your git branch above and Peter's v5 patch posted a while back
> (which doesn't cleanly apply)

Try the git branch I mention in the cover letter (or its followup),
which I've been rebasing. Or you could wait a day or two for v6.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 00/29] tcg-aarch64 improvements

[Claudio Fontana]

Hello Richard,

On 02.09.2013 19:54, Richard Henderson wrote:
> I'm not sure if I posted v2 or not, but my branch is named -3,
> therefore this is v3.  ;-)
> 
> The jumbo "fixme" patch from v1 has been split up.  This has been
> updated for the changes in the tlb helpers over the past few weeks.
> For the benefit of trivial conflict resolution, it's relative to a
> tree that contains basically all of my patches.
> 
> See git://github.com/rth7680/qemu.git tcg-aarch-3 for the tree, if
> you find yourself missing any of the dependencies.
> 
> 
> r~
> Richard Henderson (29):
>   tcg-aarch64: Set ext based on TCG_OPF_64BIT
>   tcg-aarch64: Change all ext variables to bool
>   tcg-aarch64: Don't handle mov/movi in tcg_out_op
>   tcg-aarch64: Hoist common argument loads in tcg_out_op
>   tcg-aarch64: Change enum aarch64_arith_opc to AArch64Insn
>   tcg-aarch64: Merge enum aarch64_srr_opc with AArch64Insn
>   tcg-aarch64: Introduce tcg_fmt_* functions
>   tcg-aarch64: Introduce tcg_fmt_Rdn_aimm
>   tcg-aarch64: Implement mov with tcg_fmt_* functions
>   tcg-aarch64: Handle constant operands to add, sub, and compare
>   tcg-aarch64: Handle constant operands to and, or, xor
>   tcg-aarch64: Support andc, orc, eqv, not
>   tcg-aarch64: Handle zero as first argument to sub
>   tcg-aarch64: Support movcond
>   tcg-aarch64: Support deposit
>   tcg-aarch64: Support add2, sub2
>   tcg-aarch64: Support muluh, mulsh
>   tcg-aarch64: Support div, rem
>   tcg-aarch64: Introduce tcg_fmt_Rd_uimm_s
>   tcg-aarch64: Improve tcg_out_movi
>   tcg-aarch64: Avoid add with zero in tlb load
>   tcg-aarch64: Use adrp in tcg_out_movi
>   tcg-aarch64: Pass return address to load/store helpers directly.
>   tcg-aarch64: Use tcg_out_call for qemu_ld/st
>   tcg-aarch64: Use symbolic names for branches
>   tcg-aarch64: Implement tcg_register_jit
>   tcg-aarch64: Reuse FP and LR in translated code
>   tcg-aarch64: Introduce tcg_out_ldst_pair
>   tcg-aarch64: Remove redundant CPU_TLB_ENTRY_BITS check
> 
>  include/exec/exec-all.h  |   18 -
>  tcg/aarch64/tcg-target.c | 1276 ++++++++++++++++++++++++++++++----------------
>  tcg/aarch64/tcg-target.h |   76 +--
>  3 files changed, 867 insertions(+), 503 deletions(-)
> 

after carefully reading and testing your patches, this is how I suggest to proceed: 

first do the implementation of the new functionality (tcg opcodes, jit) in a way that is consistent with the existing code.
No type changes, no refactoring, no beautification.

Once we agree on those, introduce the meaningful restructuring you want to do,
like the new INSN type, the "don't handle mov/movi in tcg_out_op", the TCG_OPF_64BIT thing, etc.

Last do the cosmetic stuff if you really want to do it, like the change all ext to bool (note that there is no point if the callers still use "1" and "0": adapt them as well) etc.

Right now the patchset is difficult to digest, given the regressions it introduces coupled with a mixing of functional changes, restructuring and cosmetics.

I think this will allow us to proceed quicker towards agreement.

Thanks,

Claudio

Re: [Qemu-devel] [PATCH v3 00/29] tcg-aarch64 improvements

[Richard Henderson]

On 09/09/2013 01:13 AM, Claudio Fontana wrote:
> after carefully reading and testing your patches, this is how I suggest to proceed: 
> 
> first do the implementation of the new functionality (tcg opcodes, jit) in a way that is consistent with the existing code.
> No type changes, no refactoring, no beautification.
> 
> Once we agree on those, introduce the meaningful restructuring you want to do,
> like the new INSN type, the "don't handle mov/movi in tcg_out_op", the TCG_OPF_64BIT thing, etc.
> 
> Last do the cosmetic stuff if you really want to do it, like the change all ext to bool (note that there is no point if the callers still use "1" and "0": adapt them as well) etc.

No, I don't agree.  Especially with respect to the insn type.

I'd much rather do all the "cosmetic stuff", as you put it, first.  It makes
all of the "real" changes much easier to understand.


r~

Re: [Qemu-devel] [PATCH v3 00/29] tcg-aarch64 improvements

[Claudio Fontana]

On 09.09.2013 16:08, Richard Henderson wrote:
> On 09/09/2013 01:13 AM, Claudio Fontana wrote:
>> after carefully reading and testing your patches, this is how I suggest to proceed: 
>>
>> first do the implementation of the new functionality (tcg opcodes, jit) in a way that is consistent with the existing code.
>> No type changes, no refactoring, no beautification.
>>
>> Once we agree on those, introduce the meaningful restructuring you want to do,
>> like the new INSN type, the "don't handle mov/movi in tcg_out_op", the TCG_OPF_64BIT thing, etc.
>>
>> Last do the cosmetic stuff if you really want to do it, like the change all ext to bool (note that there is no point if the callers still use "1" and "0": adapt them as well) etc.
> 
> No, I don't agree.  Especially with respect to the insn type.
> 
> I'd much rather do all the "cosmetic stuff", as you put it, first.  It makes
> all of the "real" changes much easier to understand.
> 
> 
> r~
> 

I guess we are stuck then. With the cosmetic and restructuring stuff coming before, I cannot cherry pick the good parts later.

Re: [Qemu-devel] [PATCH v3 00/29] tcg-aarch64 improvements

[Peter Maydell]

On 9 September 2013 16:02, Claudio Fontana <claudio.fontana@huawei.com> wrote:
> I guess we are stuck then. With the cosmetic and restructuring
> stuff coming before, I cannot cherry pick the good parts later.

...what do you need to cherry pick it into?

-- PMM

Re: [Qemu-devel] [PATCH v3 00/29] tcg-aarch64 improvements

[Richard Henderson]

On 09/09/2013 08:02 AM, Claudio Fontana wrote:
> On 09.09.2013 16:08, Richard Henderson wrote:
>> On 09/09/2013 01:13 AM, Claudio Fontana wrote:
>>> after carefully reading and testing your patches, this is how I suggest to proceed: 
>>>
>>> first do the implementation of the new functionality (tcg opcodes, jit) in a way that is consistent with the existing code.
>>> No type changes, no refactoring, no beautification.
>>>
>>> Once we agree on those, introduce the meaningful restructuring you want to do,
>>> like the new INSN type, the "don't handle mov/movi in tcg_out_op", the TCG_OPF_64BIT thing, etc.
>>>
>>> Last do the cosmetic stuff if you really want to do it, like the change all ext to bool (note that there is no point if the callers still use "1" and "0": adapt them as well) etc.
>>
>> No, I don't agree.  Especially with respect to the insn type.
>>
>> I'd much rather do all the "cosmetic stuff", as you put it, first.  It makes
>> all of the "real" changes much easier to understand.
>>
>>
>> r~
>>
> 
> I guess we are stuck then. With the cosmetic and restructuring stuff coming before, I cannot cherry pick the good parts later.
> 
> 

Have you tested the first 9 patches on their own?  I.e.

>   tcg-aarch64: Set ext based on TCG_OPF_64BIT
>   tcg-aarch64: Change all ext variables to bool
>   tcg-aarch64: Don't handle mov/movi in tcg_out_op
>   tcg-aarch64: Hoist common argument loads in tcg_out_op
>   tcg-aarch64: Change enum aarch64_arith_opc to AArch64Insn
>   tcg-aarch64: Merge enum aarch64_srr_opc with AArch64Insn
>   tcg-aarch64: Introduce tcg_fmt_* functions
>   tcg-aarch64: Introduce tcg_fmt_Rdn_aimm
>   tcg-aarch64: Implement mov with tcg_fmt_* functions

There should be no functional change to the backend, producing 100% identical
output code.  There should even be little or no change in tcg.o itself.

This should make it trivial to verify that these patches are not at fault.



r~

Re: [Qemu-devel] [PATCH v3 00/29] tcg-aarch64 improvements

[Claudio Fontana]

On 09.09.2013 17:07, Richard Henderson wrote:
> On 09/09/2013 08:02 AM, Claudio Fontana wrote:
>> On 09.09.2013 16:08, Richard Henderson wrote:
>>> On 09/09/2013 01:13 AM, Claudio Fontana wrote:
>>>> after carefully reading and testing your patches, this is how I suggest to proceed: 
>>>>
>>>> first do the implementation of the new functionality (tcg opcodes, jit) in a way that is consistent with the existing code.
>>>> No type changes, no refactoring, no beautification.
>>>>
>>>> Once we agree on those, introduce the meaningful restructuring you want to do,
>>>> like the new INSN type, the "don't handle mov/movi in tcg_out_op", the TCG_OPF_64BIT thing, etc.
>>>>
>>>> Last do the cosmetic stuff if you really want to do it, like the change all ext to bool (note that there is no point if the callers still use "1" and "0": adapt them as well) etc.
>>>
>>> No, I don't agree.  Especially with respect to the insn type.
>>>
>>> I'd much rather do all the "cosmetic stuff", as you put it, first.  It makes
>>> all of the "real" changes much easier to understand.
>>>
>>>
>>> r~
>>>
>>
>> I guess we are stuck then. With the cosmetic and restructuring stuff coming before, I cannot cherry pick the good parts later.
>>
>>
> 
> Have you tested the first 9 patches on their own?  I.e.
> 
>>   tcg-aarch64: Set ext based on TCG_OPF_64BIT
>>   tcg-aarch64: Change all ext variables to bool
>>   tcg-aarch64: Don't handle mov/movi in tcg_out_op
>>   tcg-aarch64: Hoist common argument loads in tcg_out_op
>>   tcg-aarch64: Change enum aarch64_arith_opc to AArch64Insn
>>   tcg-aarch64: Merge enum aarch64_srr_opc with AArch64Insn
>>   tcg-aarch64: Introduce tcg_fmt_* functions
>>   tcg-aarch64: Introduce tcg_fmt_Rdn_aimm
>>   tcg-aarch64: Implement mov with tcg_fmt_* functions

yes.

> 
> There should be no functional change to the backend, producing 100% identical
> output code.  There should even be little or no change in tcg.o itself.

There are two aspects.

On one side, although some changes do not break anything, I see some problems in them.
Putting them as a prerequisite for the rest forces us to agreeing on everything before moving forward, instead of being able to agree on separate chunks (meat first, rest later). In my view, this makes the process longer.

On another side, I end up having to manually revert some parts of these which you put as prerequisites, during bisection when landing after them, which is a huge time drain when tracking regressions introduced in the later part of the series.

> This should make it trivial to verify that these patches are not at fault.
> 
> r~

They don't break the targets, no.

Claudio

Re: [Qemu-devel] [PATCH v3 00/29] tcg-aarch64 improvements

[Peter Maydell]

On 10 September 2013 09:27, Claudio Fontana <claudio.fontana@huawei.com> wrote:
> On another side, I end up having to manually revert some parts
> of these which you put as prerequisites, during bisection when
> landing after them, which is a huge time drain when tracking
> regressions introduced in the later part of the series.

I don't understand this; can you explain? If these early
refactoring patches have bugs then we should just identify
them and fix them. If they don't have bugs why would you
need to manually revert parts of them?

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 00/29] tcg-aarch64 improvements

[Claudio Fontana]

On 10.09.2013 10:45, Peter Maydell wrote:
> On 10 September 2013 09:27, Claudio Fontana <claudio.fontana@huawei.com> wrote:
>> On another side, I end up having to manually revert some parts
>> of these which you put as prerequisites, during bisection when
>> landing after them, which is a huge time drain when tracking
>> regressions introduced in the later part of the series.
> 
> I don't understand this; can you explain? If these early
> refactoring patches have bugs then we should just identify
> them and fix them. If they don't have bugs why would you
> need to manually revert parts of them?
> 

To revert the next patches which do introduce bugs.

I could not see bugs in the refactoring patches, but there is stuff to fix regardless of bugs.

Re: [Qemu-devel] [PATCH v3 00/29] tcg-aarch64 improvements

[Peter Maydell]

On 12 September 2013 09:03, Claudio Fontana <claudio.fontana@huawei.com> wrote:
> On 10.09.2013 10:45, Peter Maydell wrote:
>> On 10 September 2013 09:27, Claudio Fontana <claudio.fontana@huawei.com> wrote:
>>> On another side, I end up having to manually revert some parts
>>> of these which you put as prerequisites, during bisection when
>>> landing after them, which is a huge time drain when tracking
>>> regressions introduced in the later part of the series.
>>
>> I don't understand this; can you explain? If these early
>> refactoring patches have bugs then we should just identify
>> them and fix them. If they don't have bugs why would you
>> need to manually revert parts of them?

> To revert the next patches which do introduce bugs.

Huh? The next patches would apply on top of the refactoring
patches, so you don't need to remove the refactoring to
revert the functional changes. (On the other hand if we
did things the way round you're suggesting with the
functional changes first then we would need to revert
or manually undo the refactoring parts in order to
revert the functional change patches.)

Personally I think that "first refactor/clean up, then
add new features/improvements" is a fairly standard order
to do things.

-- PMM

Re: [Qemu-devel] [PATCH v3 00/29] tcg-aarch64 improvements

[Richard Henderson]

On 09/10/2013 01:27 AM, Claudio Fontana wrote:
> There are two aspects.
> 
> On one side, although some changes do not break anything, I see some problems in them.

Then let us discuss them, sooner rather than later.

> Putting them as a prerequisite for the rest forces us to agreeing on
> everything before moving forward, instead of being able to agree on separate
> chunks (meat first, rest later). In my view, this makes the process longer.

If we have no common ground on how the port should look, then we simply cannot
move forward full stop.

Having put together a foundation of AArch64Insn and tcg_fmt_*, that I believe
to be clean and easy to understand, I simply refuse on aesthetic grounds to
rewrite later patches to instead use the magic number and open-coded insn
format used throughout the port today.  That way leads to a much greater chance
of error in my opinion.


r~

Re: [Qemu-devel] [PATCH v3 00/29] tcg-aarch64 improvements

[Claudio Fontana]

On 10.09.2013 15:16, Richard Henderson wrote:
> On 09/10/2013 01:27 AM, Claudio Fontana wrote:
>> There are two aspects.
>>
>> On one side, although some changes do not break anything, I see some problems in them.
> 
> Then let us discuss them, sooner rather than later.
> 
>> Putting them as a prerequisite for the rest forces us to agreeing on
>> everything before moving forward, instead of being able to agree on separate
>> chunks (meat first, rest later). In my view, this makes the process longer.
> 
> If we have no common ground on how the port should look, then we simply cannot
> move forward full stop.
> 
> Having put together a foundation of AArch64Insn and tcg_fmt_*, that I believe
> to be clean and easy to understand, I simply refuse on aesthetic grounds to

on aesthetic grounds?

> rewrite later patches to instead use the magic number and open-coded insn
> format used throughout the port today.  That way leads to a much greater chance
> of error in my opinion.
> 

I just asked you to reorder the way you do things, so that I had less work to do when dissecting problems in the actual functional changes.
If it's really impossible for you to do that, I guess we can move forward anyway, it just creates more work here before we can have a chunk we agree on.

I will put additional comments on the parts that I would like to see improved.

Thanks,

Claudio