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