Re: [Qemu-devel] [PATCH v3 00/29] tcg-aarch64 improvements
On 10.09.2013 10:45, Peter Maydell wrote: On 10 September 2013 09:27, Claudio Fontana claudio.font...@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
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
Re: [Qemu-devel] [PATCH v3 00/29] tcg-aarch64 improvements
On 12 September 2013 09:03, Claudio Fontana claudio.font...@huawei.com wrote: On 10.09.2013 10:45, Peter Maydell wrote: On 10 September 2013 09:27, Claudio Fontana claudio.font...@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
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
On 10 September 2013 09:27, Claudio Fontana claudio.font...@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
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
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
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
On 9 September 2013 16:02, Claudio Fontana claudio.font...@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
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
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
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
On Tue, Sep 3, 2013 at 9:37 AM, Richard W.M. Jones rjo...@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
On 3 September 2013 08:37, Richard W.M. Jones rjo...@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
[Qemu-devel] [PATCH v3 00/29] tcg-aarch64 improvements
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
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
